Fix isChildOf prefix match to require a path separator boundary#401
Open
samjanny wants to merge 1 commit into
Open
Fix isChildOf prefix match to require a path separator boundary#401samjanny wants to merge 1 commit into
samjanny wants to merge 1 commit into
Conversation
isChildOf compared parent against child with a bare string-prefix
check, so isChildOf("/foobar", "/foo") wrongly returned true. The
result is used by VolumeProvider.isChildDocument, which the SAF
framework calls to confirm a documentId belongs to a granted subtree.
A false positive let an app holding a grant on /foo reach sibling
paths like /foobar.
Require a separator boundary: treat childPath as a descendant only
when it equals parentPath plus a '/' segment, handling the root and
trailing-slash cases. Also exclude equal paths, so a path is no
longer reported as a child of itself.
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
Fixes a path-prefix confusion in
PathUtils.isChildOf.isChildOfdetermined the parent/child relationship with a barestring-prefix comparison, without requiring a separator boundary:
The value feeds
VolumeProvider.isChildDocument(
VolumeProvider.kt:119), which the Storage Access Framework calls toverify that a
documentIdbelongs to a granted tree. With the unsafeusf_exposefeature enabled and a SAF grant on a non-root subfolder,an app could craft sibling documentIds (e.g.
/foobar,/foo.txt)and have them accepted as in-tree, reaching files never granted.
Fix
Require a separator boundary: a path is a child only when it equals
the parent plus a
/-delimited segment. Root (/) and trailing-slashparents are handled, and equal paths are no longer treated as a child
of themselves.
Impact on existing callers
The only other caller, the copy/move loop in
BaseExplorerActivity,operates on genuine descendants and is unaffected; removing the
self-as-child case avoids a redundant remap.
Testing
Verified the new logic against 12 edge cases with the Kotlin compiler
(original bug, genuine children, root parent, equal paths, trailing
slash). DroidFS has no automated test suite and a full Android build
(SDK + NDK r25c) was not run; the change is pure string-comparison
logic with no native dependencies.