From 71617a1c70636a290a31e370401c344d067f6bd7 Mon Sep 17 00:00:00 2001 From: sergey-wowwow <15173437+sergey-wowwow@users.noreply.github.com> Date: Sun, 25 Oct 2020 22:18:41 +0300 Subject: [PATCH] feat(res): fix duplicate entries and deobfuscate file names in XML resources (PR #995) * Fixes dublicates entries in XML resources * can't use binary search on this list * add entry config to name comparator, preserve renames by id, improve performance * Deobf resource files * Add break * Changes ResourceFile Co-authored-by: sergey-wowwow Co-authored-by: Skylot --- .../src/main/java/jadx/api/ResourceFile.java | 17 +++++- .../java/jadx/api/ResourceFileContent.java | 2 +- .../main/java/jadx/api/ResourcesLoader.java | 12 ++-- .../java/jadx/core/dex/nodes/RootNode.java | 29 ++++++++-- .../java/jadx/core/xmlgen/ResContainer.java | 2 +- .../java/jadx/core/xmlgen/ResTableParser.java | 53 +++++++++++------ .../main/java/jadx/core/xmlgen/ResXmlGen.java | 2 +- .../jadx/core/xmlgen/ResourceStorage.java | 57 ++++++++++++++----- .../jadx/core/xmlgen/entry/ResourceEntry.java | 29 ++++++---- .../java/jadx/tests/api/IntegrationTest.java | 2 +- .../java/jadx/gui/treemodel/ApkSignature.java | 2 +- .../java/jadx/gui/treemodel/JResource.java | 2 +- .../main/java/jadx/gui/treemodel/JRoot.java | 4 +- 13 files changed, 150 insertions(+), 63 deletions(-) diff --git a/jadx-core/src/main/java/jadx/api/ResourceFile.java b/jadx-core/src/main/java/jadx/api/ResourceFile.java index ff8d0abe2..129e79e4b 100644 --- a/jadx-core/src/main/java/jadx/api/ResourceFile.java +++ b/jadx-core/src/main/java/jadx/api/ResourceFile.java @@ -4,6 +4,7 @@ import java.io.File; import jadx.api.plugins.utils.ZipSecurity; import jadx.core.xmlgen.ResContainer; +import jadx.core.xmlgen.entry.ResourceEntry; public class ResourceFile { @@ -34,6 +35,7 @@ public class ResourceFile { private final String name; private final ResourceType type; private ZipRef zipRef; + private String deobfName; public static ResourceFile createResourceFile(JadxDecompiler decompiler, String name, ResourceType type) { if (!ZipSecurity.isValidZipEntryName(name)) { @@ -48,10 +50,14 @@ public class ResourceFile { this.type = type; } - public String getName() { + public String getOriginalName() { return name; } + public String getDeobfName() { + return deobfName != null ? deobfName : name; + } + public ResourceType getType() { return type; } @@ -64,6 +70,15 @@ public class ResourceFile { this.zipRef = zipRef; } + public void setAlias(ResourceEntry ri) { + int index = name.lastIndexOf('.'); + deobfName = String.format("%s%s/%s%s", + ri.getTypeName(), + ri.getConfig(), + ri.getKeyName(), + index == -1 ? "" : name.substring(index)); + } + public ZipRef getZipRef() { return zipRef; } diff --git a/jadx-core/src/main/java/jadx/api/ResourceFileContent.java b/jadx-core/src/main/java/jadx/api/ResourceFileContent.java index d5fefbf1b..8fc6a6a8c 100644 --- a/jadx-core/src/main/java/jadx/api/ResourceFileContent.java +++ b/jadx-core/src/main/java/jadx/api/ResourceFileContent.java @@ -12,6 +12,6 @@ public class ResourceFileContent extends ResourceFile { @Override public ResContainer loadContent() { - return ResContainer.textResource(getName(), content); + return ResContainer.textResource(getDeobfName(), content); } } diff --git a/jadx-core/src/main/java/jadx/api/ResourcesLoader.java b/jadx-core/src/main/java/jadx/api/ResourcesLoader.java index 25d8cdab9..7256304f8 100644 --- a/jadx-core/src/main/java/jadx/api/ResourcesLoader.java +++ b/jadx-core/src/main/java/jadx/api/ResourcesLoader.java @@ -55,7 +55,7 @@ public final class ResourcesLoader { try { ZipRef zipRef = rf.getZipRef(); if (zipRef == null) { - File file = new File(rf.getName()); + File file = new File(rf.getOriginalName()); try (InputStream inputStream = new BufferedInputStream(new FileInputStream(file))) { return decoder.decode(file.length(), inputStream); } @@ -74,7 +74,7 @@ public final class ResourcesLoader { } } } catch (Exception e) { - throw new JadxException("Error decode: " + rf.getName(), e); + throw new JadxException("Error decode: " + rf.getDeobfName(), e); } } @@ -86,7 +86,7 @@ public final class ResourcesLoader { CodeWriter cw = new CodeWriter(); cw.add("Error decode ").add(rf.getType().toString().toLowerCase()); Utils.appendStackTrace(cw, e.getCause()); - return ResContainer.textResource(rf.getName(), cw.finish()); + return ResContainer.textResource(rf.getDeobfName(), cw.finish()); } } @@ -96,7 +96,7 @@ public final class ResourcesLoader { case MANIFEST: case XML: ICodeInfo content = jadxRef.getXmlParser().parse(inputStream); - return ResContainer.textResource(rf.getName(), content); + return ResContainer.textResource(rf.getOriginalName(), content); case ARSC: return new ResTableParser(jadxRef.getRoot()).decodeFiles(inputStream); @@ -110,12 +110,12 @@ public final class ResourcesLoader { } private static ResContainer decodeImage(ResourceFile rf, InputStream inputStream) { - String name = rf.getName(); + String name = rf.getOriginalName(); if (name.endsWith(".9.png")) { try (ByteArrayOutputStream os = new ByteArrayOutputStream()) { Res9patchStreamDecoder decoder = new Res9patchStreamDecoder(); decoder.decode(inputStream, os); - return ResContainer.decodedData(rf.getName(), os.toByteArray()); + return ResContainer.decodedData(rf.getDeobfName(), os.toByteArray()); } catch (Exception e) { LOG.error("Failed to decode 9-patch png image, path: {}", name, e); } diff --git a/jadx-core/src/main/java/jadx/core/dex/nodes/RootNode.java b/jadx-core/src/main/java/jadx/core/dex/nodes/RootNode.java index ade131e5d..8990ef409 100644 --- a/jadx-core/src/main/java/jadx/core/dex/nodes/RootNode.java +++ b/jadx-core/src/main/java/jadx/core/dex/nodes/RootNode.java @@ -39,6 +39,8 @@ import jadx.core.utils.android.AndroidResourcesUtils; import jadx.core.utils.exceptions.JadxRuntimeException; import jadx.core.xmlgen.ResTableParser; import jadx.core.xmlgen.ResourceStorage; +import jadx.core.xmlgen.entry.ResourceEntry; +import jadx.core.xmlgen.entry.ValuesParser; public class RootNode { private static final Logger LOG = LoggerFactory.getLogger(RootNode.class); @@ -131,13 +133,14 @@ public class RootNode { return; } try { - ResourceStorage resStorage = ResourcesLoader.decodeStream(arsc, (size, is) -> { - ResTableParser parser = new ResTableParser(this); - parser.decode(is); - return parser.getResStorage(); + ResTableParser parser = ResourcesLoader.decodeStream(arsc, (size, is) -> { + ResTableParser tableParser = new ResTableParser(this); + tableParser.decode(is); + return tableParser; }); - if (resStorage != null) { - processResources(resStorage); + if (parser != null) { + processResources(parser.getResStorage()); + updateObfuscatedFiles(parser, resources); } } catch (Exception e) { LOG.error("Failed to parse '.arsc' file", e); @@ -163,6 +166,20 @@ public class RootNode { } } + private void updateObfuscatedFiles(ResTableParser parser, List resources) { + ResourceStorage resStorage = parser.getResStorage(); + ValuesParser valuesParser = new ValuesParser(this, parser.getStrings(), resStorage.getResourcesNames()); + for (int i = 0; i < resources.size(); i++) { + ResourceFile resource = resources.get(i); + for (ResourceEntry ri : parser.getResStorage().getResources()) { + if (resource.getOriginalName().equals(valuesParser.getValueString(ri))) { + resource.setAlias(ri); + break; + } + } + } + } + private void initInnerClasses() { // move inner classes List inner = new ArrayList<>(); diff --git a/jadx-core/src/main/java/jadx/core/xmlgen/ResContainer.java b/jadx-core/src/main/java/jadx/core/xmlgen/ResContainer.java index 836cfc0cb..864b00f2b 100644 --- a/jadx-core/src/main/java/jadx/core/xmlgen/ResContainer.java +++ b/jadx-core/src/main/java/jadx/core/xmlgen/ResContainer.java @@ -30,7 +30,7 @@ public class ResContainer implements Comparable { } public static ResContainer resourceFileLink(ResourceFile resFile) { - return new ResContainer(resFile.getName(), Collections.emptyList(), resFile, DataType.RES_LINK); + return new ResContainer(resFile.getDeobfName(), Collections.emptyList(), resFile, DataType.RES_LINK); } public static ResContainer resourceTable(String name, List subFiles, ICodeInfo rootContent) { diff --git a/jadx-core/src/main/java/jadx/core/xmlgen/ResTableParser.java b/jadx-core/src/main/java/jadx/core/xmlgen/ResTableParser.java index 126969a1e..62831d4e2 100644 --- a/jadx-core/src/main/java/jadx/core/xmlgen/ResTableParser.java +++ b/jadx-core/src/main/java/jadx/core/xmlgen/ResTableParser.java @@ -5,6 +5,7 @@ import java.io.InputStream; import java.util.ArrayList; import java.util.HashSet; import java.util.List; +import java.util.Objects; import java.util.Set; import org.slf4j.Logger; @@ -241,12 +242,12 @@ public class ResTableParser extends CommonBinaryParser { is.checkPos(entriesStart, "Expected entry start"); for (int i = 0; i < entryCount; i++) { if (entryIndexes[i] != NO_ENTRY) { - parseEntry(pkg, id, i, config); + parseEntry(pkg, id, i, config.getQualifiers()); } } } - private void parseEntry(PackageChunk pkg, int typeId, int entryId, EntryConfig config) throws IOException { + private void parseEntry(PackageChunk pkg, int typeId, int entryId, String config) throws IOException { int size = is.readInt16(); int flags = is.readInt16(); int key = is.readInt32(); @@ -256,32 +257,50 @@ public class ResTableParser extends CommonBinaryParser { int resRef = pkg.getId() << 24 | typeId << 16 | entryId; String typeName = pkg.getTypeStrings()[typeId - 1]; - String keyName = pkg.getKeyStrings()[key]; - if (keyName.isEmpty()) { - FieldNode constField = root.getConstValues().getGlobalConstFields().get(resRef); - if (constField != null) { - keyName = constField.getName(); - constField.add(AFlag.DONT_RENAME); - } else { - keyName = "RES_" + resRef; // autogenerate key name - } + String origKeyName = pkg.getKeyStrings()[key]; + ResourceEntry newResEntry = new ResourceEntry(resRef, pkg.getName(), typeName, getResName(resRef, origKeyName), config); + ResourceEntry prevResEntry = resStorage.searchEntryWithSameName(newResEntry); + if (prevResEntry != null) { + newResEntry = newResEntry.copyWithId(); + + // rename also previous entry for consistency + ResourceEntry replaceForPrevEntry = prevResEntry.copyWithId(); + resStorage.replace(prevResEntry, replaceForPrevEntry); + resStorage.addRename(replaceForPrevEntry); + } + if (!Objects.equals(origKeyName, newResEntry.getKeyName())) { + resStorage.addRename(newResEntry); } - ResourceEntry ri = new ResourceEntry(resRef, pkg.getName(), typeName, keyName); - ri.setConfig(config); if ((flags & FLAG_COMPLEX) != 0 || size == 16) { int parentRef = is.readInt32(); int count = is.readInt32(); - ri.setParentRef(parentRef); + newResEntry.setParentRef(parentRef); List values = new ArrayList<>(count); for (int i = 0; i < count; i++) { values.add(parseValueMap()); } - ri.setNamedValues(values); + newResEntry.setNamedValues(values); } else { - ri.setSimpleValue(parseValue()); + newResEntry.setSimpleValue(parseValue()); } - resStorage.add(ri); + resStorage.add(newResEntry); + } + + private String getResName(int resRef, String origKeyName) { + String renamedKey = resStorage.getRename(resRef); + if (renamedKey != null) { + return renamedKey; + } + if (!origKeyName.isEmpty()) { + return origKeyName; + } + FieldNode constField = root.getConstValues().getGlobalConstFields().get(resRef); + if (constField != null) { + constField.add(AFlag.DONT_RENAME); + return constField.getName(); + } + return "RES_" + resRef; // autogenerate key name } private RawNamedValue parseValueMap() throws IOException { diff --git a/jadx-core/src/main/java/jadx/core/xmlgen/ResXmlGen.java b/jadx-core/src/main/java/jadx/core/xmlgen/ResXmlGen.java index 7794ef096..a8fb1bddc 100644 --- a/jadx-core/src/main/java/jadx/core/xmlgen/ResXmlGen.java +++ b/jadx-core/src/main/java/jadx/core/xmlgen/ResXmlGen.java @@ -211,7 +211,7 @@ public class ResXmlGen { private String getFileName(ResourceEntry ri) { StringBuilder sb = new StringBuilder(); - String qualifiers = ri.getConfig().getQualifiers(); + String qualifiers = ri.getConfig(); sb.append("res/values"); if (!qualifiers.isEmpty()) { sb.append(qualifiers); diff --git a/jadx-core/src/main/java/jadx/core/xmlgen/ResourceStorage.java b/jadx-core/src/main/java/jadx/core/xmlgen/ResourceStorage.java index ef162163e..c38bf5be4 100644 --- a/jadx-core/src/main/java/jadx/core/xmlgen/ResourceStorage.java +++ b/jadx-core/src/main/java/jadx/core/xmlgen/ResourceStorage.java @@ -1,39 +1,70 @@ package jadx.core.xmlgen; import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; import java.util.Comparator; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.TreeMap; import jadx.core.xmlgen.entry.ResourceEntry; public class ResourceStorage { + private static final Comparator RES_ENTRY_NAME_COMPARATOR = Comparator + .comparing(ResourceEntry::getConfig) + .thenComparing(ResourceEntry::getTypeName) + .thenComparing(ResourceEntry::getKeyName); private final List list = new ArrayList<>(); private String appPackage; - public Collection getResources() { - return list; + /** + * Names in one config and type must be unique + */ + private final Map uniqNameEntries = new TreeMap<>(RES_ENTRY_NAME_COMPARATOR); + + /** + * Preserve same name for same id across different configs + */ + private final Map renames = new HashMap<>(); + + public void add(ResourceEntry resEntry) { + list.add(resEntry); + uniqNameEntries.put(resEntry, resEntry); } - public void add(ResourceEntry ri) { - list.add(ri); + public void replace(ResourceEntry prevResEntry, ResourceEntry newResEntry) { + int idx = list.indexOf(prevResEntry); + if (idx != -1) { + list.set(idx, newResEntry); + } + // don't remove from unique names so old name stays occupied + } + + public void addRename(ResourceEntry entry) { + addRename(entry.getId(), entry.getKeyName()); + } + + public void addRename(int id, String keyName) { + renames.put(id, keyName); + } + + public String getRename(int id) { + return renames.get(id); + } + + public ResourceEntry searchEntryWithSameName(ResourceEntry resourceEntry) { + return uniqNameEntries.get(resourceEntry); } public void finish() { list.sort(Comparator.comparingInt(ResourceEntry::getId)); + uniqNameEntries.clear(); + renames.clear(); } - public ResourceEntry getByRef(int refId) { - ResourceEntry key = new ResourceEntry(refId); - int index = Collections.binarySearch(list, key, Comparator.comparingInt(ResourceEntry::getId)); - if (index < 0) { - return null; - } - return list.get(index); + public Iterable getResources() { + return list; } public String getAppPackage() { diff --git a/jadx-core/src/main/java/jadx/core/xmlgen/entry/ResourceEntry.java b/jadx-core/src/main/java/jadx/core/xmlgen/entry/ResourceEntry.java index c95ec2278..c9916bee5 100644 --- a/jadx-core/src/main/java/jadx/core/xmlgen/entry/ResourceEntry.java +++ b/jadx-core/src/main/java/jadx/core/xmlgen/entry/ResourceEntry.java @@ -8,21 +8,30 @@ public final class ResourceEntry { private final String pkgName; private final String typeName; private final String keyName; + private final String config; private int parentRef; private RawValue simpleValue; private List namedValues; - private EntryConfig config; - public ResourceEntry(int id, String pkgName, String typeName, String keyName) { + public ResourceEntry(int id, String pkgName, String typeName, String keyName, String config) { this.id = id; this.pkgName = pkgName; this.typeName = typeName; this.keyName = keyName; + this.config = config; } - public ResourceEntry(int id) { - this(id, "", "", ""); + public ResourceEntry copy(String newKeyName) { + ResourceEntry copy = new ResourceEntry(id, pkgName, typeName, newKeyName, config); + copy.parentRef = this.parentRef; + copy.simpleValue = this.simpleValue; + copy.namedValues = this.namedValues; + return copy; + } + + public ResourceEntry copyWithId() { + return copy(keyName + "_RES_" + id); } public int getId() { @@ -41,6 +50,10 @@ public final class ResourceEntry { return keyName; } + public String getConfig() { + return config; + } + public void setParentRef(int parentRef) { this.parentRef = parentRef; } @@ -65,14 +78,6 @@ public final class ResourceEntry { return namedValues; } - public void setConfig(EntryConfig config) { - this.config = config; - } - - public EntryConfig getConfig() { - return config; - } - @Override public String toString() { return " 0x" + Integer.toHexString(id) + " (" + id + ')' + config + " = " + typeName + '.' + keyName; diff --git a/jadx-core/src/test/java/jadx/tests/api/IntegrationTest.java b/jadx-core/src/test/java/jadx/tests/api/IntegrationTest.java index ebc4c005f..44351b16d 100644 --- a/jadx-core/src/test/java/jadx/tests/api/IntegrationTest.java +++ b/jadx-core/src/test/java/jadx/tests/api/IntegrationTest.java @@ -243,7 +243,7 @@ public abstract class IntegrationTest extends TestUtils { Integer id = entry.getKey(); String name = entry.getValue(); String[] parts = name.split("\\."); - resStorage.add(new ResourceEntry(id, "", parts[0], parts[1])); + resStorage.add(new ResourceEntry(id, "", parts[0], parts[1], "")); } root.processResources(resStorage); } diff --git a/jadx-gui/src/main/java/jadx/gui/treemodel/ApkSignature.java b/jadx-gui/src/main/java/jadx/gui/treemodel/ApkSignature.java index 5e280f416..56e2af3e2 100644 --- a/jadx-gui/src/main/java/jadx/gui/treemodel/ApkSignature.java +++ b/jadx-gui/src/main/java/jadx/gui/treemodel/ApkSignature.java @@ -33,7 +33,7 @@ public class ApkSignature extends JNode { public static ApkSignature getApkSignature(JadxWrapper wrapper) { // Only show the ApkSignature node if an AndroidManifest.xml is present. // Without a manifest the Google ApkVerifier refuses to work. - if (wrapper.getResources().stream().noneMatch(r -> "AndroidManifest.xml".equals(r.getName()))) { + if (wrapper.getResources().stream().noneMatch(r -> "AndroidManifest.xml".equals(r.getOriginalName()))) { return null; } File openFile = wrapper.getOpenFile(); diff --git a/jadx-gui/src/main/java/jadx/gui/treemodel/JResource.java b/jadx-gui/src/main/java/jadx/gui/treemodel/JResource.java index 72932a374..761ecd6d3 100644 --- a/jadx-gui/src/main/java/jadx/gui/treemodel/JResource.java +++ b/jadx-gui/src/main/java/jadx/gui/treemodel/JResource.java @@ -218,7 +218,7 @@ public class JResource extends JLoadableNode implements Comparable { return SyntaxConstants.SYNTAX_STYLE_XML; default: - String syntax = getSyntaxByExtension(resFile.getName()); + String syntax = getSyntaxByExtension(resFile.getDeobfName()); if (syntax != null) { return syntax; } diff --git a/jadx-gui/src/main/java/jadx/gui/treemodel/JRoot.java b/jadx-gui/src/main/java/jadx/gui/treemodel/JRoot.java index 706c537d0..4cc1fd329 100644 --- a/jadx-gui/src/main/java/jadx/gui/treemodel/JRoot.java +++ b/jadx-gui/src/main/java/jadx/gui/treemodel/JRoot.java @@ -52,9 +52,9 @@ public class JRoot extends JNode { for (ResourceFile rf : resources) { String rfName; if (rf.getZipRef() != null) { - rfName = rf.getName(); + rfName = rf.getDeobfName(); } else { - rfName = new File(rf.getName()).getName(); + rfName = new File(rf.getDeobfName()).getName(); } String[] parts = new File(rfName).getPath().split(splitPathStr); JResource curRf = root;