fix: support skipping unread header sizes of ResChunk (#3180)

* fix: support skipping unread header sizes of ResChunk

* refactor: note that header skip happens too late on some

* refactor: check for chunk header end at each of each header

* chore: skip reading header on string pools

* fix: move header check prior to reading entries on tables

brut.apktool/apktool-lib/src/main/java/brut/androlib/res/data/arsc/ARSCHeader.java

@@ -21,6 +21,8 @@ import org.apache.commons.io.input.CountingInputStream;
 
 import java.io.EOFException;
 import java.io.IOException;
+import java.math.BigInteger;
+import java.util.logging.Logger;
 
 public class ARSCHeader {
     public final short type;
@@ -48,6 +50,30 @@ public class ARSCHeader {
         return new ARSCHeader(type, in.readShort(), in.readInt(), start);
     }
 
+    public void checkForUnreadHeader(ExtDataInput in, CountingInputStream countIn) throws IOException {
+        // Some applications lie about the reported size of their chunk header. Trusting the chunkSize is misleading
+        // So compare to what we actually read in the header vs reported and skip the rest.
+        // However, this runs after each chunk and not every chunk reading has a specific distinction between the
+        // header and the body.
+        int actualHeaderSize = countIn.getCount() - this.startPosition;
+        int exceedingSize = this.headerSize - actualHeaderSize;
+        if (exceedingSize > 0) {
+            byte[] buf = new byte[exceedingSize];
+            in.readFully(buf);
+            BigInteger exceedingBI = new BigInteger(1, buf);
+
+            if (exceedingBI.equals(BigInteger.ZERO)) {
+                LOGGER.fine(String.format("Chunk header size (%d), read (%d), but exceeding bytes are all zero.",
+                    this.headerSize, actualHeaderSize
+                ));
+            } else {
+                LOGGER.warning(String.format("Chunk header size (%d), read (%d). Exceeding bytes: 0x%X.",
+                    this.headerSize, actualHeaderSize, exceedingBI
+                ));
+            }
+        }
+    }
+
     public void skipChunk(ExtDataInput in) throws IOException {
         in.skipBytes(chunkSize - headerSize);
     }
@@ -76,4 +102,6 @@ public class ARSCHeader {
     public final static short RES_XML_CDATA_TYPE = 0x0104;
     public final static short RES_XML_LAST_CHUNK_TYPE = 0x017f;
     public final static short RES_XML_RESOURCE_MAP_TYPE = 0x0180;
+
+    private static final Logger LOGGER = Logger.getLogger(ARSCHeader.class.getName());
 }

brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/ARSCDecoder.java

@@ -137,11 +137,15 @@ public class ARSCDecoder {
     private void readTableChunk() throws IOException, AndrolibException {
         checkChunkType(ARSCHeader.RES_TABLE_TYPE);
         mIn.skipInt(); // packageCount
+
+        mHeader.checkForUnreadHeader(mIn, mCountIn);
     }
 
     private void readUnknownChunk() throws IOException, AndrolibException {
         checkChunkType(ARSCHeader.RES_NULL_TYPE);
 
+        mHeader.checkForUnreadHeader(mIn, mCountIn);
+
         LOGGER.warning("Skipping unknown chunk data of size " + mHeader.chunkSize);
         mHeader.skipChunk(mIn);
     }
@@ -178,6 +182,8 @@ public class ARSCDecoder {
             LOGGER.warning("Please report this application to Apktool for a fix: https://github.com/iBotPeaches/Apktool/issues/1728");
         }
 
+        mHeader.checkForUnreadHeader(mIn, mCountIn);
+
         mTypeNames = StringBlock.readWithChunk(mIn);
         mSpecNames = StringBlock.readWithChunk(mIn);
 
@@ -194,6 +200,8 @@ public class ARSCDecoder {
         int packageId;
         String packageName;
 
+        mHeader.checkForUnreadHeader(mIn, mCountIn);
+
         for (int i = 0; i < libraryCount; i++) {
             packageId = mIn.readInt();
             packageName = mIn.readNullEndedString(128, true);
@@ -204,6 +212,8 @@ public class ARSCDecoder {
     private void readStagedAliasSpec() throws IOException {
         int count = mIn.readInt();
 
+        mHeader.checkForUnreadHeader(mIn, mCountIn);
+
         for (int i = 0; i < count; i++) {
             LOGGER.fine(String.format("Skipping staged alias stagedId (%h) finalId: %h", mIn.readInt(), mIn.readInt()));
         }
@@ -213,6 +223,9 @@ public class ARSCDecoder {
         checkChunkType(ARSCHeader.XML_TYPE_OVERLAY);
         String name = mIn.readNullEndedString(256, true);
         String actor = mIn.readNullEndedString(256, true);
+
+        mHeader.checkForUnreadHeader(mIn, mCountIn);
+
         LOGGER.fine(String.format("Overlay name: \"%s\", actor: \"%s\")", name, actor));
     }
 
@@ -221,6 +234,8 @@ public class ARSCDecoder {
         mIn.skipInt(); // policyFlags
         int count = mIn.readInt();
 
+        mHeader.checkForUnreadHeader(mIn, mCountIn);
+
         for (int i = 0; i < count; i++) {
             LOGGER.fine(String.format("Skipping overlay (%h)", mIn.readInt()));
         }
@@ -237,6 +252,8 @@ public class ARSCDecoder {
             mFlagsOffsets.add(new FlagsOffset(mCountIn.getCount(), entryCount));
         }
 
+        mHeader.checkForUnreadHeader(mIn, mCountIn);
+
         mIn.skipBytes(entryCount * 4); // flags
         mTypeSpec = new ResTypeSpec(mTypeNames.getString(id - 1), mResTable, mPkg, id, entryCount);
         mPkg.addType(mTypeSpec);
@@ -255,18 +272,12 @@ public class ARSCDecoder {
         int typeFlags = mIn.readByte();
         mIn.skipBytes(2); // reserved
         int entryCount = mIn.readInt();
-        int entriesStart = mIn.readInt();
+        mIn.skipInt(); // entriesStart
+
         mMissingResSpecMap = new LinkedHashMap<>();
-
         ResConfigFlags flags = readConfigFlags();
-        int position = (mHeader.startPosition + entriesStart) - (entryCount * 4);
 
-        // For some APKs there is a disconnect between the reported size of Configs
+        mHeader.checkForUnreadHeader(mIn, mCountIn);
-        // If we find a mismatch skip those bytes.
-        if (position != mCountIn.getCount()) {
-            LOGGER.warning("Invalid data detected. Skipping: " + (position - mCountIn.getCount()) + " byte(s)");
-            mIn.skipBytes(position - mCountIn.getCount());
-        }
 
         if ((typeFlags & 0x01) != 0) {
             LOGGER.fine("Sparse type flags detected: " + mTypeSpec.getName());