Skip to content

Commit 72c1fea

Browse files
christianaurichzmchristianaurichzm
authored
quic: reject zero addressLRUSize
SocketAddressLRU::Upsert always inserts an entry before evicting down to max_size_. With max_size_ == 0, it evicts the entry it just inserted and then accesses the now-missing key via map_[address]->second. operator[] recreates the key with a default-constructed std::list iterator, which is then dereferenced. This is undefined behavior, observed as a SIGSEGV in Endpoint::Receive on the first UDP packet accepted by a QuicEndpoint constructed with { addressLRUSize: 0 }. SocketAddressLRU has no useful semantics for a zero-capacity cache, and Upsert's callers rely on it returning a valid pointer. Reject 0 (and 0n) at the options-parsing boundary instead of changing Upsert's contract. Signed-off-by: Christian Aurich <christian.aurichzm@gmail.com> PR-URL: #65827 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
1 parent 1e0ebef commit 72c1fea

3 files changed

Lines changed: 14 additions & 3 deletions

File tree

‎doc/api/quic.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2779,8 +2779,8 @@ added: v23.8.0
27792779

27802780
The endpoint maintains an internal cache of validated socket addresses as a
27812781
performance optimization. This option sets the maximum number of addresses
2782-
that are cached. This is an advanced option that users typically won't have
2783-
need to specify.
2782+
that are cached. The value must be greater than `0`. This is an advanced option
2783+
that users typically won't have need to specify.
27842784

27852785
#### `endpointOptions.disableStatelessReset`
27862786

‎src/quic/endpoint.cc‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,17 @@ Maybe<Endpoint::Options> Endpoint::Options::From(Environment* env,
249249
return Nothing<Options>();
250250
}
251251

252+
// SocketAddressLRU::Upsert requires a positive capacity. With max_size_ ==
253+
// 0, the newly inserted entry is immediately evicted, and the final
254+
// map_[address] creates a default list iterator that is then
255+
// dereferenced, causing UB (observed as a SIGSEGV in Endpoint::Receive on
256+
// the first accepted connection).
257+
if (options.address_lru_size == 0) {
258+
THROW_ERR_INVALID_ARG_VALUE(
259+
env, "The addressLRUSize option must be greater than 0");
260+
return Nothing<Options>();
261+
}
262+
252263
Local<Value> address;
253264
if (!params->Get(env->context(), env->address_string()).ToLocal(&address)) {
254265
return Nothing<Options>();

‎test/parallel/test-quic-internal-endpoint-options.mjs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ const cases = [
5656
valid: [
5757
1, 10, 100, 1000, 10000, 10000n,
5858
],
59-
invalid: [-1, -1n, 'a', null, false, true, {}, [], () => {}]
59+
invalid: [-1, -1n, 0, 0n, 'a', null, false, true, {}, [], () => {}]
6060
},
6161
{
6262
key: 'retryRate',

0 commit comments

Comments
 (0)