Repository navigation
Conversation
…tion 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 <noreply@anthropic.com>
|
🤖 Claude is monitoring this PR I'll review code, respond to comments, and investigate CI failures.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Comment |
Code ReviewNewest first 🟡
|
| @@ -0,0 +1,130 @@ | |||
| import { describe, expect, test } from "bun:test"; | |||
There was a problem hiding this comment.
🟡 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/.
|
No description provided. |
|
Closing this PR because it has been inactive for more than 90 days. |
Summary
libarchive.zigarchive extraction that allows writing arbitrary files outside the extraction directoryisResolvedPathSafe()which usesrealpathto verify symlinks and file paths resolve within the extraction boundary after considering the actual filesystem stateDetails
The existing
isSymlinkTargetSafe()validates symlink targets against logical archive paths using a fake root, but doesn't account for how previously extracted symlinks change filesystem path resolution. A 3-level chain of individually-safe symlinks can escape the extraction directory:x/a→.(self-referencing, resolves tox/)x/a/b→.(through first symlink, createsx/b→.)x/a/b/c→../..(through both symlinks, createsx/c→../..which escapes)Each symlink passes
isSymlinkTargetSafe()individually, but on the actual filesystemx/cresolves outside the extraction directory. Subsequent file entries throughx/c/write outside the intended directory.This affects
bun create(viaextractToDisk) and cross-compilation (viaextractToDir), but NOT npm package extraction which already skips non-file entries.Test plan
bun createwith a crafted tarball confirms the fix prevents file writes outside the extraction directorybun bd)🤖 Generated with Claude Code