fix(tools): normalize .. in resolve_via_ancestors to prevent sandbox bypass#2130
Merged
fix(tools): normalize .. in resolve_via_ancestors to prevent sandbox bypass#2130
Conversation
…bypass resolve_via_ancestors silently dropped `..` components when building the path suffix (file_name() returns None for ParentDir), allowing a crafted path like /sandbox/nonexistent/../../etc/passwd to pass the starts_with check while the OS would resolve it outside the sandbox. Add normalize_path() for purely lexical collapse of .. and . components before the ancestor walk. The normalization happens in validate_path before calling resolve_via_ancestors, covering all 9 file tool handlers. Closes #2125.
github-actions
bot
added
documentation
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.
Summary
Closes #2125.
resolve_via_ancestorssilently dropped..path components when building the non-existing suffix:Path::file_name()returnsNoneforParentDir, so the walk advancedexistingpast sandbox boundaries without recording the..in the suffix. The finalbase.join(suffix)then produced a path that looked inside the sandbox tostarts_with, while the OS would resolve it outside.Attack vector:
→
starts_with("/tmp/sandbox")returnedtrue, OS opened/etc/passwd.Fix
Add
normalize_path()— a purely lexical stack-based../.collapse with no filesystem calls — and call it invalidate_pathbeforeresolve_via_ancestors. All 9 file tool handlers are covered via the singlevalidate_pathentry point.Tests added (9)
normalize_path_normal_path— no-op on clean pathnormalize_path_collapses_dot—.eliminatednormalize_path_collapses_dotdot— multi-component..collapsednormalize_path_nested_dotdot— multi-level traversalnormalize_path_at_sandbox_boundary— exact boundary preservedvalidate_path_dotdot_bypass_nonexistent_blocked— primary CVE scenario blockedvalidate_path_dotdot_nested_bypass_blocked— deeper escape blockedvalidate_path_inside_sandbox_passes— normal access still allowedvalidate_path_dot_components_inside_sandbox_passes—.inside sandbox allowedChecks
cargo +nightly fmt --check— PASScargo clippy --workspace --features full -- -D warnings— PASS (0 warnings)cargo nextest run --workspace --features full --lib --bins— 6384 passed, 0 failedRelated
allowed_paths(separate fix, merged)