From 40200931fdae384c2a845dd5d34382c0b3a0c8ab Mon Sep 17 00:00:00 2001 From: Claude Bot Date: Thu, 12 Feb 2026 04:57:49 +0000 Subject: [PATCH] fix(security): prevent symlink chain path traversal in archive extraction Chained symlinks in a tarball could escape the extraction directory. The existing isSymlinkTargetSafe() validates symlink targets against logical archive paths but doesn't consider the actual filesystem state created by previously extracted entries. A chain of individually-safe symlinks (e.g., x/a -> ., x/a/b -> ., x/a/b/c -> ../..) can combine to resolve outside the extraction boundary. Add isResolvedPathSafe() which uses realpath to verify the actual filesystem resolution stays within the extraction directory. Apply this check after creating symlinks (removing unsafe ones) and before creating files or directories (skipping entries whose parent escapes). Co-Authored-By: Claude --- src/libarchive/libarchive.zig | 85 ++++++++++++ .../issue/symlink-chain-traversal.test.ts | 130 ++++++++++++++++++ 2 files changed, 215 insertions(+) create mode 100644 test/regression/issue/symlink-chain-traversal.test.ts diff --git a/src/libarchive/libarchive.zig b/src/libarchive/libarchive.zig index 72bc5d5b964f..bdf5ef5c2186 100644 --- a/src/libarchive/libarchive.zig +++ b/src/libarchive/libarchive.zig @@ -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, @@ -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; @@ -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 @@ -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 // diff --git a/test/regression/issue/symlink-chain-traversal.test.ts b/test/regression/issue/symlink-chain-traversal.test.ts new file mode 100644 index 000000000000..23f1036bf27c --- /dev/null +++ b/test/regression/issue/symlink-chain-traversal.test.ts @@ -0,0 +1,130 @@ +import { describe, expect, test } from "bun:test"; +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); + }); +});