Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent arbitrary file writes with malicious resource names. #3484

Merged
merged 15 commits into from
Jan 2, 2024

Conversation

iBotPeaches
Copy link
Owner

@iBotPeaches
Copy link
Owner Author

I clearly suck at understanding Windows pathing and cross-platform. Will revisit after some learning/research

@0x33c0unt
Copy link

I still think that we can stick to extracting to the same path inside of the apk/zip itself. Basically unzipping but decoding some types on-the-fly.

@iBotPeaches
Copy link
Owner Author

The challenge I suffer with here is that Path/File want to detect the path separator from the host OS. I want to specify one because as you mentioned these are paths correlating to Zip/Apk and then transitioned to host system. So for now - a basic check for traversal, which should suffice.

@0x33c0unt
Copy link

Looks nice. Great effort!

@iBotPeaches iBotPeaches merged commit d348c43 into master Jan 2, 2024
25 checks passed
@iBotPeaches iBotPeaches deleted the advisory-fix-1 branch January 2, 2024 11:11
@iBotPeaches iBotPeaches added this to the v2.10.0 milestone Jan 2, 2024
iBotPeaches added a commit that referenced this pull request Jan 2, 2024
@ajinabraham
Copy link

@iBotPeaches Can you cut a new release for apktool with this patch? I see that there is 2.10.0 scoped for April. Since this is a security fix, can we get a fixed version in the interim?

@iBotPeaches
Copy link
Owner Author

@iBotPeaches Can you cut a new release for apktool with this patch? I see that there is 2.10.0 scoped for April. Since this is a security fix, can we get a fixed version in the interim?

I was planning a patch release this weekend.

iBotPeaches added a commit that referenced this pull request Jan 5, 2024
* refactor: rename sanitize function

* fix: expose getDir

* fix: safe handling of untrusted resource names

 - fixes: GHSA-2hqv-2xv4-5h5w

* test: sample file for GHSA-2hqv-2xv4-5h5w

* refactor: avoid detection of absolute files for resource check

* chore: enable info mode on gradle

* test: skip test on windows

* chore: debug windows handling

* fix: normalize entry with file separators

* fix: normalize filepath after cleansing

* chore: Android paths are not OS specific

* refactor: use java.nio for path traversal checking

* chore: align path separator on Windows for Zip files

* chore: rework towards basic directory traversal

* chore: remove '--info' on build.yml
@iBotPeaches iBotPeaches removed this from the v2.10.0 milestone Jan 6, 2024
@iBotPeaches
Copy link
Owner Author

@iBotPeaches Can you cut a new release for apktool with this patch? I see that there is 2.10.0 scoped for April. Since this is a security fix, can we get a fixed version in the interim?

Now its out - v2.9.2 - https://apktool.org/blog/apktool-2.9.2

iBotPeaches added a commit that referenced this pull request Jan 10, 2024
* refactor: rename sanitize function

* fix: expose getDir

* fix: safe handling of untrusted resource names

 - fixes: GHSA-2hqv-2xv4-5h5w

* test: sample file for GHSA-2hqv-2xv4-5h5w

* refactor: avoid detection of absolute files for resource check

* chore: enable info mode on gradle

* test: skip test on windows

* chore: debug windows handling

* fix: normalize entry with file separators

* fix: normalize filepath after cleansing

* chore: Android paths are not OS specific

* refactor: use java.nio for path traversal checking

* chore: align path separator on Windows for Zip files

* chore: rework towards basic directory traversal

* chore: remove '--info' on build.yml
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.

None yet

4 participants