Skip to content

fix(android): skip symlink entries during extraction (#357)#361

Merged
plrthink merged 1 commit into
masterfrom
fix/android-symlink-escape
Jul 22, 2026
Merged

fix(android): skip symlink entries during extraction (#357)#361
plrthink merged 1 commit into
masterfrom
fix/android-symlink-escape

Conversation

@plrthink

@plrthink plrthink commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Fixes #357.

Problem

zip4j 2.11.5 defaults UnzipParameters.extractSymbolicLinks to true and does not validate that a symlink's resolved target stays inside the destination directory. unzip/unzipWithPassword called extractFile without parameters, so a crafted archive entry like link_to_outside -> ../outside_target.txt would materialize a symlink escaping the extraction root. (The v9 ZipSecurity Zip Slip guard covers entry names, not symlink targets.)

Fix

  • ZipSecurity.createExtractParameters() returns UnzipParameters with setExtractSymbolicLinks(false); both extraction call sites use it. With symlinks disabled, zip4j skips symlink entries entirely (verified against the 2.11.5 bytecode), so nothing can escape the destination.
  • unzipAssets is unaffected by construction (manual stream copy, never creates symlinks).

Tests

  • extractWithZipSecurityParams_doesNotCreateSymlink: regression test using an embedded base64 fixture zip containing link_to_outside -> ../outside_target.txt; asserts nothing is materialized and nothing appears outside the destination.
  • extractWithDefaultParams_createsEscapingSymlink: control test proving the fixture exercises the vulnerable zip4j default path.
  • createExtractParameters_disablesSymlinkExtraction: sanity check on the params themselves.

./gradlew :react-native-zip-archive:testDebugUnitTest — 11/11 pass. npx jest — 27/27 pass.

Thanks @kimdu0 for the thorough report and repro.


Open in Devin Review

zip4j defaults UnzipParameters.extractSymbolicLinks to true and does not
validate that a symlink's resolved target stays inside the destination
directory, so a crafted archive could plant a symlink escaping the
extraction root. unzip/unzipWithPassword now extract with symlinks
disabled (zip4j skips such entries entirely); unzipAssets is unaffected
by construction. Adds a regression test with an embedded symlink-entry
fixture plus a control test proving the fixture exercises the vulnerable
default path.
@cursor

cursor Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Open in Devin Review


if (!fileHeader.isDirectory()) {
zipFile.extractFile(fileHeader, destDirectory);
zipFile.extractFile(fileHeader, destDirectory, ZipSecurity.createExtractParameters());

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 iOS symlink-target handling not addressed by this Android-only fix

REVIEW.md flags path-traversal / Zip Slip changes for extra scrutiny across both platforms. This PR only hardens the Android extraction path against symlink escape; ios/RNZipArchive.mm is untouched. Since the PR does not modify the codegen spec, this is not a spec-parity violation, and the symlink-target escape may or may not apply to the iOS unzip backend. Worth confirming whether the iOS implementation has an equivalent exposure so the fix isn't left half-applied.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@plrthink
plrthink merged commit 4848930 into master Jul 22, 2026
9 checks passed
@plrthink
plrthink deleted the fix/android-symlink-escape branch July 22, 2026 00:36
@plrthink

Copy link
Copy Markdown
Collaborator Author

Re: Devin's review comment on iOS symlink-target handling — checked this, and iOS is already protected: our unzipFile: calls SSZipArchive's unzipFileAtPath:toDestination:preserveAttributes:overwrite:nestedZipLevel:... overload, which passes symlinksValidWithin:destination. SSZipArchive then rejects any symlink whose resolved target escapes the destination (_escapesTargetDirectory: check in SSZipArchive.m). So the fix is not half-applied — the iOS backend has the equivalent containment guard built in, and this PR correctly closes the gap on the Android/zip4j side, which had no such check.

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.

React Native Zip Archive Android zip4j symlink target escape

1 participant