fix(xml): allow for non-standard attributes sizes and avoid index exceptions when decoding some strings (PR #2210)
More lenient AXML parsing: allow for non-standard attributes sizes and avoid index exceptions when decoding some strings * 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. * Minor code restyling --------- Co-authored-by: qfalconer <knm241@gmail.com>
This commit is contained in:
@@ -265,9 +265,10 @@ public class BinaryXMLParser extends CommonBinaryParser {
|
||||
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 @@ public class BinaryXMLParser extends CommonBinaryParser {
|
||||
Set<String> 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 @@ public class BinaryXMLParser extends CommonBinaryParser {
|
||||
}
|
||||
}
|
||||
|
||||
private void parseAttribute(int i, boolean newLine, Set<String> attrCache) throws IOException {
|
||||
private void parseAttribute(int i, boolean newLine, Set<String> attrCache, int attributeSize) throws IOException {
|
||||
int attributeNS = is.readInt32();
|
||||
int attributeName = is.readInt32();
|
||||
int attributeRawValue = is.readInt32();
|
||||
@@ -305,6 +306,8 @@ public class BinaryXMLParser extends CommonBinaryParser {
|
||||
int attrValDataType = is.readInt8();
|
||||
int attrValData = is.readInt32();
|
||||
|
||||
is.skip(attributeSize - 0x14);
|
||||
|
||||
String shortNsName = null;
|
||||
if (attributeNS != -1) {
|
||||
shortNsName = getAttributeNS(attributeNS, newLine);
|
||||
|
||||
@@ -7,6 +7,7 @@ import java.util.HashMap;
|
||||
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 class BinaryXMLStrings {
|
||||
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 class BinaryXMLStrings {
|
||||
|
||||
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 @@ public class BinaryXMLStrings {
|
||||
}
|
||||
|
||||
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;
|
||||
|
||||
Reference in New Issue
Block a user