Skip to content

Commit cb2ac09

Browse files
authored
fix(upgrade): bound untrusted patch newSize to prevent pre-verification OOM (#1279)
## What The TRDIFF10 patch header's `newSize` field is attacker-controlled, and `applyReaderToMemory` uses it to preallocate the output buffer (`new Uint8Array(newSize)`) **before** the patch content is verified. With no upper bound, a malicious or corrupt patch declaring a huge `newSize` (e.g. 8 GiB) forces an out-of-memory allocation — a crash (or worse) reachable from any patch the CLI downloads or loads from cache. ## Fix Adds `MAX_OUTPUT_SIZE` (2 GiB) and enforces it in `parsePatchHeader`, rejecting any `newSize` above the cap **before any allocation**. 2 GiB is far above any realistic sentry-cli binary (~30 MB), so legitimate patched outputs never approach the cap. This mirrors the same fix we made in the Lore gateway's ported copy of this file (where an adversarial security review flagged the identical bug). ## Tests - A header claiming `newSize = MAX_OUTPUT_SIZE + 1` is rejected (`"exceeds maximum"`). - A header at `newSize = MAX_OUTPUT_SIZE` (boundary) is still accepted. - **Mutation-verified:** removing the guard turns the over-cap test RED while the boundary test stays GREEN (sharp boundary). ## Verification - 17/17 `test/lib/bspatch.test.ts` pass, `tsc --noEmit` clean, `biome check` clean.
1 parent 7019b60 commit cb2ac09

2 files changed

Lines changed: 44 additions & 0 deletions

File tree

src/lib/bspatch.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,17 @@ const TRDIFF10_MAGIC = "TRDIFF10";
4545
/** Header size in bytes (magic + 3 × i64) */
4646
const HEADER_SIZE = 32;
4747

48+
/**
49+
* Upper bound on the declared output size (`newSize`) a patch may claim.
50+
*
51+
* The header's `newSize` is attacker-controlled and used to preallocate the
52+
* output buffer BEFORE the patch content is verified, so an unbounded value
53+
* would let a malicious or corrupt patch force an out-of-memory allocation.
54+
* 2 GiB is far above any realistic binary (sentry-cli binaries are ~30 MB);
55+
* legitimate patched outputs never approach it.
56+
*/
57+
export const MAX_OUTPUT_SIZE = 2_147_483_648; // 2 GiB
58+
4859
/** Parsed TRDIFF10 header fields */
4960
export type PatchHeader = {
5061
/** Compressed size of the control block (bytes) */
@@ -114,6 +125,14 @@ export function parsePatchHeader(patch: Uint8Array): PatchHeader {
114125
throw new Error("Corrupt patch: negative length in header");
115126
}
116127

128+
// Bound the attacker-controlled newSize before it is used to preallocate
129+
// the output buffer (applyReaderToMemory) — see MAX_OUTPUT_SIZE.
130+
if (newSize > MAX_OUTPUT_SIZE) {
131+
throw new Error(
132+
`Corrupt patch: declared output size ${newSize} exceeds maximum ${MAX_OUTPUT_SIZE} bytes`
133+
);
134+
}
135+
117136
const totalCompressed = HEADER_SIZE + controlLen + diffLen;
118137
if (totalCompressed > patch.byteLength) {
119138
throw new Error(

test/lib/bspatch.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
applyPatch,
2222
applyPatchChainInMemory,
2323
applyPatchToMemory,
24+
MAX_OUTPUT_SIZE,
2425
parsePatchHeader,
2526
} from "../../src/lib/bspatch.js";
2627

@@ -149,6 +150,30 @@ describe("parsePatchHeader: fixtures", () => {
149150
buf[15] = 0x80; // Set sign bit → negative controlLen
150151
expect(() => parsePatchHeader(buf)).toThrow("negative");
151152
});
153+
154+
test("rejects an attacker-controlled newSize above MAX_OUTPUT_SIZE", () => {
155+
// The header's newSize is used to preallocate the output buffer before the
156+
// patch content is verified. An unbounded value lets a malicious/corrupt
157+
// patch force an OOM via `new Uint8Array(newSize)` in applyReaderToMemory.
158+
// Regression test: a header claiming newSize > MAX_OUTPUT_SIZE must be
159+
// rejected here, before any allocation.
160+
const buf = new Uint8Array(64);
161+
buf.set(new TextEncoder().encode("TRDIFF10"));
162+
writeI64LE(buf, 8, 0); // controlLen = 0
163+
writeI64LE(buf, 16, 0); // diffLen = 0
164+
writeI64LE(buf, 24, MAX_OUTPUT_SIZE + 1); // newSize just over the cap
165+
expect(() => parsePatchHeader(buf)).toThrow("exceeds maximum");
166+
});
167+
168+
test("accepts a newSize at MAX_OUTPUT_SIZE (boundary)", () => {
169+
const buf = new Uint8Array(64);
170+
buf.set(new TextEncoder().encode("TRDIFF10"));
171+
writeI64LE(buf, 8, 0);
172+
writeI64LE(buf, 16, 0);
173+
writeI64LE(buf, 24, MAX_OUTPUT_SIZE); // exactly the cap — allowed
174+
const header = parsePatchHeader(buf);
175+
expect(header.newSize).toBe(MAX_OUTPUT_SIZE);
176+
});
152177
});
153178

154179
describe("applyPatch", () => {

0 commit comments

Comments
 (0)