Skip to content

Commit 71170f6

Browse files
committed
fix(encoding/varint): throw on buffer overflow instead of truncating (#7147)
`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.
1 parent f0c9f14 commit 71170f6

2 files changed

Lines changed: 67 additions & 15 deletions

File tree

encoding/varint.ts

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -221,26 +221,38 @@ export function encodeVarint(
221221
buf: Uint8Array = new Uint8Array(MaxVarintLen64),
222222
offset = 0,
223223
): [Uint8Array_, number] {
224-
num = BigInt(num);
225-
if (num < 0n) {
224+
const input = BigInt(num);
225+
if (input < 0n) {
226226
throw new RangeError(
227-
`Cannot encode the input into varint as it should be non-negative integer: received ${num}`,
227+
`Cannot encode the input into varint as it should be non-negative integer: received ${input}`,
228228
);
229229
}
230-
for (
231-
let i = offset;
232-
i <= Math.min(buf.length, MaxVarintLen64);
233-
i += 1
234-
) {
235-
if (num < MSBN) {
236-
buf[i] = Number(num);
237-
i += 1;
238-
return [buf.slice(offset, i), i];
230+
// #7147: the previous loop used `i <= Math.min(buf.length, MaxVarintLen64)`,
231+
// which would walk one byte past the end of `buf` on the final iteration.
232+
// For values that needed all 10 uint64 varint bytes plus a continuation,
233+
// the OOB write was silently dropped by `Uint8Array`, the function
234+
// returned a too-short slice, and the caller never learned the buffer
235+
// was full. Cap the iteration count at the smaller of `buf.length`
236+
// (relative to `offset`) and `MaxVarintLen64`, then explicitly
237+
// distinguish "buffer too small" from "input overflows uint64".
238+
const cap = Math.min(buf.length - offset, MaxVarintLen64);
239+
let pending = input;
240+
for (let i = 0; i < cap; i += 1) {
241+
if (pending < MSBN) {
242+
buf[offset + i] = Number(pending);
243+
return [buf.slice(offset, offset + i + 1), offset + i + 1];
239244
}
240-
buf[i] = Number((num & 0xFFn) | MSBN);
241-
num >>= SHIFTN;
245+
buf[offset + i] = Number((pending & 0xFFn) | MSBN);
246+
pending >>= SHIFTN;
247+
}
248+
if (buf.length - offset < MaxVarintLen64) {
249+
throw new RangeError(
250+
`Cannot encode the input ${input} into varint: the provided buffer holds at most ${
251+
buf.length - offset
252+
} byte(s) after offset, but the value requires more`,
253+
);
242254
}
243255
throw new RangeError(
244-
`Cannot encode the input ${num} into varint as it overflows uint64`,
256+
`Cannot encode the input ${input} into varint as it overflows uint64`,
245257
);
246258
}

encoding/varint_test.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,46 @@ Deno.test("encodeVarint() throws on overflow with negative", () => {
120120
"Cannot encode the input into varint as it should be non-negative integer: received -1",
121121
);
122122
});
123+
Deno.test(
124+
"encodeVarint() throws when the default buffer is too small (#7147)",
125+
() => {
126+
// 0x1234567891234567891n is 76 bits → cannot fit in 10-byte uint64
127+
// varint and must throw. Previously the OOB write at the 11th byte was
128+
// silently dropped and the function returned `[Uint8Array(10), 11]`.
129+
assertThrows(
130+
() => encodeVarint(0x1234567891234567891n),
131+
RangeError,
132+
"overflows uint64",
133+
);
134+
},
135+
);
136+
137+
Deno.test(
138+
"encodeVarint() throws when a caller-provided buffer is too small",
139+
() => {
140+
// 200000 needs 3 varint bytes, buffer only has 2 → must throw rather
141+
// than silently truncate to 2 bytes.
142+
assertThrows(
143+
() => encodeVarint(200_000, new Uint8Array(2)),
144+
RangeError,
145+
"buffer holds at most 2 byte(s) after offset",
146+
);
147+
},
148+
);
149+
150+
Deno.test(
151+
"encodeVarint() throws when offset leaves no room in the buffer",
152+
() => {
153+
// 10-byte buf but offset=10 → 0 bytes available; any positive value
154+
// overflows the slice and must throw.
155+
assertThrows(
156+
() => encodeVarint(42n, new Uint8Array(10), 10),
157+
RangeError,
158+
"buffer holds at most 0 byte(s) after offset",
159+
);
160+
},
161+
);
162+
123163
Deno.test("encodeVarint() encodes with offset", () => {
124164
let uint = new Uint8Array(3);
125165
assertEquals(

0 commit comments

Comments
 (0)