fix(services): reject absolute paths in confined_join and prepare_path - #8005
Open
aysha-afrah26 wants to merge 1 commit into
Open
fix(services): reject absolute paths in confined_join and prepare_path#8005aysha-afrah26 wants to merge 1 commit into
aysha-afrah26 wants to merge 1 commit into
Conversation
RFC 7799 specifies rejecting RootDir and Prefix alongside ParentDir so an absolute key cannot replace the root through PathBuf::join. Only the ParentDir arm shipped, so a key that skips normalize_path, such as one passed to Deleter, reached the filesystem outside the configured root.
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
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.
Which issue does this PR close?
None
Rationale for this change
RFC 7799 specifies the local filesystem root check as rejecting
Component::ParentDir,Component::RootDirandComponent::Prefix(_), and gives the reason for the last two: they prevent absolute paths from replacing the root viaPathBuf::join. The shipped guard matches only theParentDirarm, so an absolute key walks straight through it androot.join("/etc/passwd")evaluates to/etc/passwd.Operator::delete_withrunsnormalize_pathfirst, which strips the leading/, so the single-path API is fine.Deleterdoes not:Deleter::deletepasses the caller's key to the service verbatim. With anFsoperator rooted at a temp dir,op.delete_iter(["<absolute path outside root>"])deletes the target, and withrecursiveset it isremove_dir_all.delete_try_iter,delete_stream,delete_try_streamand theSinkimpl all reach the same place. On Windows the exposure is not limited to delete:normalize_pathonly splits on/, soC:/…, a leading\,\\?\…and UNC paths survive it unchanged and are absolute tostd::path, which escapes the root on every operation.What changes are included in this PR?
Adds the two missing component arms to
FsCore::confined_joinand to theprepare_pathcopies incompfsandmonoiofs, which carry the same guard. The error message loses its parent-dir wording and matches the RFC text, since the check now covers more than...Keeping the check inside the join helper means every caller is covered whether or not the path reached it through
normalize_path, which is what theDeletercase turns on.Regression tests go in next to the existing
..ones:test_prepare_path_rejects_absolute_pathforcompfsandmonoiofs,test_confined_join_rejects_escaping_keysforfs, plus an end-to-endfstest that drivesdelete_iterwith an absolute path and asserts the outside file survives. The end-to-end test fails on the current tree.Are there any user-facing changes?
An absolute key passed to
fs,compfsormonoiofsnow returnsErrorKind::NotFoundinstead of resolving outside the configured root. Paths that arrive through the normalOperatormethods are unaffected, sincenormalize_pathhas already stripped the leading/by then."/"still resolves to the root itself.AI Usage Statement