From 71170f607183bc55500041ed3f07f75377261421 Mon Sep 17 00:00:00 2001 From: Shaurya Singh Date: Tue, 26 May 2026 14:28:01 -0700 Subject: [PATCH] fix(encoding/varint): throw on buffer overflow instead of truncating (#7147) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `encodeVarint` walked the loop with `i <= Math.min(buf.length, MaxVarintLen64)`. For a value that needed all 10 uint64 varint bytes plus a continuation (e.g. `0x1234567891234567891n`), the 11th iteration wrote to `buf[10]` on the default 10-byte buffer — silently dropped by `Uint8Array` — and the function returned `[Uint8Array(10), 11]`, a slice shorter than the reported byte count. Callers then handed that truncated buffer to `decodeVarint` and got a wrong number back. Cap the loop at `min(buf.length - offset, MaxVarintLen64)`, write through `offset + i` so the offset path can't drift either, and split the post-loop throw into two messages: "buffer holds at most N byte(s) after offset" when the user supplied (or defaulted to) a short buffer, and the existing "overflows uint64" when the value genuinely exceeds the protocol limit. The success path returns identical results to before. Three new tests in `encoding/varint_test.ts`: the issue's exact 76-bit value (must throw "overflows uint64"), a 3-byte value into a 2-byte buffer (must throw "buffer holds at most 2 bytes"), and offset == buf length (must throw "buffer holds at most 0 bytes"). Full suite stays green: 18 passed. --- encoding/varint.ts | 42 ++++++++++++++++++++++++++--------------- encoding/varint_test.ts | 40 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 15 deletions(-) diff --git a/encoding/varint.ts b/encoding/varint.ts index de7f1a748080..478035178cf8 100644 --- a/encoding/varint.ts +++ b/encoding/varint.ts @@ -221,26 +221,38 @@ export function encodeVarint( buf: Uint8Array = new Uint8Array(MaxVarintLen64), offset = 0, ): [Uint8Array_, number] { - num = BigInt(num); - if (num < 0n) { + const input = BigInt(num); + if (input < 0n) { throw new RangeError( - `Cannot encode the input into varint as it should be non-negative integer: received ${num}`, + `Cannot encode the input into varint as it should be non-negative integer: received ${input}`, ); } - for ( - let i = offset; - i <= Math.min(buf.length, MaxVarintLen64); - i += 1 - ) { - if (num < MSBN) { - buf[i] = Number(num); - i += 1; - return [buf.slice(offset, i), i]; + // #7147: the previous loop used `i <= Math.min(buf.length, MaxVarintLen64)`, + // which would walk one byte past the end of `buf` on the final iteration. + // For values that needed all 10 uint64 varint bytes plus a continuation, + // the OOB write was silently dropped by `Uint8Array`, the function + // returned a too-short slice, and the caller never learned the buffer + // was full. Cap the iteration count at the smaller of `buf.length` + // (relative to `offset`) and `MaxVarintLen64`, then explicitly + // distinguish "buffer too small" from "input overflows uint64". + const cap = Math.min(buf.length - offset, MaxVarintLen64); + let pending = input; + for (let i = 0; i < cap; i += 1) { + if (pending < MSBN) { + buf[offset + i] = Number(pending); + return [buf.slice(offset, offset + i + 1), offset + i + 1]; } - buf[i] = Number((num & 0xFFn) | MSBN); - num >>= SHIFTN; + buf[offset + i] = Number((pending & 0xFFn) | MSBN); + pending >>= SHIFTN; + } + if (buf.length - offset < MaxVarintLen64) { + throw new RangeError( + `Cannot encode the input ${input} into varint: the provided buffer holds at most ${ + buf.length - offset + } byte(s) after offset, but the value requires more`, + ); } throw new RangeError( - `Cannot encode the input ${num} into varint as it overflows uint64`, + `Cannot encode the input ${input} into varint as it overflows uint64`, ); } diff --git a/encoding/varint_test.ts b/encoding/varint_test.ts index 8a4ba06021f3..b8fa992ba1c6 100644 --- a/encoding/varint_test.ts +++ b/encoding/varint_test.ts @@ -120,6 +120,46 @@ Deno.test("encodeVarint() throws on overflow with negative", () => { "Cannot encode the input into varint as it should be non-negative integer: received -1", ); }); +Deno.test( + "encodeVarint() throws when the default buffer is too small (#7147)", + () => { + // 0x1234567891234567891n is 76 bits → cannot fit in 10-byte uint64 + // varint and must throw. Previously the OOB write at the 11th byte was + // silently dropped and the function returned `[Uint8Array(10), 11]`. + assertThrows( + () => encodeVarint(0x1234567891234567891n), + RangeError, + "overflows uint64", + ); + }, +); + +Deno.test( + "encodeVarint() throws when a caller-provided buffer is too small", + () => { + // 200000 needs 3 varint bytes, buffer only has 2 → must throw rather + // than silently truncate to 2 bytes. + assertThrows( + () => encodeVarint(200_000, new Uint8Array(2)), + RangeError, + "buffer holds at most 2 byte(s) after offset", + ); + }, +); + +Deno.test( + "encodeVarint() throws when offset leaves no room in the buffer", + () => { + // 10-byte buf but offset=10 → 0 bytes available; any positive value + // overflows the slice and must throw. + assertThrows( + () => encodeVarint(42n, new Uint8Array(10), 10), + RangeError, + "buffer holds at most 0 byte(s) after offset", + ); + }, +); + Deno.test("encodeVarint() encodes with offset", () => { let uint = new Uint8Array(3); assertEquals(