-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[BUG] mResTable poisoned if framework is sparse #3298
Comments
Odd. I'm fairly accurate that how it works now is correct and it was not in the past. I'm going exactly by the spec when it comes to determining whether it was a sparsely packed or not.
So honestly not sure at the moment. |
Odd again. Taking a look. |
One assertion that can be made is that AAPT2 does correctly not decode resources sparely, if not instructed to do that, because the app does not produce the stack dump in that case. |
I did update the aapt2/aapt binaries as part of 2.8.0. Its very very rare that any change to upstream AOSP build tools causes a regression though - 8634050 |
Yes, AAPT2 is working fine is what I mean as we can see that not using the option to encode resources sparsely fixes the stack dump. What is odd, though, is that sparsely encoded resources are detected on an APK whose resources are not encoded sparsely by AAPT2 as seen in #3199 and now. |
@oSumAtrIX - Are you positive you are running a bleeding edge build and/or grabbing upstream patches correctly? I decoded the application you gave with the official 2.7.0 and the top of master.
I placed breakpoints, etc. Never replicated what you are saying. Both are indeed as I expect with |
2.7.0 is a fairly old build. I am running APKTool off commit 06c5f46. This is currently versioned as I will follow up with a complete recording of the issue for reference in a minute. |
Here is a followup recording (trimmed when nothing happens): evidence.mp4The recording shows, that APKTool detects resources of an APK as sparsely encoded that has been compiled by AAPT2 without sparsely encoding enabled. |
Okay thanks. To be honest, I'm unaware why I cannot replicate still. So my guess if this is occurring then I bet I have a bug where we aren't isolating the framework decode from the application decode. So a sparse framework may be leaking into main. |
Yeah that was it. I have a patch pending, but I gotta take it a bit slow and figure out a more proper solution to prevent this cross contamination/poisoning. |
Interesting that it took years to find this. It was never noticeable previously because we accidentally blew out the config for each package. Frameworks come first, so this was always last with the intended application which meant it seemed it worked. However, it was always additionally wrong since it was assuming sparse resources whether the spec order was different than intended. This is a good indication of a sparsely package table, but the real indicator is just whether the sparse flag is true or not. The refactors to optimize since 2.7.0 to now adapted that into a more proper sparse check described above, at the cost of not realizing that we were storing all packages parsed into one single info class. This is intended though, because you need to know frameworks, etc from the main app. So you do want one central storage mechanism. So tldr - I need to do some mini refactors and I'll get that PR cleaned up. |
Information
apktool -version
) - 2.8.1 (The issue is still present after 5483650)java --version
) - OpenJDK 17Stacktrace/Logcat
As #3199
Issue
sparseResources
is set tofalse
manually inapktool.yml
, the app runs fine (under required modifications)sparseResources
tofalse
manually fixes the issue - the stacktrace above does not occurevidence.mp4
APK
https://www.apkmirror.com/apk/google-inc/youtube/youtube-18-32-39-release/
Steps to Reproduce
Decode & build
apktool d youtube-18.32.39.apk -s
(Notice how sparse resources are true)apktool b youtube-18.32.39
Questions to ask before submission
apktool d
,apktool b
without changing anything? YesThe text was updated successfully, but these errors were encountered: