Skip to content

Conversation

iBotPeaches
Copy link
Owner

@iBotPeaches iBotPeaches commented May 3, 2025

ApkEditor made some improvements which broke Apktool, but kept working on AOSP. Things I saw:

  • ApkEditor will make a StringPool chunk and then mislabel the type as something else. So if you are reading the chunk and parse via the type - it'll crash out. This is sneaky because AOSP does not parse any chunks except the one it cares about. So in the situation where AOSP parses AndroidManifest.xml it parses ahead (reading header, jumping to end of chunk) until it finds a few elements and in this case START_ELEMENT. So when ApkEditor packs bogus chunks of XML_END_NAMESPACE and PACKAGE Apktool may crash out as it tries to parse those as-is in the middle of an XML node and die.
    private void placeBadChunk(AndroidManifestBlock manifestBlock) {
        placeBadChunk(manifestBlock, ChunkType.XML_END_NAMESPACE);
        placeBadChunk(manifestBlock, ChunkType.PACKAGE);
    }

REAndroid/APKEditor@4e7ce00

So Apktool made a few changes:

  1. Aligned towards the jumpTo method instead of having skip lines around. Even though it does the same thing under the hood - its a bit easier to read/understand. This is helpful because ApkEditor injects a StringPool then marks it as a Package type. So our older code would have attempted to parse the Package type (even if it was going to skip it). So now it just skips a chunk that it does not care about without parsing more than the arsc header.
  2. Removed the hard-coded assumption of header size, which may be different when bogus chunks are injected into the XML area of the manifest.
  3. Detects if a END_NAMESPACE event is obtained prior to START_ELEMENT. If so - it just skips that chunk.

This isn't perfect yet as END_NAMESPACE bogus element could just become any other and Apktool would break again. However, the 15 year old XML node reading code isn't as elegant at the moment to transition into a while() loop of parsing chunks with a preference to skip unknown chunks.

fixes: #3838

@iBotPeaches iBotPeaches merged commit bb8011c into master May 4, 2025
25 checks passed
@iBotPeaches iBotPeaches deleted the issue-3838 branch May 4, 2025 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] ApkEditor Protect: decode error Unknown chunk type
1 participant