Skip to content

Harden OS.rmdir() against symlink traversal during forced decode - #4184

Draft
iBotPeaches with Copilot wants to merge 2 commits into
mainfrom
copilot/harden-deletion-of-symlinks
Draft

Harden OS.rmdir() against symlink traversal during forced decode#4184
iBotPeaches with Copilot wants to merge 2 commits into
mainfrom
copilot/harden-deletion-of-symlinks

Conversation

Copilot AI commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

OS.rmdir() used File.isDirectory() which follows symlinks, causing apktool d --force to recurse into symlink targets and delete files outside the intended output directory — a path traversal via symlink vulnerability.

Changes

  • brut.j.util/OS.java: Added Files.isSymbolicLink() guards (NIO, does not follow symlinks) in two places within rmdir():
    • Top-level: if the argument itself is a symlink, delete only the symlink and return
    • Recursion loop: if a child entry is a symlink (even to a directory), delete only the symlink — never recurse into it
// Before: File.isDirectory() follows symlinks
if (file.isDirectory()) {
    rmdir(file);  // recurses into symlink target!
}

// After: symlinks are treated as opaque files regardless of target type
if (!Files.isSymbolicLink(file.toPath()) && file.isDirectory()) {
    rmdir(file);
} else {
    rmfile(file);  // deletes the symlink itself
}
  • brut.j.util/build.gradle.kts: Added testImplementation(libs.junit) to enable unit tests in this module
  • brut.j.util/OSTest.java: Two new unit tests covering the symlink cases — symlink as the root argument and symlink nested inside a directory being deleted

Copilot AI linked an issue Jul 22, 2026 that may be closed by this pull request
Copilot AI changed the title [WIP] Harden deletion of symlinks during build Harden OS.rmdir() against symlink traversal during forced decode Jul 22, 2026
Copilot AI requested a review from iBotPeaches July 22, 2026 15:49
Previously, OS.rmdir() would treat symlinks to directories as real
directories and recurse into them, potentially deleting files outside
the intended path. This is a security vulnerability when using
`apktool d --force` on a symlinked output directory.

Fix: Use Files.isSymbolicLink() to detect symlinks before recursing.
Symlinks are now deleted directly (like regular files) without following
them into their targets.

Closes #4183
@iBotPeaches
iBotPeaches force-pushed the copilot/harden-deletion-of-symlinks branch from aad85ab to 3cebe29 Compare July 24, 2026 10:43
@IgorEisberg

Copy link
Copy Markdown
Collaborator

@copilot This is a very elementary change, there's no need for a whole test for that in the brut.j.util module that otherwise has no tests. Otherwise the change is fine as long as the user doesn't have any intentional/non-malicious use of symlinks in his project.

@iBotPeaches

Copy link
Copy Markdown
Owner

@copilot This is a very elementary change, there's no need for a whole test for that in the brut.j.util module that otherwise has no tests. Otherwise the change is fine as long as the user doesn't have any intentional/non-malicious use of symlinks in his project.

Per reviewer feedback, the OS.rmdir() symlink fix is elementary enough
that dedicated tests in this otherwise-test-free module are unnecessary.

Copilot AI commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

@copilot This is a very elementary change, there's no need for a whole test for that in the brut.j.util module that otherwise has no tests. Otherwise the change is fine as long as the user doesn't h...

Removed the test file and the testImplementation dependency from build.gradle.kts in commit Remove test file and test dependency from brut.j.util.

@copilot This is a very elementary change, there's no need for a whole test for that in the brut.j.util module that otherwise has no tests. Otherwise the change is fine as long as the user doesn't hav...

Removed the test file and the testImplementation dependency from build.gradle.kts in commit Remove test file and test dependency from brut.j.util.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Harden deletion of symlinks during build

3 participants