Skip to content

Commit 98486dd

Browse files
christianaurichzmaduh95
authored andcommitted
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 a3ababc commit 98486dd

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
@@ -2657,8 +2657,8 @@ added: v23.8.0
26572657

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

26632663
#### `endpointOptions.disableStatelessReset`
26642664

‎src/quic/endpoint.cc‎

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

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