fix(services): confine compfs/monoiofs object keys to root by rejecting parent-dir traversal#7702
Open
tonghuaroot wants to merge 1 commit into
Open
Conversation
…ng parent-dir traversal
59442fc to
a86bab4
Compare
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?
This is a direct continuation of #7684 (
fsroot confinement, merged). As with that PR, it was discussed with the PMC on the OpenDAL security list rather than via a public issue; happy to open one if preferred.Rationale for this change
#7684 added
..-traversal confinement to thefsbackend:normalize_pathin opendal-core strips leading/and empty segments but, per RFC 0112, intentionally does not resolve./.., andPathBuf::joinis purely lexical, so a key such as../../etc/passwdis joined ontorootas-is and resolved by the host kernel at syscall time, escaping therootthe backend documents as the boundary for "all operations".The same defect exists, unchanged, in the two other local-filesystem backends that #7684 did not touch:
compfs(compio-backed local FS)monoiofs(monoio / io_uring local FS)Both build every on-disk path via
self.root.join(key.trim_end_matches('/'))(inprepare_path/prepare_write_path) with no..reject. Because resolution happens against the host kernel on a local filesystem, the threat model is identical tofs: a key containing..escapes the configuredroot. This is the same class of issue #7684 fixed, on the sibling local-FS backends.This is defense-in-depth confinement, not a redesign of path normalization. RFC 0112 deliberately defers
./..handling; this PR leavesnormalize_pathuntouched and only adds a..reject on the local-FS join path itself, mirroring #7684. The remote path backends (webdav,sftp,hdfs, …) are intentionally out of scope here: their paths are resolved server-side by the remote daemon, which is a different (operator/server-trust) boundary.What changes are included in this PR?
compfs:CompfsCore::prepare_pathnow rejects any key whose path components include a parent-dir (..) traversal, returningErrorKind::NotFoundwith the offending path in context; otherwise it joins as before (trailing-/trim preserved). Every key-join site already routed throughprepare_path(create_dir,stat,copy,rename,read,write,list, and the deleter), so each call now propagates the result with?.monoiofs: same confinement added toMonoiofsCore::prepare_path;prepare_write_path(which callsprepare_path) and every backend/deleter call site propagate with?.A single
.(CurDir) is unaffected, and well-behaved keys resolve to exactly the same path as before, so there is no change on the normal path. Regression tests in both crates cover..keys being rejected and normal/./trailing-slash/a..b-substring keys resolving unchanged.Are there any user-facing changes?
No public API change. The only behavioral change is that a key containing
..is now rejected withNotFoundinstead of being resolved outsideroot. Keys without..(including absolute-looking inputs, which are unchanged here) behave exactly as before.AI Usage Statement
Drafted with assistance from an AI coding tool (Claude). All changes were reviewed by me. Both crates were verified locally:
cargo fmt -- --check,cargo clippy --all-targets -- -D warnings, andcargo testforopendal-service-compfsandopendal-service-monoiofsall pass, including the new negative/positive confinement regression tests (..keys rejected; non-..keys,., trailing-slash, anda..b-substring keys resolve unchanged).