quic: validate session close error codes

`session.close()` and `session.destroy()` only checked that
`options.code` was a bigint or a number. Numbers were then converted
to `uint64_t` unchecked, so fractions were truncated and negative,
non-finite or too large values were undefined behavior. Bigints were
only checked to fit in 64 bits, but QUIC error codes are
variable-length integers limited to 62 bits, so larger codes cannot
be encoded in the `CONNECTION_CLOSE` frame.

Reject codes that are not integers between `0` and `2n ** 62n - 1n`,
the range `QuicError` already enforces, before any close or destroy
side effects.

Signed-off-by: Christian Aurich Zanettini Martins <christian.aurichzm@gmail.com>
PR-URL: #66302
Reviewed-By: Tim Perry <pimterry@gmail.com>

Author: christianaurichzm

doc/api/quic.md

@@ -974,7 +974,8 @@ added: v23.8.0
 
 * `options` {Object}
   * `code` {bigint|number} The error code to include in the `CONNECTION_CLOSE`
-    frame sent to the peer. **Default:** `0` (no error).
+    frame sent to the peer. Must be a non-negative 62-bit unsigned varint
+    (`0n <= code <= 2n ** 62n - 1n`). **Default:** `0` (no error).
   * `type` {string} Either `'transport'` or `'application'`. Determines the
     error code namespace used in the `CONNECTION_CLOSE` frame. When `'transport'`
     (the default), the frame type is `0x1c` and the code is interpreted as a QUIC
@@ -1056,7 +1057,8 @@ added: v23.8.0
 * `error` {any}
 * `options` {Object}
   * `code` {bigint|number} The error code to include in the `CONNECTION_CLOSE`
-    frame sent to the peer. **Default:** `0`.
+    frame sent to the peer. Must be a non-negative 62-bit unsigned varint
+    (`0n <= code <= 2n ** 62n - 1n`). **Default:** `0`.
   * `type` {string} Either `'transport'` or `'application'`. **Default:**
     `'transport'`.
   * `reason` {string} An optional human-readable reason string included in

lib/internal/quic/quic.js

@@ -13,6 +13,7 @@ const {
   ErrorCaptureStackTrace,
   FunctionPrototypeBind,
   FunctionPrototypeCall,
+  NumberIsInteger,
   ObjectDefineProperties,
   ObjectKeys,
   PromisePrototypeThen,
@@ -5485,6 +5486,13 @@ function validateCloseOptions(options) {
       throw new ERR_INVALID_ARG_TYPE('options.code',
                                      ['bigint', 'number'], code);
     }
+    if (typeof code === 'number' && !NumberIsInteger(code)) {
+      throw new ERR_OUT_OF_RANGE('options.code', 'an integer', code);
+    }
+    if (code < 0 || code > kMaxQuicErrorCode) {
+      throw new ERR_OUT_OF_RANGE('options.code',
+                                 `>= 0 and <= ${kMaxQuicErrorCode}`, code);
+    }
   }
   validateOneOf(type, 'options.type', ['transport', 'application']);
   if (reason !== undefined) {

test/parallel/test-quic-session-destroy-validate-options.mjs

@@ -97,6 +97,20 @@ assert.throws(() => clientSession.destroy(goodError, { reason: 42 }), {
 assert.strictEqual(clientSession.destroyed, false);
 assert.strictEqual(stream.destroyed, false);
 
+// 5. options.code does not fit in a QUIC varint -> throws ERR_OUT_OF_RANGE,
+//    from both destroy() and close().
+for (const code of [-1, 1.5, NaN, Infinity, 2 ** 62, -1n, 2n ** 62n]) {
+  assert.throws(() => clientSession.destroy(goodError, { code }), {
+    code: 'ERR_OUT_OF_RANGE',
+  });
+  assert.throws(() => clientSession.close({ code }), {
+    code: 'ERR_OUT_OF_RANGE',
+  });
+}
+assert.strictEqual(clientSession.destroyed, false);
+assert.strictEqual(clientSession.closing, false);
+assert.strictEqual(stream.destroyed, false);
+
 // Now switch the handlers to expect the real teardown so the final
 // destroy with valid options can run cleanly.
 diagnostics_channel.unsubscribe('quic.session.error', errSub);
@@ -107,19 +121,22 @@ stream.onerror = mustCall((err) => { assert.strictEqual(err, goodError); });
 // final destroy, so the rejections do not race ahead of any awaits in
 // the test body. The client rejects with the original `goodError`;
 // the server decodes the CONNECTION_CLOSE frame transport code into
-// an `ERR_QUIC_TRANSPORT_ERROR`.
+// an `ERR_QUIC_TRANSPORT_ERROR`. The code is the largest one that fits
+// in a QUIC varint, so it must be accepted and arrive unchanged.
+const maxCode = 2n ** 62n - 1n;
 const clientClosedAssertion = assert.rejects(clientSession.closed, goodError);
 const serverClosedAssertion = assert.rejects(serverSession.closed, mustCall((err) => {
   assert.strictEqual(err.code, 'ERR_QUIC_TRANSPORT_ERROR');
+  assert.strictEqual(err.errorCode, maxCode);
   return true;
 }));
 
-// 5. Valid options after the failed attempts -> session destroys
+// 6. Valid options after the failed attempts -> session destroys
 //    normally, the underlying handle sends CONNECTION_CLOSE with the
 //    supplied transport code, and the local closed promise rejects
 //    with the original error.
 clientSession.destroy(goodError, {
-  code: 1n,
+  code: maxCode,
   type: 'transport',
   reason: 'after validation throw',
 });