Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Hardened String Block Parser #3239

Merged
merged 9 commits into from
Aug 1, 2023
Merged

Hardened String Block Parser #3239

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
76e2bbb
fix: add headerSize to stringBlock to detect larger headers
iBotPeaches Jul 31, 2023
a9ebcd7
fix: handle app with style offset, but 0 styles
iBotPeaches Jul 31, 2023
0dd8feb
refactor: split counting stream into CountingDataInput
iBotPeaches Jul 31, 2023
0297845
fix: read strings till end of string pool chunk
iBotPeaches Jul 31, 2023
cfa6ecd
fix: support out of bound string reading
iBotPeaches Jul 31, 2023
9e6b597
fix: don't read string/style offset out of bounds
iBotPeaches Aug 1, 2023
43f7783
refactor: cleanup comments for string parser
iBotPeaches Aug 1, 2023
c4d3e9b
style: comment on 4 byte alignment
iBotPeaches Aug 1, 2023
338f2a5
fix: only warn if utf16 string
iBotPeaches Aug 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions brut.apktool/apktool-lib/src/main/java/brut/androlib/res/data/arsc/ARSCHeader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
*/
package brut.androlib.res.data.arsc;

import brut.util.ExtCountingDataInput;
import brut.util.ExtDataInput;
import org.apache.commons.io.input.CountingInputStream;

import java.io.EOFException;
import java.io.IOException;
Expand All @@ -39,23 +39,23 @@ public ARSCHeader(short type, int headerSize, int chunkSize, int headerStart) {
this.endPosition = headerStart + chunkSize;
}

public static ARSCHeader read(ExtDataInput in, CountingInputStream countIn) throws IOException {
public static ARSCHeader read(ExtCountingDataInput in) throws IOException {
short type;
int start = countIn.getCount();
int start = in.position();
try {
type = in.readShort();
} catch (EOFException ex) {
return new ARSCHeader(RES_NONE_TYPE, 0, 0, countIn.getCount());
return new ARSCHeader(RES_NONE_TYPE, 0, 0, in.position());
}
return new ARSCHeader(type, in.readShort(), in.readInt(), start);
}

public void checkForUnreadHeader(ExtDataInput in, CountingInputStream countIn) throws IOException {
public void checkForUnreadHeader(ExtCountingDataInput in) 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 actualHeaderSize = in.position() - this.startPosition;
int exceedingSize = this.headerSize - actualHeaderSize;
if (exceedingSize > 0) {
byte[] buf = new byte[exceedingSize];
Expand Down
42 changes: 18 additions & 24 deletions brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/ARSCDecoder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,8 @@
import brut.androlib.res.data.arsc.*;
import brut.androlib.res.data.value.*;
import brut.util.Duo;
import brut.util.ExtDataInput;
import brut.util.ExtCountingDataInput;
import com.google.common.io.LittleEndianDataInputStream;
import org.apache.commons.io.input.CountingInputStream;

import java.io.*;
import java.math.BigInteger;
Expand Down Expand Up @@ -52,16 +51,12 @@ public static ARSCData decode(InputStream arscStream, boolean findFlagsOffsets,
}

private ARSCDecoder(InputStream arscStream, ResTable resTable, boolean storeFlagsOffsets, boolean keepBroken) {
arscStream = mCountIn = new CountingInputStream(arscStream);
if (storeFlagsOffsets) {
mFlagsOffsets = new ArrayList<>();
} else {
mFlagsOffsets = null;
}
// We need to explicitly cast to DataInput as otherwise the constructor is ambiguous.
// We choose DataInput instead of InputStream as ExtDataInput wraps an InputStream in
// a DataInputStream which is big-endian and ignores the little-endian behavior.
mIn = new ExtDataInput((DataInput) new LittleEndianDataInputStream(arscStream));
mIn = new ExtCountingDataInput(new LittleEndianDataInputStream(arscStream));
mResTable = resTable;
mKeepBroken = keepBroken;
}
Expand Down Expand Up @@ -126,20 +121,20 @@ private ResPackage[] readResourceTable() throws IOException, AndrolibException {

private void readStringPoolChunk() throws IOException, AndrolibException {
checkChunkType(ARSCHeader.RES_STRING_POOL_TYPE);
mTableStrings = StringBlock.readWithoutChunk(mIn, mHeader.chunkSize);
mTableStrings = StringBlock.readWithoutChunk(mIn, mHeader.startPosition, mHeader.headerSize, mHeader.chunkSize);
}

private void readTableChunk() throws IOException, AndrolibException {
checkChunkType(ARSCHeader.RES_TABLE_TYPE);
mIn.skipInt(); // packageCount

mHeader.checkForUnreadHeader(mIn, mCountIn);
mHeader.checkForUnreadHeader(mIn);
}

private void readUnknownChunk() throws IOException, AndrolibException {
checkChunkType(ARSCHeader.RES_NULL_TYPE);

mHeader.checkForUnreadHeader(mIn, mCountIn);
mHeader.checkForUnreadHeader(mIn);

LOGGER.warning("Skipping unknown chunk data of size " + mHeader.chunkSize);
mHeader.skipChunk(mIn);
Expand Down Expand Up @@ -177,7 +172,7 @@ private ResPackage readTablePackage() throws IOException, AndrolibException {
LOGGER.warning("Please report this application to Apktool for a fix: https://github.com/iBotPeaches/Apktool/issues/1728");
}

mHeader.checkForUnreadHeader(mIn, mCountIn);
mHeader.checkForUnreadHeader(mIn);

mTypeNames = StringBlock.readWithChunk(mIn);
mSpecNames = StringBlock.readWithChunk(mIn);
Expand All @@ -195,7 +190,7 @@ private void readLibraryType() throws AndrolibException, IOException {
int packageId;
String packageName;

mHeader.checkForUnreadHeader(mIn, mCountIn);
mHeader.checkForUnreadHeader(mIn);

for (int i = 0; i < libraryCount; i++) {
packageId = mIn.readInt();
Expand All @@ -207,7 +202,7 @@ private void readLibraryType() throws AndrolibException, IOException {
private void readStagedAliasSpec() throws IOException {
int count = mIn.readInt();

mHeader.checkForUnreadHeader(mIn, mCountIn);
mHeader.checkForUnreadHeader(mIn);

for (int i = 0; i < count; i++) {
LOGGER.fine(String.format("Skipping staged alias stagedId (%h) finalId: %h", mIn.readInt(), mIn.readInt()));
Expand All @@ -219,7 +214,7 @@ private void readOverlaySpec() throws AndrolibException, IOException {
String name = mIn.readNullEndedString(256, true);
String actor = mIn.readNullEndedString(256, true);

mHeader.checkForUnreadHeader(mIn, mCountIn);
mHeader.checkForUnreadHeader(mIn);

LOGGER.fine(String.format("Overlay name: \"%s\", actor: \"%s\")", name, actor));
}
Expand All @@ -229,7 +224,7 @@ private void readOverlayPolicySpec() throws AndrolibException, IOException {
mIn.skipInt(); // policyFlags
int count = mIn.readInt();

mHeader.checkForUnreadHeader(mIn, mCountIn);
mHeader.checkForUnreadHeader(mIn);

for (int i = 0; i < count; i++) {
LOGGER.fine(String.format("Skipping overlay (%h)", mIn.readInt()));
Expand All @@ -244,10 +239,10 @@ private ResTypeSpec readTableSpecType() throws AndrolibException, IOException {
int entryCount = mIn.readInt();

if (mFlagsOffsets != null) {
mFlagsOffsets.add(new FlagsOffset(mCountIn.getCount(), entryCount));
mFlagsOffsets.add(new FlagsOffset(mIn.position(), entryCount));
}

mHeader.checkForUnreadHeader(mIn, mCountIn);
mHeader.checkForUnreadHeader(mIn);

mIn.skipBytes(entryCount * 4); // flags
mTypeSpec = new ResTypeSpec(mTypeNames.getString(id - 1), mResTable, mPkg, id, entryCount);
Expand All @@ -272,7 +267,7 @@ private ResType readTableType() throws IOException, AndrolibException {
mMissingResSpecMap = new LinkedHashMap<>();
ResConfigFlags flags = readConfigFlags();

mHeader.checkForUnreadHeader(mIn, mCountIn);
mHeader.checkForUnreadHeader(mIn);

if ((typeFlags & 0x01) != 0) {
LOGGER.fine("Sparse type flags detected: " + mTypeSpec.getName());
Expand Down Expand Up @@ -310,7 +305,7 @@ private ResType readTableType() throws IOException, AndrolibException {
mResId = (mResId & 0xffff0000) | i;

// As seen in some recent APKs - there are more entries reported than can fit in the chunk.
if (mCountIn.getCount() == mHeader.endPosition) {
if (mIn.position() == mHeader.endPosition) {
int remainingEntries = entryCount - i;
LOGGER.warning(String.format("End of chunk hit. Skipping remaining entries (%d) in type: %s",
remainingEntries, mTypeSpec.getName()
Expand All @@ -325,8 +320,8 @@ private ResType readTableType() throws IOException, AndrolibException {
}

// skip "TYPE 8 chunks" and/or padding data at the end of this chunk
if (mCountIn.getCount() < mHeader.endPosition) {
long bytesSkipped = mCountIn.skip(mHeader.endPosition - mCountIn.getCount());
if (mIn.position() < mHeader.endPosition) {
long bytesSkipped = mIn.skip(mHeader.endPosition - mIn.position());
LOGGER.warning("Unknown data detected. Skipping: " + bytesSkipped + " byte(s)");
}

Expand Down Expand Up @@ -630,7 +625,7 @@ private void removeResSpec(ResResSpec spec) {
}

private ARSCHeader nextChunk() throws IOException {
return mHeader = ARSCHeader.read(mIn, mCountIn);
return mHeader = ARSCHeader.read(mIn);
}

private void checkChunkType(int expectedType) throws AndrolibException {
Expand All @@ -640,9 +635,8 @@ private void checkChunkType(int expectedType) throws AndrolibException {
}
}

private final ExtDataInput mIn;
private final ExtCountingDataInput mIn;
private final ResTable mResTable;
private final CountingInputStream mCountIn;
private final List<FlagsOffset> mFlagsOffsets;
private final boolean mKeepBroken;

Expand Down
19 changes: 6 additions & 13 deletions brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/AXmlResourceParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,8 @@
import brut.androlib.res.data.arsc.ARSCHeader;
import brut.androlib.res.data.axml.NamespaceStack;
import brut.androlib.res.xml.ResXmlEncoders;
import brut.util.ExtDataInput;
import brut.util.ExtCountingDataInput;
import com.google.common.io.LittleEndianDataInputStream;
import org.apache.commons.io.input.CountingInputStream;
import org.xmlpull.v1.XmlPullParserException;

import java.io.*;
Expand Down Expand Up @@ -64,11 +63,7 @@ public void setAttrDecoder(ResAttrDecoder attrDecoder) {
public void open(InputStream stream) {
close();
if (stream != null) {
stream = mCountIn = new CountingInputStream(stream);
// We need to explicitly cast to DataInput as otherwise the constructor is ambiguous.
// We choose DataInput instead of InputStream as ExtDataInput wraps an InputStream in
// a DataInputStream which is big-endian and ignores the little-endian behavior.
mIn = new ExtDataInput((DataInput) new LittleEndianDataInputStream(stream));
mIn = new ExtCountingDataInput(new LittleEndianDataInputStream(stream));
}
}

Expand All @@ -79,7 +74,6 @@ public void close() {
}
isOperational = false;
mIn = null;
mCountIn = null;
mStringBlock = null;
mResourceIds = null;
mNamespaces.reset();
Expand Down Expand Up @@ -687,8 +681,8 @@ private void doNext() throws IOException {
}

// #2070 - Some applications have 2 start namespaces, but only 1 end namespace.
if (mCountIn.available() == 0) {
LOGGER.warning(String.format("AXML hit unexpected end of file at byte: 0x%X", mCountIn.getCount()));
if (mIn.remaining() == 0) {
LOGGER.warning(String.format("AXML hit unexpected end of file at byte: 0x%X", mIn.position()));
mEvent = END_DOCUMENT;
break;
}
Expand All @@ -715,7 +709,7 @@ private void doNext() throws IOException {
if (chunkType < ARSCHeader.RES_XML_FIRST_CHUNK_TYPE || chunkType > ARSCHeader.RES_XML_LAST_CHUNK_TYPE) {
int chunkSize = mIn.readInt();
mIn.skipBytes(chunkSize - 8);
LOGGER.warning(String.format("Unknown chunk type at: (0x%08x) skipping...", mCountIn.getCount()));
LOGGER.warning(String.format("Unknown chunk type at: (0x%08x) skipping...", mIn.position()));
break;
}

Expand Down Expand Up @@ -806,8 +800,7 @@ private void setFirstError(AndrolibException error) {
}
}

private ExtDataInput mIn;
private CountingInputStream mCountIn;
private ExtCountingDataInput mIn;
private ResAttrDecoder mAttrDecoder;
private AndrolibException mFirstError;

Expand Down
55 changes: 39 additions & 16 deletions brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/StringBlock.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

import brut.androlib.res.data.arsc.ARSCHeader;
import brut.androlib.res.xml.ResXmlEncoders;
import brut.util.ExtDataInput;
import brut.util.ExtCountingDataInput;
import com.google.common.annotations.VisibleForTesting;

import java.io.IOException;
Expand All @@ -30,44 +30,61 @@
import java.util.logging.Logger;

public class StringBlock {
public static StringBlock readWithChunk(ExtDataInput reader) throws IOException {
public static StringBlock readWithChunk(ExtCountingDataInput reader) throws IOException {
int startPosition = reader.position();
reader.skipCheckShort(ARSCHeader.RES_STRING_POOL_TYPE);
reader.skipShort(); // headerSize
int headerSize = reader.readShort();
int chunkSize = reader.readInt();

return readWithoutChunk(reader, chunkSize);
return readWithoutChunk(reader, startPosition, headerSize, chunkSize);
}

public static StringBlock readWithoutChunk(ExtDataInput reader, int chunkSize) throws IOException {
public static StringBlock readWithoutChunk(ExtCountingDataInput reader, int startPosition, int headerSize,
int chunkSize) throws IOException
{
// ResStringPool_header
int stringCount = reader.readInt();
int styleCount = reader.readInt();
int flags = reader.readInt();
int stringsOffset = reader.readInt();
int stylesOffset = reader.readInt();

// For some applications they pack the StringBlock header with more unused data at end.
if (headerSize > STRING_BLOCK_HEADER_SIZE) {
reader.skipBytes(headerSize - STRING_BLOCK_HEADER_SIZE);
}

StringBlock block = new StringBlock();
block.m_isUTF8 = (flags & UTF8_FLAG) != 0;
block.m_stringOffsets = reader.readIntArray(stringCount);
block.m_stringOffsets = reader.readSafeIntArray(stringCount, startPosition + stringsOffset);

if (styleCount != 0) {
block.m_styleOffsets = reader.readIntArray(styleCount);
block.m_styleOffsets = reader.readSafeIntArray(styleCount, startPosition + stylesOffset);
}

// #3236 - Some applications give a style offset, but have 0 styles. Make this check more robust.
boolean hasStyles = stylesOffset != 0 && styleCount != 0;
int size = chunkSize - stringsOffset;

// If we have both strings and even just a lying style offset - lets calculate the size of the strings without
// accidentally parsing all the styles.
if (styleCount > 0) {
size = stylesOffset - stringsOffset;
}

int size = ((stylesOffset == 0) ? chunkSize : stylesOffset) - stringsOffset;
block.m_strings = new byte[size];
reader.readFully(block.m_strings);

if (stylesOffset != 0) {
size = (chunkSize - stylesOffset);
if (hasStyles) {
size = chunkSize - stylesOffset;
block.m_styles = reader.readIntArray(size / 4);
}

// read remaining bytes
int remaining = size % 4;
if (remaining >= 1) {
while (remaining-- > 0) {
reader.readByte();
}
// In case we aren't 4 byte aligned we need to skip the remaining bytes.
int remaining = size % 4;
if (remaining >= 1) {
while (remaining-- > 0) {
reader.readByte();
}
}

Expand Down Expand Up @@ -211,6 +228,11 @@ String decodeString(int offset, int length) {
LOGGER.warning("Failed to decode a string at offset " + offset + " of length " + length);
return null;
}
} catch (IndexOutOfBoundsException ex) {
if (!m_isUTF8) {
LOGGER.warning("String extends outside of pool at " + offset + " of length " + length);
return null;
}
}

try {
Expand Down Expand Up @@ -275,4 +297,5 @@ private static int[] getUtf16(byte[] array, int offset) {
private static final Logger LOGGER = Logger.getLogger(StringBlock.class.getName());

private static final int UTF8_FLAG = 0x00000100;
private static final int STRING_BLOCK_HEADER_SIZE = 28;
}
1 change: 1 addition & 0 deletions brut.j.util/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,5 @@
dependencies {
implementation project(':brut.j.common')
implementation depends.commons_io
implementation depends.guava
}
Loading