From d571f24073e196a48024d598146aee5608b3b582 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 24 Jun 2026 17:45:25 +0000 Subject: [PATCH 1/4] install: reject out-of-bounds ExternalSlice offsets in bun.lockb ExternalSlice::get/mut_ clamped end to the buffer length but not off, so a crafted bun.lockb whose per-package dependencies/resolutions slice (or per-tree hoisted-dependencies slice) carried off > buffer.len() reached &in_[off..end] with off > end and panicked in release. Tree::folder_name likewise indexed the dependencies buffer with tree.dependency_id taken verbatim from disk. Clamp both bounds in get/mut_, bounds-check folder_name, and validate these offsets at bun.lockb load time so the installer reports InvalidLockfile and re-resolves instead of aborting. --- src/install/lockfile/Tree.rs | 2 +- src/install/lockfile/bun.lockb.rs | 39 ++++++++++ src/install_types/resolver_hooks.rs | 19 ++++- test/cli/install/bun-lockb.test.ts | 109 +++++++++++++++++++++++++++- 4 files changed, 163 insertions(+), 6 deletions(-) diff --git a/src/install/lockfile/Tree.rs b/src/install/lockfile/Tree.rs index 4f867c8e83f3..50a512c04d3b 100644 --- a/src/install/lockfile/Tree.rs +++ b/src/install/lockfile/Tree.rs @@ -90,7 +90,7 @@ pub(crate) fn depth_buf_uninit() -> DepthBuf { impl Tree { pub fn folder_name<'b>(&self, deps: &'b [Dependency], buf: &'b [u8]) -> &'b [u8] { let dep_id = self.dependency_id; - if dep_id == invalid_dependency_id { + if dep_id == invalid_dependency_id || dep_id as usize >= deps.len() { return b""; } deps[dep_id as usize].name.slice(buf) diff --git a/src/install/lockfile/bun.lockb.rs b/src/install/lockfile/bun.lockb.rs index 6dc55dca00e9..1ec198d1028c 100644 --- a/src/install/lockfile/bun.lockb.rs +++ b/src/install/lockfile/bun.lockb.rs @@ -427,6 +427,45 @@ pub(crate) fn load( )); } + // Per-package `dependencies`/`resolutions` and per-tree `dependencies` + // are `ExternalSlice` (off,len) pairs memcpy'd verbatim from disk; a + // crafted `off` past the backing buffer would otherwise reach + // `ExternalSlice::get` and panic. `tree.dependency_id` likewise indexes + // the dependencies buffer directly. Reject all of these here so the + // installer can warn + re-resolve. + { + let deps_len = lockfile.buffers.dependencies.len(); + let res_len = lockfile.buffers.resolutions.len(); + let hoisted_len = lockfile.buffers.hoisted_dependencies.len(); + let trees_len = lockfile.buffers.trees.len(); + for (dep_slice, res_slice) in lockfile + .packages + .items_dependencies() + .iter() + .zip(lockfile.packages.items_resolutions().iter()) + { + if !dep_slice.in_bounds(deps_len) || !res_slice.in_bounds(res_len) { + return Err(bun_core::err!("InvalidLockfile")); + } + } + for tree in lockfile.buffers.trees.iter() { + if !tree.dependencies.in_bounds(hoisted_len) { + return Err(bun_core::err!("InvalidLockfile")); + } + let dep_id = tree.dependency_id; + if dep_id != super::tree::ROOT_DEP_ID + && dep_id != crate::invalid_dependency_id + && dep_id as usize >= deps_len + { + return Err(bun_core::err!("InvalidLockfile")); + } + let parent = tree.parent; + if parent != super::tree::INVALID_ID && parent as usize >= trees_len { + return Err(bun_core::err!("InvalidLockfile")); + } + } + } + let has_workspace_name_hashes = false; // < Bun v1.0.4 stopped right here when reading the lockfile // So we add an extra 8 byte tag to say "hey, there's more data here" diff --git a/src/install_types/resolver_hooks.rs b/src/install_types/resolver_hooks.rs index b1aea8524e70..e79c36f616a8 100644 --- a/src/install_types/resolver_hooks.rs +++ b/src/install_types/resolver_hooks.rs @@ -109,18 +109,29 @@ impl ExternalSlice { #[inline] pub fn get(self, in_: &[T]) -> &[T] { - // Compute the sum in usize so - // the release-mode clamp applies instead of a debug u32-overflow panic. + // Both bounds are clamped: (off, len) may come straight off disk + // (bun.lockb, npm manifest cache), and a crafted `off > in_.len()` + // would otherwise panic in release even with `end` clamped. + let off = in_.len().min(self.off as usize); let end = in_.len().min(self.off as usize + self.len as usize); debug_assert!(self.off as usize + self.len as usize <= in_.len()); - &in_[self.off as usize..end] + &in_[off..end] } #[inline] pub fn mut_(self, in_: &mut [T]) -> &mut [T] { + let off = in_.len().min(self.off as usize); let end = in_.len().min(self.off as usize + self.len as usize); debug_assert!(self.off as usize + self.len as usize <= in_.len()); - &mut in_[self.off as usize..end] + &mut in_[off..end] + } + + /// True when `(off, len)` addresses a subrange of a buffer of length + /// `buf_len`. Used by the lockfile loader to reject crafted slices before + /// they reach `get()`/`mut_()`. + #[inline] + pub fn in_bounds(self, buf_len: usize) -> bool { + self.off as usize + self.len as usize <= buf_len } #[inline] diff --git a/test/cli/install/bun-lockb.test.ts b/test/cli/install/bun-lockb.test.ts index 69dd9e6df1c2..bdb1fd7b47dc 100644 --- a/test/cli/install/bun-lockb.test.ts +++ b/test/cli/install/bun-lockb.test.ts @@ -1,5 +1,5 @@ import { file, spawn, write } from "bun"; -import { afterAll, beforeAll, expect, it } from "bun:test"; +import { afterAll, beforeAll, describe, expect, it } from "bun:test"; import { copyFile, exists, open, rm, writeFile } from "fs/promises"; import { bunExe, bunEnv as env, isWindows, runBunInstall, stderrForInstall, VerdaccioRegistry } from "harness"; import { join } from "path"; @@ -176,6 +176,113 @@ it("recovers from a corrupted binary lockfile instead of panicking", async () => expect(await exists(join(packageDir, "node_modules", "a-dep"))).toBe(true); }); +describe("rejects crafted ExternalSlice offsets and tree indices in bun.lockb instead of panicking", () => { + // These offsets/indices are memcpy'd verbatim from disk and used to slice + // into / index the lockfile's backing buffers. A crafted value past the + // buffer end previously panicked in release builds (slice index order / + // bounds check) when a victim ran `bun install` in a repo carrying a + // tampered bun.lockb. Each case below must be rejected at load time so the + // installer warns + re-resolves rather than aborting. + async function lockbWithPkgs() { + const { packageDir, packageJson } = await registry.createTestDir({ bunfigOpts: { saveTextLockfile: false } }); + // Local file: tarballs keep this hermetic; the crafted lockfile is the + // only thing being exercised. + await copyFile(join(__dirname, "bar-0.0.2.tgz"), join(packageDir, "bar-0.0.2.tgz")); + await copyFile(join(__dirname, "baz-0.0.3.tgz"), join(packageDir, "baz-0.0.3.tgz")); + await write( + packageJson, + JSON.stringify({ + name: "external-slice-lockb", + version: "1.0.0", + dependencies: { + "bar": "file:./bar-0.0.2.tgz", + "baz": "file:./baz-0.0.3.tgz", + }, + }), + ); + await runBunInstall(env, packageDir); + const lockbPath = join(packageDir, "bun.lockb"); + expect(await exists(lockbPath)).toBe(true); + const lockb = Buffer.from(await file(lockbPath).arrayBuffer()); + const fmt = lockb.readUInt32LE(42); + const N = Number(lockb.readBigUInt64LE(86)); + const begin = Number(lockb.readBigUInt64LE(110)); + const resolutionSize = fmt === 2 ? 64 : 72; + expect(N).toBeGreaterThanOrEqual(3); + return { packageDir, lockbPath, lockb, N, begin, resolutionSize }; + } + + async function expectRecovers(packageDir: string, lockbPath: string, lockb: Buffer) { + await write(lockbPath, lockb); + await rm(join(packageDir, "node_modules"), { recursive: true, force: true }); + + const { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), "install", "--no-progress"], + cwd: packageDir, + stdout: "pipe", + stderr: "pipe", + env, + }); + const [out, rawErr, code] = await Promise.all([stdout.text(), stderr.text(), exited]); + const err = stderrForInstall(rawErr); + + expect(err).toContain("Ignoring lockfile"); + expect(err).not.toContain("error:"); + expect(out).toContain("bar@"); + expect(out).toContain("baz@"); + expect(code).toBe(0); + expect(await exists(join(packageDir, "node_modules", "bar"))).toBe(true); + expect(await exists(join(packageDir, "node_modules", "baz"))).toBe(true); + } + + it.concurrent("package dependencies slice with off > buffer length", async () => { + const { packageDir, lockbPath, lockb, N, begin, resolutionSize } = await lockbWithPkgs(); + // Dependencies column (ExternalSlice, 8 bytes each: off u32 + len u32). + const depsCol = begin + N * (8 + 8 + resolutionSize); + // Sanity: the root package's dependencies slice starts at 0. + expect(lockb.readUInt32LE(depsCol + 0 * 8)).toBe(0); + // Craft package[1].dependencies.off far past any plausible buffer end. + lockb.writeUInt32LE(0x7fffffff, depsCol + 1 * 8); + await expectRecovers(packageDir, lockbPath, lockb); + }); + + it.concurrent("package resolutions slice with off > buffer length", async () => { + const { packageDir, lockbPath, lockb, N, begin, resolutionSize } = await lockbWithPkgs(); + // Resolutions column (ExternalSlice, 8 bytes each) follows Dependencies. + const resCol = begin + N * (8 + 8 + resolutionSize + 8); + expect(lockb.readUInt32LE(resCol + 0 * 8)).toBe(0); + lockb.writeUInt32LE(0x7fffffff, resCol + 1 * 8); + await expectRecovers(packageDir, lockbPath, lockb); + }); + + it.concurrent("package dependencies slice with off + len > buffer length", async () => { + const { packageDir, lockbPath, lockb, N, begin, resolutionSize } = await lockbWithPkgs(); + const depsCol = begin + N * (8 + 8 + resolutionSize); + expect(lockb.readUInt32LE(depsCol + 0 * 8)).toBe(0); + // Craft package[1].dependencies.len far past the buffer end while + // leaving off in range, so `off..end` with only end clamped would + // succeed but the slice still addresses past the buffer. + lockb.writeUInt32LE(0x7fffffff, depsCol + 1 * 8 + 4); + await expectRecovers(packageDir, lockbPath, lockb); + }); + + it.concurrent("tree dependencies slice with off > hoisted buffer length", async () => { + const { packageDir, lockbPath, lockb } = await lockbWithPkgs(); + // Trees are the first `Buffers` array, written as + // [start: u64][end: u64][ascii prefix][padding][20-byte records]. Each + // record is id(4) dependency_id(4) parent(4) off(4) len(4). + const prefixOff = lockb.indexOf("\n"); + expect(prefixOff).toBeGreaterThan(16); + const treesStart = Number(lockb.readBigUInt64LE(prefixOff - 16)); + const treesEnd = Number(lockb.readBigUInt64LE(prefixOff - 8)); + expect((treesEnd - treesStart) % 20).toBe(0); + expect(treesEnd - treesStart).toBeGreaterThanOrEqual(20); + // tree[0].dependencies.off + lockb.writeUInt32LE(0x7fffffff, treesStart + 0 * 20 + 12); + await expectRecovers(packageDir, lockbPath, lockb); + }); +}); + it("rejects a binary lockfile whose patched-dependency flag byte is out of range", async () => { const { packageDir, packageJson } = await registry.createTestDir({ bunfigOpts: { saveTextLockfile: false } }); From 7944d4625ee5c42500d03606c2cee46dc714a12e Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 24 Jun 2026 18:11:53 +0000 Subject: [PATCH 2/4] test: cover tree.dependency_id and tree.parent guards Switch the crafted-lockb fixture to nested folder deps so the lockfile has a non-root tree, then corrupt tree[1].dependency_id and tree[1].parent. Deleting either guard in bun.lockb.rs now fails its matching test. --- test/cli/install/bun-lockb.test.ts | 84 +++++++++++++++++++----------- 1 file changed, 54 insertions(+), 30 deletions(-) diff --git a/test/cli/install/bun-lockb.test.ts b/test/cli/install/bun-lockb.test.ts index bdb1fd7b47dc..adaf54691580 100644 --- a/test/cli/install/bun-lockb.test.ts +++ b/test/cli/install/bun-lockb.test.ts @@ -185,21 +185,28 @@ describe("rejects crafted ExternalSlice offsets and tree indices in bun.lockb in // installer warns + re-resolves rather than aborting. async function lockbWithPkgs() { const { packageDir, packageJson } = await registry.createTestDir({ bunfigOpts: { saveTextLockfile: false } }); - // Local file: tarballs keep this hermetic; the crafted lockfile is the - // only thing being exercised. - await copyFile(join(__dirname, "bar-0.0.2.tgz"), join(packageDir, "bar-0.0.2.tgz")); - await copyFile(join(__dirname, "baz-0.0.3.tgz"), join(packageDir, "baz-0.0.3.tgz")); - await write( - packageJson, - JSON.stringify({ - name: "external-slice-lockb", - version: "1.0.0", - dependencies: { - "bar": "file:./bar-0.0.2.tgz", - "baz": "file:./baz-0.0.3.tgz", - }, - }), - ); + // Local folder deps keep this hermetic and produce a second Tree node + // (folder deps are never hoisted), so both package columns and + // tree[1]'s fields are available to corrupt. + await Promise.all([ + write( + packageJson, + JSON.stringify({ + name: "external-slice-lockb", + version: "1.0.0", + dependencies: { "pkg-a": "file:./pkg-a" }, + }), + ), + write( + join(packageDir, "pkg-a", "package.json"), + JSON.stringify({ + name: "pkg-a", + version: "1.0.0", + dependencies: { "pkg-b": "file:../pkg-b" }, + }), + ), + write(join(packageDir, "pkg-b", "package.json"), JSON.stringify({ name: "pkg-b", version: "1.0.0" })), + ]); await runBunInstall(env, packageDir); const lockbPath = join(packageDir, "bun.lockb"); expect(await exists(lockbPath)).toBe(true); @@ -209,7 +216,16 @@ describe("rejects crafted ExternalSlice offsets and tree indices in bun.lockb in const begin = Number(lockb.readBigUInt64LE(110)); const resolutionSize = fmt === 2 ? 64 : 72; expect(N).toBeGreaterThanOrEqual(3); - return { packageDir, lockbPath, lockb, N, begin, resolutionSize }; + // Trees are the first `Buffers` array, written as + // [start: u64][end: u64][ascii prefix][padding][20-byte records]. Each + // record is id(4) dependency_id(4) parent(4) off(4) len(4). + const prefixOff = lockb.indexOf("\n"); + expect(prefixOff).toBeGreaterThan(16); + const treesStart = Number(lockb.readBigUInt64LE(prefixOff - 16)); + const treesEnd = Number(lockb.readBigUInt64LE(prefixOff - 8)); + expect((treesEnd - treesStart) % 20).toBe(0); + expect(treesEnd - treesStart).toBeGreaterThanOrEqual(40); + return { packageDir, lockbPath, lockb, N, begin, resolutionSize, treesStart }; } async function expectRecovers(packageDir: string, lockbPath: string, lockb: Buffer) { @@ -228,11 +244,9 @@ describe("rejects crafted ExternalSlice offsets and tree indices in bun.lockb in expect(err).toContain("Ignoring lockfile"); expect(err).not.toContain("error:"); - expect(out).toContain("bar@"); - expect(out).toContain("baz@"); + expect(out).toContain("pkg-a@"); expect(code).toBe(0); - expect(await exists(join(packageDir, "node_modules", "bar"))).toBe(true); - expect(await exists(join(packageDir, "node_modules", "baz"))).toBe(true); + expect(await exists(join(packageDir, "node_modules", "pkg-a", "package.json"))).toBe(true); } it.concurrent("package dependencies slice with off > buffer length", async () => { @@ -267,20 +281,30 @@ describe("rejects crafted ExternalSlice offsets and tree indices in bun.lockb in }); it.concurrent("tree dependencies slice with off > hoisted buffer length", async () => { - const { packageDir, lockbPath, lockb } = await lockbWithPkgs(); - // Trees are the first `Buffers` array, written as - // [start: u64][end: u64][ascii prefix][padding][20-byte records]. Each - // record is id(4) dependency_id(4) parent(4) off(4) len(4). - const prefixOff = lockb.indexOf("\n"); - expect(prefixOff).toBeGreaterThan(16); - const treesStart = Number(lockb.readBigUInt64LE(prefixOff - 16)); - const treesEnd = Number(lockb.readBigUInt64LE(prefixOff - 8)); - expect((treesEnd - treesStart) % 20).toBe(0); - expect(treesEnd - treesStart).toBeGreaterThanOrEqual(20); + const { packageDir, lockbPath, lockb, treesStart } = await lockbWithPkgs(); // tree[0].dependencies.off lockb.writeUInt32LE(0x7fffffff, treesStart + 0 * 20 + 12); await expectRecovers(packageDir, lockbPath, lockb); }); + + it.concurrent("tree dependency_id past dependencies buffer", async () => { + const { packageDir, lockbPath, lockb, treesStart } = await lockbWithPkgs(); + // Sanity: tree[1] is a non-root tree with a real dependency_id and + // parent; its sentinels are not in play. + expect(lockb.readUInt32LE(treesStart + 1 * 20 + 0)).toBe(1); + expect(lockb.readUInt32LE(treesStart + 1 * 20 + 8)).toBe(0); + // tree[1].dependency_id + lockb.writeUInt32LE(0x7fffffff, treesStart + 1 * 20 + 4); + await expectRecovers(packageDir, lockbPath, lockb); + }); + + it.concurrent("tree parent past trees buffer", async () => { + const { packageDir, lockbPath, lockb, treesStart } = await lockbWithPkgs(); + expect(lockb.readUInt32LE(treesStart + 1 * 20 + 8)).toBe(0); + // tree[1].parent + lockb.writeUInt32LE(0x7fffffff, treesStart + 1 * 20 + 8); + await expectRecovers(packageDir, lockbPath, lockb); + }); }); it("rejects a binary lockfile whose patched-dependency flag byte is out of range", async () => { From 7eb4c673d9806e9c9c6f833c83808d3e214b2e3a Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 24 Jun 2026 19:09:07 +0000 Subject: [PATCH 3/4] ci: retrigger From 9a2d73297516d1e5920ac10ac7a469207039d367 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Wed, 24 Jun 2026 19:17:01 +0000 Subject: [PATCH 4/4] review: shorten comments to 3 lines --- src/install/lockfile/bun.lockb.rs | 9 +++------ test/cli/install/bun-lockb.test.ts | 9 +++------ 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/install/lockfile/bun.lockb.rs b/src/install/lockfile/bun.lockb.rs index 1ec198d1028c..6ba3140220bb 100644 --- a/src/install/lockfile/bun.lockb.rs +++ b/src/install/lockfile/bun.lockb.rs @@ -427,12 +427,9 @@ pub(crate) fn load( )); } - // Per-package `dependencies`/`resolutions` and per-tree `dependencies` - // are `ExternalSlice` (off,len) pairs memcpy'd verbatim from disk; a - // crafted `off` past the backing buffer would otherwise reach - // `ExternalSlice::get` and panic. `tree.dependency_id` likewise indexes - // the dependencies buffer directly. Reject all of these here so the - // installer can warn + re-resolve. + // Per-package and per-tree `ExternalSlice` (off,len) pairs and tree + // indices are memcpy'd verbatim from disk; reject out-of-range ones so + // the installer warns + re-resolves instead of panicking at use. { let deps_len = lockfile.buffers.dependencies.len(); let res_len = lockfile.buffers.resolutions.len(); diff --git a/test/cli/install/bun-lockb.test.ts b/test/cli/install/bun-lockb.test.ts index adaf54691580..6ccd8eabeb48 100644 --- a/test/cli/install/bun-lockb.test.ts +++ b/test/cli/install/bun-lockb.test.ts @@ -177,12 +177,9 @@ it("recovers from a corrupted binary lockfile instead of panicking", async () => }); describe("rejects crafted ExternalSlice offsets and tree indices in bun.lockb instead of panicking", () => { - // These offsets/indices are memcpy'd verbatim from disk and used to slice - // into / index the lockfile's backing buffers. A crafted value past the - // buffer end previously panicked in release builds (slice index order / - // bounds check) when a victim ran `bun install` in a repo carrying a - // tampered bun.lockb. Each case below must be rejected at load time so the - // installer warns + re-resolves rather than aborting. + // Disk-loaded (off,len) slices and tree indices past their backing buffer + // must be rejected at load time so a tampered bun.lockb warns + re-resolves + // instead of panicking on the slice index. async function lockbWithPkgs() { const { packageDir, packageJson } = await registry.createTestDir({ bunfigOpts: { saveTextLockfile: false } }); // Local folder deps keep this hermetic and produce a second Tree node