Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 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
7 changes: 7 additions & 0 deletions src/install/TarballStream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -799,6 +799,13 @@ impl TarballStream {
let rest: &[OSPathChar] = tokenize_rest_after_first(&pathname[..]);

let mut norm_buf = OSPathBuffer::uninit();
// PAX paths are unbounded; +1 covers the Windows UNC/drive case
// where `normalize_buf_t` grows output by one, plus the NUL below.
Comment thread
robobun marked this conversation as resolved.
if rest.len() + 1 >= norm_buf.len() {
self.phase = Phase::WantData;
self.out_fd = None;
return Ok(());
}
let normalized =
resolve_path::normalize_buf_t::<OSPathChar, platform::Auto>(rest, &mut norm_buf[..]);
let norm_len = normalized.len();
Expand Down
4 changes: 3 additions & 1 deletion src/libarchive/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1494,7 +1494,9 @@ impl Archiver {
// at its original `.len()`; therefore `remaining[remaining.len()] == 0`.
let pathname: &[OSPathChar] = remaining;

if pathname.len() >= normalized_buf.len() {
// +1: `normalize_buf_t` can grow a Windows UNC/drive
// input by one, and we write a NUL after it.
Comment thread
robobun marked this conversation as resolved.
if pathname.len() + 1 >= normalized_buf.len() {
if options.log {
bun_core::warn!(
"Skipping entry with a path longer than the maximum path length: {}\n",
Expand Down
53 changes: 53 additions & 0 deletions test/cli/install/bun-install-streaming-extract.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,59 @@ describe("streaming tarball extraction", () => {
expect(exitCode).toBe(0);
});

// The streaming extractor normalizes each entry's path into a fixed-size
// OSPathBuffer. PAX extended headers can carry paths of arbitrary length,
// so an entry longer than that buffer must be skipped rather than
// overrunning it. The buffered extractor already has this guard; this
// covers the streaming path.
test("streaming extract skips an overlong PAX path instead of crashing", async () => {
// Longer than OSPathBuffer on every platform: 1024 on macOS/BSD,
// 4096 on Linux, 32767 u16 units on Windows. Built from hash output
// so gzip cannot collapse it; the compressed body must span multiple
// HTTP chunks for the streaming path to commit.
let longName = "";
let seed = createHash("sha256").update("overlong-pax").digest();
while (longName.length < 40_000) {
longName += seed.toString("hex");
seed = createHash("sha256").update(seed).digest();
}
longName = longName.slice(0, 40_000);

const { tgz, shasum, integrity } = buildTarball([
{
path: "package.json",
body: Buffer.from(JSON.stringify({ name: "stream-pkg", version: "1.0.0" }) + "\n"),
},
{ path: longName, body: Buffer.from("unreachable\n") },
{ path: "after.txt", body: Buffer.from("after\n") },
]);
expect(tgz.length).toBeGreaterThan(4096);

await using reg = await makeRegistry(tgz, shasum, integrity, 512);
using dir = tempDir("streaming-extract-overlong-pax", {
"package.json": JSON.stringify({
name: "app",
version: "1.0.0",
dependencies: { "stream-pkg": "1.0.0" },
}),
"bunfig.toml": `[install]\nregistry = "${reg.url}"\n`,
});

// The crafted tarball is small; lower the threshold so the streaming
// extractor commits to it instead of falling back to the buffered path.
const { stderr, exitCode } = await runInstall(String(dir), {
BUN_INSTALL_STREAMING_MIN_SIZE: "1",
});
expect(stderr).toContain("Streamed ");

const pkgRoot = join(String(dir), "node_modules", "stream-pkg");
expect(readFileSync(join(pkgRoot, "after.txt"), "utf8")).toBe("after\n");
// The overlong entry must produce no on-disk artifact, not even under
// the truncated ustar fallback name.
expect(await readdirSorted(pkgRoot)).toEqual(["after.txt", "package.json"]);
expect(exitCode).toBe(0);
});

test("streaming rejects a tarball whose integrity does not match", async () => {
// Serve the valid tarball but advertise the integrity of a
// *different* blob. Extraction will stream to completion (so we
Expand Down
Loading