From b58b1b52c1bb50103b5e9a39b53ef1f2ed8d2558 Mon Sep 17 00:00:00 2001 From: qfalconer Date: Wed, 3 Jul 2024 16:20:39 +0200 Subject: [PATCH 1/2] The attributes size of an XML element is now accounted for. This size must be at least 20 (0x14) bytes but can be greater. Extra bytes are just skipped. When decoding a string, if such decoding is impossible a placeholder string is returned instead of throwing an exception. This is necessary because some malware purposely add android:tag attributes with invalid string index to throw parsers off. They also employ non-standard attribute sizes. --- .../main/java/jadx/core/xmlgen/BinaryXMLParser.java | 11 +++++++---- .../main/java/jadx/core/xmlgen/BinaryXMLStrings.java | 11 ++++++++++- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/jadx-core/src/main/java/jadx/core/xmlgen/BinaryXMLParser.java b/jadx-core/src/main/java/jadx/core/xmlgen/BinaryXMLParser.java index a66ce2d8a11..c0769540068 100644 --- a/jadx-core/src/main/java/jadx/core/xmlgen/BinaryXMLParser.java +++ b/jadx-core/src/main/java/jadx/core/xmlgen/BinaryXMLParser.java @@ -265,9 +265,10 @@ private void parseElement() throws IOException { die("startNS's attributeStart is not 0x14"); } int attributeSize = is.readInt16(); - if (attributeSize != 0x14) { - die("startNS's attributeSize is not 0x14"); + if (attributeSize < 0x14) { + die("startNS's attributeSize is less than 0x14"); } + int attributeCount = is.readInt16(); int idIndex = is.readInt16(); int classIndex = is.readInt16(); @@ -289,7 +290,7 @@ private void parseElement() throws IOException { Set attrCache = new HashSet<>(); boolean attrNewLine = attributeCount != 1 && this.attrNewLine; for (int i = 0; i < attributeCount; i++) { - parseAttribute(i, attrNewLine, attrCache); + parseAttribute(i, attrNewLine, attrCache, attributeSize); } long endPos = is.getPos(); if (endPos - startPos + 0x4 < elementSize) { @@ -297,7 +298,7 @@ private void parseElement() throws IOException { } } - private void parseAttribute(int i, boolean newLine, Set attrCache) throws IOException { + private void parseAttribute(int i, boolean newLine, Set attrCache, int attributeSize) throws IOException { int attributeNS = is.readInt32(); int attributeName = is.readInt32(); int attributeRawValue = is.readInt32(); @@ -305,6 +306,8 @@ private void parseAttribute(int i, boolean newLine, Set attrCache) throw int attrValDataType = is.readInt8(); int attrValData = is.readInt32(); + is.skip(attributeSize-0x14); + String shortNsName = null; if (attributeNS != -1) { shortNsName = getAttributeNS(attributeNS, newLine); diff --git a/jadx-core/src/main/java/jadx/core/xmlgen/BinaryXMLStrings.java b/jadx-core/src/main/java/jadx/core/xmlgen/BinaryXMLStrings.java index 7b9effe45b7..3733af2838d 100644 --- a/jadx-core/src/main/java/jadx/core/xmlgen/BinaryXMLStrings.java +++ b/jadx-core/src/main/java/jadx/core/xmlgen/BinaryXMLStrings.java @@ -7,6 +7,7 @@ import java.util.Map; public class BinaryXMLStrings { + public static final String INVALID_STRING_PLACEHOLDER = "⟨STRING_DECODE_ERROR⟩"; private final int stringCount; private final long stringsStart; @@ -40,6 +41,10 @@ public String get(int id) { return cached; } + if (id*4 >= buffer.limit() - 3) { + return INVALID_STRING_PLACEHOLDER; + } + long offset = stringsStart + buffer.getInt(id * 4); String extracted; if (isUtf8) { @@ -63,7 +68,7 @@ public int size() { private static String extractString8(byte[] strArray, int offset) { if (offset >= strArray.length) { - return "STRING_DECODE_ERROR"; + return INVALID_STRING_PLACEHOLDER; } int start = offset + skipStrLen8(strArray, offset); int len = strArray[start++]; @@ -78,6 +83,10 @@ private static String extractString8(byte[] strArray, int offset) { } private static String extractString16(byte[] strArray, int offset) { + if (offset + 2 >= strArray.length) { + return INVALID_STRING_PLACEHOLDER; + } + int len = strArray.length; int start = offset + skipStrLen16(strArray, offset); int end = start; From af76eec1813f2c213622ef86ef2192e0ce58aae4 Mon Sep 17 00:00:00 2001 From: qfalconer Date: Wed, 3 Jul 2024 16:37:08 +0200 Subject: [PATCH 2/2] Minor code restyling --- jadx-core/src/main/java/jadx/core/xmlgen/BinaryXMLParser.java | 2 +- jadx-core/src/main/java/jadx/core/xmlgen/BinaryXMLStrings.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/jadx-core/src/main/java/jadx/core/xmlgen/BinaryXMLParser.java b/jadx-core/src/main/java/jadx/core/xmlgen/BinaryXMLParser.java index c0769540068..203f127e6cc 100644 --- a/jadx-core/src/main/java/jadx/core/xmlgen/BinaryXMLParser.java +++ b/jadx-core/src/main/java/jadx/core/xmlgen/BinaryXMLParser.java @@ -306,7 +306,7 @@ private void parseAttribute(int i, boolean newLine, Set attrCache, int a int attrValDataType = is.readInt8(); int attrValData = is.readInt32(); - is.skip(attributeSize-0x14); + is.skip(attributeSize - 0x14); String shortNsName = null; if (attributeNS != -1) { diff --git a/jadx-core/src/main/java/jadx/core/xmlgen/BinaryXMLStrings.java b/jadx-core/src/main/java/jadx/core/xmlgen/BinaryXMLStrings.java index 3733af2838d..a5090f044ff 100644 --- a/jadx-core/src/main/java/jadx/core/xmlgen/BinaryXMLStrings.java +++ b/jadx-core/src/main/java/jadx/core/xmlgen/BinaryXMLStrings.java @@ -41,7 +41,7 @@ public String get(int id) { return cached; } - if (id*4 >= buffer.limit() - 3) { + if (id * 4 >= buffer.limit() - 3) { return INVALID_STRING_PLACEHOLDER; }