Repository navigation
fix: refuse object keys that resolve outside the local destination (#872) - #886
Open
terryrankine wants to merge 1 commit into
Open
terryrankine wants to merge 1 commit into
terryrankine wants to merge 1 commit into
Conversation
S3 object keys are arbitrary strings and may contain ".." segments. cp and sync built the local destination from the raw key with path.Join, which resolves ".." and lands outside the chosen directory: downloading "s3://bucket/*" where an attacker wrote one object with key "data/../../../.bashrc" overwrites the caller's shell profile. Reported upstream as peak#872. Add URL.JoinInside, which joins and then checks the cleaned result is still under the destination, and use it at every local sink: batch downloads in prepareLocalDestination, the trailing-slash and directory cases in prepareDownloadDestination, and sync's generateDestinationURL. Offending objects are reported as errors and skipped; the rest of the run continues and the exit code is non-zero. Remote destinations are unchanged (keys are opaque there). --flatten and single-object copies use Base() and were never affected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Et7MCFVnqJFF8NLkA1VJrP
This was referenced Sep 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Fixes #872. S3 object keys are arbitrary strings and may contain
..segments.cpandsyncbuilt the local destination from the raw key withpath.Join, which resolves..and lands outside the chosen directory: downloadings3://bucket/*where an attacker wrote one object with keydata/../../../.bashrcoverwrites the caller's shell profile. Confirmed onmasterwith a crafted key: bothcpandsyncwrote the file two levels above the destination.Fix
URL.JoinInsidejoins and then checks the cleaned result is still under the destination. It is used at every local sink: batch downloads inprepareLocalDestination, the trailing-slash and directory cases inprepareDownloadDestination, andsync'sgenerateDestinationURL. An offending object is reported as an error and skipped; the rest of the run continues and the exit code is non-zero. Remote destinations are unchanged (keys are opaque there).--flattenand single-object copies useBase()and were never affected.Tests
storage/url:TestJoinInside(11 cases: inside,a/../b,..,../x,.base,/base, remote pass-through).e2e:TestCopyS3ObjectsToLocalWithPathTraversalKeyandTestSyncS3ObjectsToLocalWithPathTraversalKey— adata/../../escape.txtkey alongside a safe one: exit 1,escapes destinationon stderr, safe object copied, nothing outside the destination. Both fail onmaster(they writeescape.txtoutside the directory).Linux, Go 1.24,
go test -count=1 -race ./...: all packages ok.gofmt,go vet,staticcheck,unparam: clean.🤖 Generated with Claude Code
https://claude.ai/code/session_01Et7MCFVnqJFF8NLkA1VJrP