Skip to content

fix(encoding): throw on buffer overflow instead of silent truncation in encodeVarint (#7147)#7159

Open
LeSingh1 wants to merge 1 commit into
denoland:mainfrom
LeSingh1:fix/7147-varint-buffer-overflow
Open

fix(encoding): throw on buffer overflow instead of silent truncation in encodeVarint (#7147)#7159
LeSingh1 wants to merge 1 commit into
denoland:mainfrom
LeSingh1:fix/7147-varint-buffer-overflow

Conversation

@LeSingh1
Copy link
Copy Markdown

Fixes #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, 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 byte count it reported. Callers then fed that truncated buffer to decodeVarint and got a wrong number back. Repro from the issue:

console.log(encodeVarint(0x1234567891234567891n));
// before: [Uint8Array(10), 11]   (looks fine, but the slice is missing a byte)
// after:  RangeError: Cannot encode the input ... into varint as it overflows uint64

Fix

  • Cap the loop at min(buf.length - offset, MaxVarintLen64), write through offset + i so the offset path can't drift either.
  • Split the post-loop throw into two messages: "buffer holds at most N byte(s) after offset" when the caller (or default) supplied a short buffer, and the existing "overflows uint64" when the value genuinely exceeds the protocol limit. Both are RangeError so existing assertThrows(_, RangeError) callsites still pass.
  • The happy path is byte-for-byte identical: same returned slice, same byte count.

Tests

Three cases added to encoding/varint_test.ts:

  • throws when the default buffer is too small (#7147) — exact repro from issue, asserts RangeError with "overflows uint64"
  • throws when a caller-provided buffer is too small — 200000 (3 varint bytes) into a 2-byte buffer
  • throws when offset leaves no room in the buffer — 10-byte buffer with offset=10
$ deno test --allow-read encoding/varint_test.ts
ok | 18 passed | 0 failed (5ms)

$ deno lint encoding/varint.ts encoding/varint_test.ts && deno check encoding/varint.ts encoding/varint_test.ts
Checked 2 files
Check encoding/varint.ts
Check encoding/varint_test.ts

…enoland#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.
@codecov
Copy link
Copy Markdown

codecov Bot commented May 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.57%. Comparing base (f0c9f14) to head (71170f6).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7159   +/-   ##
=======================================
  Coverage   94.57%   94.57%           
=======================================
  Files         636      636           
  Lines       52138    52141    +3     
  Branches     9399     9400    +1     
=======================================
+ Hits        49311    49314    +3     
  Misses       2249     2249           
  Partials      578      578           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@LeSingh1 LeSingh1 changed the title fix(encoding/varint): throw on buffer overflow instead of silent truncation (#7147) fix(encoding): throw on buffer overflow instead of silent truncation in encodeVarint (#7147) May 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug encoding/varint.ts: Should panic, When default buffer is too small

1 participant