Skip to content
Closed

ai slop #32679

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/install/lockfile/Tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
36 changes: 36 additions & 0 deletions src/install/lockfile/bun.lockb.rs
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,42 @@ pub(crate) fn load(
));
}

// 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();
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"
Expand Down
19 changes: 15 additions & 4 deletions src/install_types/resolver_hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,18 +109,29 @@ impl<T> ExternalSlice<T> {

#[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]
Expand Down
130 changes: 129 additions & 1 deletion test/cli/install/bun-lockb.test.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -176,6 +176,134 @@ 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", () => {
// 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
// (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);
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);
// 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<install.lockfile.Tree>");
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) {
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("pkg-a@");
expect(code).toBe(0);
expect(await exists(join(packageDir, "node_modules", "pkg-a", "package.json"))).toBe(true);
}

it.concurrent("package dependencies slice with off > buffer length", async () => {
const { packageDir, lockbPath, lockb, N, begin, resolutionSize } = await lockbWithPkgs();
// Dependencies column (ExternalSlice<Dependency>, 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<PackageID>, 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, treesStart } = await lockbWithPkgs();
// tree[0].dependencies.off
lockb.writeUInt32LE(0x7fffffff, treesStart + 0 * 20 + 12);
await expectRecovers(packageDir, lockbPath, lockb);
});
Comment thread
robobun marked this conversation as resolved.

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 () => {
const { packageDir, packageJson } = await registry.createTestDir({ bunfigOpts: { saveTextLockfile: false } });

Expand Down
Loading