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
85 changes: 85 additions & 0 deletions src/libarchive/libarchive.zig
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,42 @@ fn isSymlinkTargetSafe(symlink_path: []const u8, link_target: [:0]const u8, syml
return strings.hasPrefix(resolved, fake_root);
}

/// Verifies that a path within the extraction directory resolves (via realpath,
/// which follows all symlinks) to a location still within the extraction directory.
/// This catches chained symlink attacks where individually-safe symlinks combine
/// to escape the extraction boundary.
///
/// Returns true if the resolved path is safe (stays within extraction dir).
fn isResolvedPathSafe(dir: std.fs.Dir, path_slice: []const u8, realpath_buf: *bun.PathBuffer) bool {
// Resolve the real filesystem path, following all symlinks.
const resolved = dir.realpath(path_slice, realpath_buf) catch {
// If we can't resolve (e.g., broken symlink), treat as safe since
// the path doesn't actually lead anywhere exploitable.
return true;
};

// Now get the real path of the extraction directory itself.
var dir_realpath_buf: bun.PathBuffer = undefined;
const dir_resolved = dir.realpath(".", &dir_realpath_buf) catch {
// If we can't resolve the extraction dir itself, allow the operation
// since we can't perform the check.
return true;
};

// The resolved path must start with the extraction directory's real path.
if (!strings.hasPrefix(resolved, dir_resolved)) {
return false;
}

// Also verify there's either nothing after the prefix, or a path separator follows.
// This prevents "/extract2" from matching "/extract".
if (resolved.len > dir_resolved.len and resolved[dir_resolved.len] != '/') {
return false;
}

return true;
}

pub const Archiver = struct {
// impl: *lib.archive = undefined,
// buf: []const u8 = undefined,
Expand Down Expand Up @@ -353,6 +389,8 @@ pub const Archiver = struct {
var symlink_join_buf: ?*bun.PathBuffer = null;
defer if (symlink_join_buf) |join_buf| bun.path_buffer_pool.put(join_buf);

var realpath_buf: bun.PathBuffer = undefined;

var normalized_buf: bun.OSPathBuffer = undefined;
var use_pwrite = Environment.isPosix;
var use_lseek = true;
Expand Down Expand Up @@ -446,6 +484,21 @@ pub const Archiver = struct {

switch (kind) {
.directory => {
if (Environment.isPosix) {
// Verify that the directory's parent resolves within the
// extraction directory. This prevents creating directories
// through symlink chains that escape the extraction boundary.
const parent_dir = std.fs.path.dirname(path_slice) orelse "";
if (parent_dir.len > 0 and !isResolvedPathSafe(dir, parent_dir, &realpath_buf)) {
if (options.log) {
Output.warn("Skipping directory whose parent resolves outside extraction dir: {f}\n", .{
bun.fmt.fmtOSPath(path_slice, .{}),
});
}
continue;
}
}

var mode = @as(i32, @intCast(entry.perm()));

// if dirs are readable, then they should be listable
Expand Down Expand Up @@ -495,9 +548,41 @@ pub const Archiver = struct {
else => return err,
}
};

// After creating the symlink, verify that it resolves to a path
// within the extraction directory. This catches chained symlink
// attacks where multiple individually-safe symlinks combine to
// escape the extraction boundary.
if (!isResolvedPathSafe(dir, path_slice, &realpath_buf)) {
// The symlink chain resolves outside the extraction dir.
// Remove the symlink we just created and skip this entry.
dir.deleteFile(path_slice) catch {};
if (options.log) {
Output.warn("Removing symlink whose resolved path escapes extraction dir: {f} -> {s}\n", .{
bun.fmt.fmtOSPath(path_slice, .{}),
link_target,
});
}
continue;
}
}
},
.file => {
if (Environment.isPosix) {
// Verify that the file's parent directory resolves within the
// extraction directory. This prevents writing files through
// symlink chains that escape the extraction boundary.
const parent_dir = std.fs.path.dirname(path_slice) orelse "";
if (parent_dir.len > 0 and !isResolvedPathSafe(dir, parent_dir, &realpath_buf)) {
if (options.log) {
Output.warn("Skipping file whose parent resolves outside extraction dir: {f}\n", .{
bun.fmt.fmtOSPath(path_slice, .{}),
});
}
continue;
}
}

// first https://github.com/npm/cli/blob/feb54f7e9a39bd52519221bae4fafc8bc70f235e/node_modules/pacote/lib/fetcher.js#L65-L66
// this.fmode = opts.fmode || 0o666
//
Expand Down
130 changes: 130 additions & 0 deletions test/regression/issue/symlink-chain-traversal.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
import { describe, expect, test } from "bun:test";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Minor: This test file is in test/regression/issue/ but doesn't have a GitHub issue number in its filename. Per CLAUDE.md:

If a test is for a specific numbered GitHub Issue, it should be placed in test/regression/issue/${issueNumber}.test.ts. Ensure the issue number is REAL and not a placeholder!
If no valid issue number is provided, find the best existing file to modify instead.

And test/CLAUDE.md:

Regression tests for specific issues go in /test/regression/issue/${issueNumber}.test.ts. If there's no issue number, do not put them in the regression directory.

This should either be renamed to include a real issue number (e.g., test/regression/issue/12345.test.ts) or moved to a more appropriate location like test/js/bun/.

import { existsSync, mkdirSync, realpathSync, symlinkSync } from "fs";
import { bunEnv, bunExe, tempDir, tls } from "harness";
import { dirname, join } from "path";

// Symlink chain path traversal vulnerability in archive extraction (libarchive.zig).
//
// The attack uses a 3-level chain of symlinks to escape extraction bounds:
// 1. `x/a` -> `.` creates a self-referencing symlink (resolves to x/ itself)
// 2. `x/a/b` -> `.` through the first symlink creates x/b -> . (also self-referencing)
// 3. `x/a/b/c` -> `../..` through both symlinks creates x/c -> ../..
// which resolves from x/ to two levels up = parent of extraction dir
//
// isSymlinkTargetSafe passes all three because it only checks logical path
// resolution without considering previously-created symlinks on the filesystem.

describe("symlink chain path traversal", () => {
test("chained symlinks in tarball should not escape extraction directory via bun create", async () => {
using dir = tempDir("symlink-chain-test", {});
const baseDir = String(dir);
const tgzPath = join(baseDir, "malicious.tgz");

// Create a malicious tarball with 3-level symlink chain.
// root/ prefix gets stripped by depth_to_skip=1 (matching bun create behavior).
const pyResult = Bun.spawnSync({
cmd: [
"python3",
"-c",
`
import tarfile, io

with tarfile.open(${JSON.stringify(tgzPath)}, 'w:gz') as tar:
for name, typ, extra in [
('root/', tarfile.DIRTYPE, {}),
('root/x/', tarfile.DIRTYPE, {}),
('root/x/a', tarfile.SYMTYPE, {'linkname': '.'}),
('root/x/a/b', tarfile.SYMTYPE, {'linkname': '.'}),
('root/x/a/b/c', tarfile.SYMTYPE, {'linkname': '../..'}),
]:
info = tarfile.TarInfo(name=name)
info.type = typ
info.mode = 0o755
for k, v in extra.items():
setattr(info, k, v)
tar.addfile(info)

content = b'PWNED_CANARY'
info = tarfile.TarInfo(name='root/x/c/evil.txt')
info.type = tarfile.REGTYPE
info.size = len(content)
info.mode = 0o644
tar.addfile(info, io.BytesIO(content))
`,
],
env: bunEnv,
});
expect(pyResult.exitCode).toBe(0);
expect(existsSync(tgzPath)).toBe(true);

// Serve the tarball over HTTPS, mimicking the GitHub API tarball endpoint.
await using server = Bun.serve({
port: 0,
tls,
fetch(req) {
if (new URL(req.url).pathname === "/repos/test/malicious/tarball") {
return new Response(Bun.file(tgzPath), {
headers: { "content-type": "application/x-gzip" },
});
}
return new Response("not found", { status: 404 });
},
});

const createDir = join(baseDir, "created-project");

await using proc = Bun.spawn({
cmd: [bunExe(), "create", "test/malicious", createDir],
env: {
...bunEnv,
GITHUB_API_DOMAIN: `localhost:${server.port}`,
NODE_TLS_REJECT_UNAUTHORIZED: "0",
},
stderr: "pipe",
stdout: "pipe",
});

const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);

// x/c -> ../.. resolves from createDir/x/ up two levels to dirname(createDir)
const evilOutside = join(dirname(createDir), "evil.txt");
const fileEscaped = existsSync(evilOutside);

if (fileEscaped) {
console.log("VULNERABILITY: evil.txt written outside extraction directory at", evilOutside);
Bun.spawnSync({ cmd: ["rm", "-f", evilOutside], env: bunEnv });
}

// Security requirement: no file should exist outside the extraction directory
expect(fileEscaped).toBe(false);

// Verify: if x/c symlink exists, it should not resolve outside createDir
const escapeLink = join(createDir, "x", "c");
if (existsSync(escapeLink)) {
try {
const resolved = realpathSync(escapeLink);
expect(resolved.startsWith(createDir)).toBe(true);
} catch {
// Broken symlink is OK - the escape was prevented
}
}
});

test("concept validation: 3-level symlink chain escapes extraction directory", () => {
// Validates the attack concept independently of Bun's extraction.
using dir = tempDir("symlink-concept-test", {});
const baseDir = String(dir);
const extractDir = join(baseDir, "extract");
mkdirSync(join(extractDir, "x"), { recursive: true });

// Chain: x/a -> . , x/a/b -> . , x/a/b/c -> ../..
symlinkSync(".", join(extractDir, "x", "a"));
symlinkSync(".", join(extractDir, "x", "a", "b"));
symlinkSync("../..", join(extractDir, "x", "a", "b", "c"));

// x/c -> ../.. from x/ goes two levels up: extract/ -> baseDir/
const escapeResolved = realpathSync(join(extractDir, "x", "c"));
expect(escapeResolved).toBe(baseDir);
expect(escapeResolved.startsWith(extractDir)).toBe(false);
});
});