Exception while decoding PNG with out of bounds nptc chunk values...

[sebras]

Apktool "crashes" when the nptc chunk in a PNG image resource has xdivs/ydivs which are out of bounds. This is because the underlying BufferedImage.setRGB() throws an exception indicating that the coordinates are out of bounds.

It is the resource res/drawable-nodpi/abs__shadow_holo_new.9.png in the linked apk that is causing problems. Its PNG header dimensions are 10x51 pixels, and the nptc chunk has two xDivs with values 12 and 13, and two yDivs with values 31 and 32. Maybe this is simply the Res9patchStreamDecoder needing to handle out of bounds values . Note that as I read the android sources it really does look like this ought to be drawHLine(im2, 0, xDivs[i], xDivs[i + 1] - 1);, and maybe it influences the bug I'm reporting?

I'm unable to contribute a patch since I don't fully understand when and how Res9patchStreamDecoder.drawHLine() is supposed to be called and why. The same bugs are likely there for yDivs and Res9patchStreamDecoder.drawVLine() as well. I hope this information helps you in tracking down the bug.

Information

1. Apktool Version (apktool -version) - 2.2.5-0a1670-SNAPSHOT
2. Operating System (Mac, Linux, Windows) - Linux
3. APK From? (Playstore, ROM, Other) - Originally at playstore

Stacktrace/Logcat

Exception in thread "main" java.lang.ArrayIndexOutOfBoundsException: Coordinate out of bounds!
        at sun.awt.image.IntegerInterleavedRaster.setDataElements(IntegerInterleavedRaster.java:301)
        at java.awt.image.BufferedImage.setRGB(BufferedImage.java:1016)
        at brut.androlib.res.decoder.Res9patchStreamDecoder.drawHLine(Res9patchStreamDecoder.java:149)
        at brut.androlib.res.decoder.Res9patchStreamDecoder.decode(Res9patchStreamDecoder.java:69)
        at brut.androlib.res.decoder.ResStreamDecoderContainer.decode(ResStreamDecoderContainer.java:33)
        at brut.androlib.res.decoder.ResFileDecoder.decode(ResFileDecoder.java:120)
        at brut.androlib.res.decoder.ResFileDecoder.decode(ResFileDecoder.java:87)
        at brut.androlib.res.AndrolibResources.decode(AndrolibResources.java:263)
        at brut.androlib.Androlib.decodeResourcesFull(Androlib.java:132)
        at brut.androlib.ApkDecoder.decode(ApkDecoder.java:115)
        at brut.apktool.Main.cmdDecode(Main.java:166)
        at brut.apktool.Main.main(Main.java:80)

Steps to Reproduce

1. apktool -f -v d com.zeropc.tablet-44.apk

Frameworks

N/A

APK

Can be downloaded from archive.org. Note that the latest version of the APK does not exhibit the same error.

Questions to ask before submission

1. Have you tried apktool d, apktool b without changing anything? Yes apktool d com.zeropc.tablet-44.apk on a pristine .apk-file gives the same stacktrace.
2. If you are trying to install a modified apk, did you resign it? No.
3. Are you using the latest apktool version? Yes, I'm using commit 0a16705430b52fd9a6f1d00ec1a3e60b3fbfe604

[sebras]

While waiting for this bug to be resolved I applied the following changes locally since I don't really care if the PNG resources are properly decoded.

@@ -145,12 +145,26 @@ public class Res9patchStreamDecoder implements ResStreamDecoder {
     }

     private void drawHLine(BufferedImage im, int y, int x1, int x2) {
+        int w = im.getWidth();
+
+        if (x1 < 0) x1 = 0;
+        if (x1 > w - 1) x1 = w - 1;
+        if (x2 < 0) x2 = 0;
+        if (x2 > w - 1) x2 = w - 1;
+
         for (int x = x1; x <= x2; x++) {
             im.setRGB(x, y, NP_COLOR);
         }
     }

     private void drawVLine(BufferedImage im, int x, int y1, int y2) {
+        int h = im.getHeight();
+
+        if (y1 < 0) y1 = 0;
+        if (y1 > h - 1) y1 = h - 1;
+        if (y2 < 0) y2 = 0;
+        if (y2 > h - 1) y2 = h - 1;
+
         for (int y = y1; y <= y2; y++) {
             im.setRGB(x, y, NP_COLOR);
         }

[iBotPeaches]

Thanks for the report. When we take a compiled 9patch (The one with binary chunks npTc and npLb) we have to rebuild those binary chunks in raw form by remaking the horizontal/vertical lines that representing the stretchable region of an image.

My memory about drawing the lines is a bit hazy, but we do some logic of placing the vertical/horizontal lines 1 to 2 pixels away from the original image. This is why we create a new temporary image +2 pixels in both height/width of the original image, because in situations we may have a 1px by ?px image that if you applied the 9patch to, would wipe the existing color (since 1px), thus we make the image 3px by ?px, which aapt strips back to 1px as it rips out the 9patch regions and embeds them in the binary chunk.

That may be confusing to read, but short version - while this may fix the crash, does it break the 9patch image? I dunno. I'll look into it.

[sebras]

I'm certain that my workaround messes up the decoding of the image. :) However I wanted apktool to process the file despite the decoded PNG files being corrupt (I don't currently depend on them being correct).

And yes that is a bit confusing to read. if you embed the segments in the image data for aapt later to recode them into nptc chunks, wouldn't you need to extend the image by N - 1 pixels in either direction where N is the number of segments? How would + 2 pixels be enough? There is still obviously something I don't get about this...

[iBotPeaches]

The 9patch decoder was surprisingly built before the Android source was out, so lots of the "magic" numbers were just left behind and after all these years I don't remember them. It just became a "it just works" sort of thing. Sort of an area I didn't explore unless bug reports came in.

It is very possible that the hard coded +2 pixels extend in either direction is wrong and may be best replaced with the (segment - 1) count you mentioned. I don't know if I can slip this ticket into the upcoming 2.2.5 release but our 9patch code might need a slight rewrite in that regard in the next release. So I'll try and dig into the internals to refresh my knowledge of 9patch both compiled & raw.

[iBotPeaches]

Unfortunately I've lost the sample for this. Since over the 5 years this was kinda ignored :( I haven't see another sample with this exact error. Paired with the fact that it was fixed in the updates - I figure it was a bugged 9patch file.

So I think I'll add a try-catch for the ArrayIndexOutOfBounds and then fallback to regular raw / png decoder. That at least will not pause the disassembly and close this.

[sebras]

@iBotPeaches I still have the file: sample.zip

Does this help?

[iBotPeaches]

Does this help?

Thanks. I'll take another look when free.

[sebras]

FWIW here is a backtrace after having built apktool commit 9d7d5801:

FINE: Decoding file: res/drawable-nodpi/abs__shadow_holo_new.9.png to: res/drawable-nodpi-v4/abs__shadow_holo_new.png
Exception in thread "main" java.lang.ArrayIndexOutOfBoundsException: Coordinate out of bounds!
        at java.desktop/sun.awt.image.IntegerInterleavedRaster.setDataElements(IntegerInterleavedRaster.java:298)
        at java.desktop/java.awt.image.BufferedImage.setRGB(BufferedImage.java:1017)
        at brut.androlib.res.decoder.Res9patchStreamDecoder.drawHLine(Res9patchStreamDecoder.java:156)
        at brut.androlib.res.decoder.Res9patchStreamDecoder.decode(Res9patchStreamDecoder.java:73)
        at brut.androlib.res.decoder.ResStreamDecoderContainer.decode(ResStreamDecoderContainer.java:28)
        at brut.androlib.res.decoder.ResFileDecoder.decode(ResFileDecoder.java:145)
        at brut.androlib.res.decoder.ResFileDecoder.decode(ResFileDecoder.java:100)
        at brut.androlib.res.ResourcesDecoder.decodeResources(ResourcesDecoder.java:172)
        at brut.androlib.ApkDecoder.decode(ApkDecoder.java:103)
        at brut.apktool.Main.cmdDecode(Main.java:189)
        at brut.apktool.Main.main(Main.java:92)

[iBotPeaches]

Thanks - I'll peek the 9patch and see if its just bugged or a flaw in our decoder.