Skip to content
Closed
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
27 changes: 25 additions & 2 deletions src/install/lockfile/Tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,14 +338,37 @@ pub(crate) fn relative_path_and_depth<'b, const PATH_STYLE: IteratorPathStyle>(

while parent_id > 0 && (parent_id as usize) < trees.len() {
if depth_buf_len == MAX_DEPTH {
path_buf[path_written] = 0;
return (ZStr::from_buf(path_buf, path_written), 0);
// The parent chain cannot legitimately be this long: MAX_DEPTH
// segments of "node_modules" alone would already overflow
// MAX_PATH_BYTES. Reaching it means the on-disk trees form a
// parent cycle (a corrupted bun.lockb). Returning a truncated
// path here would let the caller install/link into the wrong
// directory, so fail loudly like the other malformed-lockfile
// checks below.
Output::err_generic(
"Lockfile is malformed (dependency tree has a cycle or exceeds the maximum depth)",
(),
);
bun_core::Global::crash();
Comment thread
robobun marked this conversation as resolved.
}
depth_buf[depth_buf_len] = parent_id;
parent_id = trees[parent_id as usize].parent;
depth_buf_len += 1;
}

if parent_id != 0 {
// A well-formed parent chain always terminates at the root
// (id 0). Exiting the loop via the `parent_id < trees.len()`
// guard means a non-root node's parent points past the end of
// the trees buffer, so the path built below would be missing
// every segment above the break.
Output::err_generic(
"Lockfile is malformed (dependency tree parent is out of bounds)",
(),
);
bun_core::Global::crash();
}
Comment thread
robobun marked this conversation as resolved.

depth_buf_len -= 1;

depth = depth_buf_len;
Expand Down
83 changes: 83 additions & 0 deletions test/cli/install/bun-lockb.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,89 @@ it("recovers from a corrupted binary lockfile instead of panicking", async () =>
expect(await exists(join(packageDir, "node_modules", "a-dep"))).toBe(true);
});

it.each([
// tree[1].parent = 1: a self-cycle. Walking the parent chain never reaches
// the root and hits the MAX_DEPTH guard.
{ name: "form a cycle", parent: 1, reason: "cycle" },
// tree[1].parent = 99: points past trees.len(). The parent walk exits via
// the bounds guard without reaching the root, so the built path would be
// missing every segment above the break.
{ name: "point out of bounds", parent: 99, reason: "out of bounds" },
])("fails loudly when a binary lockfile's tree parents $name", async ({ parent, reason }) => {
const { packageDir, packageJson } = await registry.createTestDir({ bunfigOpts: { saveTextLockfile: false } });

// Folder deps are never hoisted (Tree.rs places them in their own tree
// unconditionally), so pkg-a → pkg-b gives a second Tree node without
// touching any registry.
await Promise.all([
write(
packageJson,
JSON.stringify({
name: "tree-parent-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);

// Each `writeArray` record in bun.lockb is
// [start:u64][end:u64]\n<type> <size> sizeof, <align> alignof\n[pad][payload]
// with start/end as absolute file offsets. Trees are 20-byte records
// laid out id|dep_id|parent|off|len (all u32 LE).
const lockb = Buffer.from(await file(lockbPath).arrayBuffer());
const marker = lockb.indexOf("\n<install.lockfile.Tree> 20 sizeof, 4 alignof\n");
expect(marker).toBeGreaterThan(0);
const treesStart = Number(lockb.readBigUInt64LE(marker - 16));
const treesEnd = Number(lockb.readBigUInt64LE(marker - 8));
const treeCount = (treesEnd - treesStart) / 20;
expect(treeCount).toBeGreaterThanOrEqual(2);

// Sanity: well-formed tree[i].id == i; tree[1].parent == 0.
const tree1 = treesStart + 1 * 20;
expect(lockb.readUInt32LE(treesStart + 0)).toBe(0);
expect(lockb.readUInt32LE(tree1 + 0)).toBe(1);
expect(lockb.readUInt32LE(tree1 + 8)).toBe(0);

lockb.writeUInt32LE(parent, tree1 + 8);
await write(lockbPath, lockb);

// `bun pm ls` iterates the on-disk trees directly (no re-hoist) so it
// reaches `relative_path_and_depth` with the corrupted parent.
const { stdout, stderr, exited } = spawn({
cmd: [bunExe(), "pm", "ls"],
cwd: packageDir,
stdout: "pipe",
stderr: "pipe",
env,
});
const [out, rawErr, code] = await Promise.all([stdout.text(), stderr.text(), exited]);
const err = stderrForInstall(rawErr);

// Previously the tree iterator silently returned a truncated path for
// the corrupted node and `bun pm ls` exited 0 having dropped the nested
// dependencies. The corrupted lockfile must be reported instead of
// producing a wrong path.
expect({ out, err }).toEqual({
out: expect.any(String),
err: expect.stringContaining("Lockfile is malformed"),
});
expect(err).toContain(reason);
expect(code).not.toBe(0);
});

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