Skip to content

Commit

Permalink
fix: decoding APK with many compact entries and unknown uses-sdk attrs (
Browse files Browse the repository at this point in the history 
#3705)

* fix: decoding APK with many compact entries and unknown uses-sdk attrs

This fixes 2 new issues with a stock APK sourced from an Android 15 ROM.

https://drive.google.com/file/d/1x9udLN4W5I7chyGp1ZY8Cyfhu1vXezU9/view

1) mIn.readShort() for size in readEntryData is incorrect and the size < 0 check is not possible.
   Entry size is stored by AAPT2 as an unsigned short and thus will never be negative.
   Reading it as a signed short will cause negative entry sizes in compactly packed entries in
   very large string pools and will result in a lot of "APKTOOL_DUMMYVAL_" values.

2) sdkInfo isn't stored properly for APKs with unexpected properties in uses-sdk tag.
   As far as I can tell, these attributes serve no purpose and can be ignored.
   In the given APK, additional "android:versionCode" and "android:versionName" attributes appear
   in the uses-sdk tag, purpose unknown and they don't represent the actual version of the app.

   E: uses-sdk (line=26)
     A: http://schemas.android.com/apk/res/android:minSdkVersion(0x0101020c)=35
     A: http://schemas.android.com/apk/res/android:versionCode(0x0101021b)=31
     A: http://schemas.android.com/apk/res/android:versionName(0x0101021c)="3.1"
     A: http://schemas.android.com/apk/res/android:targetSdkVersion(0x01010270)=35

* test: add assertion for issue 3705

---------

Co-authored-by: Connor Tumbleson <[email protected]>
Co-authored-by: Connor Tumbleson <[email protected]>
  • Loading branch information
@IgorEisberg @iBotPeaches
3 people authored Oct 4, 2024
1 parent 5c99919 commit 24541c3
Show file tree
Hide file tree
Showing 8 changed files with 137 additions and 68 deletions.
15 changes: 5 additions & 10 deletions brut.apktool/apktool-lib/src/main/java/brut/androlib/res/data/ResResSpec.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,12 @@ public class ResResSpec {

public ResResSpec(ResID id, String name, ResPackage pkg, ResTypeSpec type) {
this.mId = id;
String cleanName;
name = EMPTY_RESOURCE_NAMES.contains(name) ? null : name;

ResResSpec resResSpec = type.getResSpecUnsafe(name);
if (resResSpec != null) {
cleanName = String.format("APKTOOL_DUPLICATE_%s_%s", type, id.toString());
} else {
cleanName = ((name == null || name.isEmpty()) ? ("APKTOOL_DUMMYVAL_" + id.toString()) : name);
if (name == null || name.isEmpty() || EMPTY_RESOURCE_NAMES.contains(name)) {
name = "APKTOOL_DUMMYVAL_" + id.toString();
} else if (type.getResSpecUnsafe(name) != null) {
name = String.format("APKTOOL_DUPLICATE_%s_%s", type, id.toString());
}

this.mName = cleanName;
this.mName = name;
this.mPackage = pkg;
this.mType = type;
}
Expand Down
4 changes: 0 additions & 4 deletions brut.apktool/apktool-lib/src/main/java/brut/androlib/res/data/ResTable.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,10 +266,6 @@ public void setSparseResources(boolean flag) {
mApkInfo.sparseResources = flag;
}

public void clearSdkInfo() {
mApkInfo.sdkInfo.clear();
}

public void addSdkInfo(String key, String value) {
mApkInfo.sdkInfo.put(key, value);
}
Expand Down
6 changes: 1 addition & 5 deletions brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/ARSCDecoder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -355,16 +355,12 @@ private ResType readTableType() throws IOException, AndrolibException {
}

private EntryData readEntryData() throws IOException, AndrolibException {
short size = mIn.readShort();
int size = mIn.readUnsignedShort();
short flags = mIn.readShort();

boolean isComplex = (flags & ENTRY_FLAG_COMPLEX) != 0;
boolean isCompact = (flags & ENTRY_FLAG_COMPACT) != 0;

if (size < 0 && !isCompact) {
throw new AndrolibException("Entry size is under 0 bytes and not compactly packed.");
}

int specNamesId = mIn.readInt();
if (specNamesId == NO_ENTRY && !isCompact) {
return null;
Expand Down
2 changes: 1 addition & 1 deletion brut.apktool/apktool-lib/src/main/java/brut/androlib/res/decoder/AXmlResourceParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -863,5 +863,5 @@ private void setFirstError(AndrolibException error) {
private static final int PRIVATE_PKG_ID = 0x7F;

private static final String ANDROID_RES_NS_AUTO = "http://schemas.android.com/apk/res-auto";
private static final String ANDROID_RES_NS = "http://schemas.android.com/apk/res/android";
public static final String ANDROID_RES_NS = "http://schemas.android.com/apk/res/android";
}
95 changes: 47 additions & 48 deletions ...apktool-lib/src/main/java/brut/androlib/res/decoder/AndroidManifestPullStreamDecoder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ public void decode(InputStream in, OutputStream out)
final ResTable resTable = mParser.getResTable();

XmlSerializerWrapper ser = new StaticXmlSerializerWrapper(mSerial, factory) {
boolean hideSdkInfo = false;
boolean hidePackageInfo = false;
final boolean hideSdkInfo = !resTable.getAnalysisMode();

@Override
public void event(XmlPullParser pp)
Expand All @@ -57,76 +56,76 @@ public void event(XmlPullParser pp)
if (type == XmlPullParser.START_TAG) {
if ("manifest".equals(pp.getName())) {
try {
hidePackageInfo = parseManifest(pp);
parseManifest(pp);
} catch (AndrolibException ignored) {}
} else if ("uses-sdk".equals(pp.getName())) {
try {
hideSdkInfo = parseAttr(pp);
if (hideSdkInfo) {
return;
}
parseUsesSdk(pp);
} catch (AndrolibException ignored) {}
if (hideSdkInfo) {
return;
}
}
} else if (hideSdkInfo && type == XmlPullParser.END_TAG
} else if (type == XmlPullParser.END_TAG
&& "uses-sdk".equals(pp.getName())) {
return;
} else if (hidePackageInfo && type == XmlPullParser.END_TAG
&& "manifest".equals(pp.getName())) {
super.event(pp);
return;
if (hideSdkInfo) {
return;
}
}

super.event(pp);
}

private boolean parseManifest(XmlPullParser pp)
private void parseManifest(XmlPullParser pp)
throws AndrolibException {
String attr_name;

// read <manifest> for package:
for (int i = 0; i < pp.getAttributeCount(); i++) {
attr_name = pp.getAttributeName(i);

if (attr_name.equals(("package"))) {
resTable.setPackageRenamed(pp.getAttributeValue(i));
} else if (attr_name.equals("versionCode")) {
resTable.setVersionCode(pp.getAttributeValue(i));
} else if (attr_name.equals("versionName")) {
resTable.setVersionName(pp.getAttributeValue(i));
String ns = pp.getAttributeNamespace(i);
String name = pp.getAttributeName(i);
String value = pp.getAttributeValue(i);

if (value.isEmpty()) {
continue;
}

if (ns.isEmpty()) {
if (name.equals("package")) {
resTable.setPackageRenamed(value);
}
} else if (ns.equals(AXmlResourceParser.ANDROID_RES_NS)) {
switch (name) {
case "versionCode":
resTable.setVersionCode(value);
break;
case "versionName":
resTable.setVersionName(value);
break;
}
}
}
return true;
}

private boolean parseAttr(XmlPullParser pp)
private void parseUsesSdk(XmlPullParser pp)
throws AndrolibException {
for (int i = 0; i < pp.getAttributeCount(); i++) {
final String a_ns = "http://schemas.android.com/apk/res/android";
String ns = pp.getAttributeNamespace(i);
String name = pp.getAttributeName(i);
String value = pp.getAttributeValue(i);

if (a_ns.equals(ns)) {
String name = pp.getAttributeName(i);
String value = pp.getAttributeValue(i);
if (name != null && value != null) {
if (name.equals("minSdkVersion")
|| name.equals("targetSdkVersion")
|| name.equals("maxSdkVersion")
|| name.equals("compileSdkVersion")) {
resTable.addSdkInfo(name, value);
} else {
resTable.clearSdkInfo();
return false; // Found unknown flags
}
}
} else {
resTable.clearSdkInfo();
if (value.isEmpty()) {
continue;
}

if (i >= pp.getAttributeCount()) {
return false; // Found unknown flags
if (ns.equals(AXmlResourceParser.ANDROID_RES_NS)) {
switch (name) {
case "minSdkVersion":
case "targetSdkVersion":
case "maxSdkVersion":
case "compileSdkVersion":
resTable.addSdkInfo(name, value);
break;
}
}
}

return ! resTable.getAnalysisMode();
}
};

Expand Down
16 changes: 16 additions & 0 deletions brut.apktool/apktool-lib/src/test/java/brut/androlib/BaseTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import org.custommonkey.xmlunit.*;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
import org.xml.sax.SAXException;

import javax.xml.parsers.DocumentBuilder;
Expand Down Expand Up @@ -155,6 +157,20 @@ protected static int getStringEntryCount(Document doc, String key) {
return count;
}

protected static boolean resourceNameContains(Element element, String name) {
if (element.hasAttribute("name") && element.getAttribute("name").contains(name)) {
return true;
}
NodeList children = element.getChildNodes();
for (int i = 0; i < children.getLength(); i++) {
Node child = children.item(i);
if (child.getNodeType() == Node.ELEMENT_NODE && resourceNameContains((Element) child, name)) {
return true;
}
}
return false;
}

protected static ExtFile sTmpDir;
protected static ExtFile sTestOrigDir;
protected static ExtFile sTestNewDir;
Expand Down
67 changes: 67 additions & 0 deletions brut.apktool/apktool-lib/src/test/java/brut/androlib/decode/LargeCompactResourceTest.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/*
* Copyright (C) 2010 Ryszard Wiśniewski <[email protected]>
* Copyright (C) 2010 Connor Tumbleson <[email protected]>
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package brut.androlib.decode;

import brut.androlib.*;
import brut.directory.ExtFile;
import brut.common.BrutException;
import brut.util.OS;
import java.io.File;
import java.io.IOException;

import org.junit.*;
import org.w3c.dom.Document;
import org.xml.sax.SAXException;

import javax.xml.parsers.ParserConfigurationException;

import static org.junit.Assert.*;

public class LargeCompactResourceTest extends BaseTest {

@BeforeClass
public static void beforeClass() throws Exception {
TestUtils.cleanFrameworkFile();
sTmpDir = new ExtFile(OS.createTempDirectory());
TestUtils.copyResourceDir(CompactResourceTest.class, "decode/issue3705/", sTmpDir);
}

@AfterClass
public static void afterClass() throws BrutException {
OS.rmdir(sTmpDir);
}

@Test
public void checkIfDecodeSucceeds() throws BrutException, IOException, ParserConfigurationException, SAXException {
String apk = "issue3705.apk";
ExtFile testApk = new ExtFile(sTmpDir, apk);

// decode issue3705.apk
ApkDecoder apkDecoder = new ApkDecoder(testApk);
sTestOrigDir = new ExtFile(sTmpDir + File.separator + apk + ".out");

File outDir = new File(sTmpDir + File.separator + apk + ".out");
apkDecoder.decode(outDir);

Document doc = loadDocument(new File(sTestOrigDir + "/res/values/strings.xml"));
assertFalse(resourceNameContains(doc.getDocumentElement(), "APKTOOL"));

Config config = Config.getDefaultConfig();
LOGGER.info("Building issue3705.apk...");
new ApkBuilder(sTestOrigDir, config).build(testApk);
}
}
Binary file added brut.apktool/apktool-lib/src/test/resources/decode/issue3705/issue3705.apk
View file
Binary file not shown.

0 comments on commit 24541c3

Please sign in to comment.