Skip to content

Commit b6b86d3

Browse files
authored
Strengthen path traversal protection in archive extraction (#295)
## Summary Enhance the `safeJoin` function in the self-update archive extraction logic to explicitly reject `".."` path segments, preventing potential path traversal attacks even when path normalization might be bypassed. ## Changes - **Explicit `".."` segment rejection**: Added validation to detect and reject any `".."` path segment in archive entries, including those using backslash separators on Windows - **Normalized path checking**: Convert backslash separators to forward slashes before splitting and validating path segments - **Expanded test coverage**: Added test cases covering: - Bare `".."` entries - Mixed path traversal patterns (`a/../b`, `a/..`) - Backslash-separated traversal attempts (`a\\..\\..\escape`, `..\escape`) ## Implementation Details The fix normalizes the archive entry name by converting all backslashes to forward slashes, then validates each path segment individually. Any segment matching `".."` is rejected with a clear error message. This defense-in-depth approach complements the existing `filepath.Clean` validation and ensures traversal attempts cannot slip through regardless of path separator style or normalization edge cases. https://claude.ai/code/session_01GJyEVeydpmtcNCqKHfhiEu <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Low Risk** > Small, additive hardening in archive path validation with no change to successful extraction paths; lowers zip-slip risk rather than introducing new behavior. > > **Overview** > Hardens **`safeJoin`** in beacon self-update archive extraction by rejecting any path segment equal to **`..`** before the existing `filepath.Clean` containment check. > > Archive entry names are normalized (`\` → `/`), split on `/`, and rejected with a dedicated traversal error if any segment is **`..`**, including Windows-style backslash patterns. **`TestSafeJoinRejectsTraversal`** gains cases for bare **`..`**, mixed **`a/../b`** / **`a/..`**, and backslash traversal strings. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit cbae2c7. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
2 parents 1d5f4c4 + cbae2c7 commit b6b86d3

2 files changed

Lines changed: 14 additions & 2 deletions

File tree

cli/beacon/internal/endpoint/selfupdate/fsutil.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,19 @@ func acquireLock(path string) (func(), error) {
3333
}
3434

3535
// safeJoin resolves an archive entry name against dest and rejects anything that
36-
// would escape dest (path traversal / "zip slip"). It rejects absolute paths and
37-
// any entry whose cleaned, joined path is not contained within dest.
36+
// would escape dest (path traversal / "zip slip"). It rejects absolute paths,
37+
// any ".." path segment (including backslash-separated ones), and any entry
38+
// whose cleaned, joined path is not contained within dest.
3839
func safeJoin(dest, name string) (string, error) {
3940
if filepath.IsAbs(name) || strings.HasPrefix(name, "/") || strings.HasPrefix(name, "\\") {
4041
return "", fmt.Errorf("absolute path in archive entry: %q", name)
4142
}
43+
normalized := strings.ReplaceAll(name, "\\", "/")
44+
for _, seg := range strings.Split(normalized, "/") {
45+
if seg == ".." {
46+
return "", fmt.Errorf("path traversal in archive entry: %q", name)
47+
}
48+
}
4249
cleanDest := filepath.Clean(dest)
4350
target := filepath.Clean(filepath.Join(cleanDest, name))
4451
if target != cleanDest && !strings.HasPrefix(target, cleanDest+string(os.PathSeparator)) {

cli/beacon/internal/endpoint/selfupdate/fsutil_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,14 @@ import (
1212
func TestSafeJoinRejectsTraversal(t *testing.T) {
1313
dest := "/tmp/dest"
1414
bad := []string{
15+
"..",
1516
"../escape",
1617
"../../etc/passwd",
1718
"a/../../escape",
19+
"a/../b",
20+
"a/..",
21+
"a\\..\\..\\escape",
22+
"..\\escape",
1823
"/etc/passwd",
1924
"\\windows\\system32",
2025
}

0 commit comments

Comments
 (0)