quic: drop version negotiation packets with oversized CIDs

Signed-off-by: Mohamed Sayed <k@3zrv.com>

Author: Mohamed Sayed

src/quic/endpoint.cc

@@ -1802,6 +1802,24 @@ void Endpoint::Receive(const uint8_t* data,
       // successfully decoded. Send a Version Negotiation response
       // per RFC 9000 Section 6. The VN packet's DCID is the client's
       // SCID and vice versa (mirrored back to the client).
+      //
+      // ngtcp2_pkt_decode_version_cid() only enforces the
+      // NGTCP2_MAX_CIDLEN limit for *supported* versions; for an
+      // unsupported version it returns the raw connection ID lengths
+      // taken from the single-byte length fields on the wire, which can
+      // be up to 255. Constructing a CID -- backed by a fixed
+      // NGTCP2_MAX_CIDLEN-byte buffer -- from such a length writes past
+      // the buffer (an assertion abort in release builds). A single
+      // unauthenticated UDP datagram could therefore crash the endpoint
+      // before any handshake. Drop these packets, mirroring the
+      // CID-length policy applied below for supported versions.
+      if (pversion_cid.dcidlen > NGTCP2_MAX_CIDLEN ||
+          pversion_cid.scidlen > NGTCP2_MAX_CIDLEN) {
+        Debug(this,
+              "Version negotiation packet had incorrectly sized CIDs, "
+              "ignoring");
+        return;
+      }
       Debug(this,
             "Packet version %d is not supported, sending version negotiation",
             pversion_cid.version);

test/parallel/test-quic-version-negotiation-oversized-cid.mjs

@@ -0,0 +1,84 @@
+// Flags: --experimental-quic --no-warnings
+
+// Regression test for an unauthenticated remote crash in the QUIC Version
+// Negotiation path.
+//
+// ngtcp2_pkt_decode_version_cid() does not clamp the connection ID lengths
+// to NGTCP2_MAX_CIDLEN (20) when the packet's version is unsupported -- it
+// returns the raw single-byte length fields from the wire, which can be up
+// to 255. Endpoint::Receive() used to build CID objects (each backed by a
+// fixed 20-byte buffer) directly from those lengths before any bound check,
+// so a single crafted UDP datagram with an oversized DCID/SCID length would
+// overflow the buffer and abort the process before the handshake.
+//
+// This test sends such a datagram directly with node:dgram and asserts the
+// endpoint drops it without crashing, while a well-formed unsupported-version
+// datagram still produces exactly one Version Negotiation response.
+
+import { hasQuic, skip, mustCall } from '../common/index.mjs';
+import assert from 'node:assert';
+
+const { strictEqual } = assert;
+
+if (!hasQuic) {
+  skip('QUIC is not enabled');
+}
+
+const { createSocket } = await import('node:dgram');
+const { listen } = await import('../common/quic.mjs');
+
+// A long-header QUIC packet must be at least NGTCP2_MAX_UDP_PAYLOAD_SIZE
+// (1200) bytes for an unsupported version to decode as a VN trigger.
+const PACKET_SIZE = 1200;
+
+// Build a QUIC long-header packet with an unsupported version and the given
+// DCID/SCID lengths. Lengths greater than 20 are only expressible because the
+// on-wire length field is a single byte (0-255).
+function buildLongHeaderPacket(dcidLen, scidLen) {
+  const packet = Buffer.alloc(PACKET_SIZE);
+  // Header form bit (0x80) + fixed bit (0x40).
+  packet[0] = 0xc0;
+  // Version 0x0a0a0a0a: nonzero (so it is not a real VN packet) and not a
+  // version Node supports, which forces the NGTCP2_ERR_VERSION_NEGOTIATION
+  // decode path.
+  packet.writeUInt32BE(0x0a0a0a0a, 1);
+  packet[5] = dcidLen;          // DCID length byte
+  // DCID bytes occupy [6, 6 + dcidLen); SCID length byte follows them.
+  packet[6 + dcidLen] = scidLen;
+  // Remaining bytes stay zero-filled as padding to reach PACKET_SIZE.
+  return packet;
+}
+
+const serverEndpoint = await listen(mustCall(() => {}, 0));
+const { address, port } = serverEndpoint.address;
+
+const socket = createSocket('udp4');
+
+function send(packet) {
+  return new Promise((resolve, reject) => {
+    socket.send(packet, port, address, (err) => err ? reject(err) : resolve());
+  });
+}
+
+// 1. Oversized DCID (21 > NGTCP2_MAX_CIDLEN). Before the fix this aborted the
+//    process. After the fix it must be dropped silently.
+await send(buildLongHeaderPacket(21, 0));
+
+// 2. A well-formed unsupported-version packet with valid (<= 20 byte) CIDs.
+//    This must still elicit exactly one Version Negotiation response, proving
+//    the fix did not break legitimate version negotiation.
+await send(buildLongHeaderPacket(8, 8));
+
+// Poll until the valid packet has been processed into a VN response.
+const deadline = Date.now() + 2000;
+while (serverEndpoint.stats.versionNegotiationCount === 0n) {
+  if (Date.now() > deadline) break;
+  await new Promise((resolve) => setTimeout(resolve, 25));
+}
+
+// Exactly one VN response: the oversized packet was dropped (not crashed, not
+// negotiated), the valid packet was negotiated.
+strictEqual(serverEndpoint.stats.versionNegotiationCount, 1n);
+
+socket.close();
+await serverEndpoint.close();