diff --git a/jadx-core/src/main/java/jadx/api/ResourceFile.java b/jadx-core/src/main/java/jadx/api/ResourceFile.java index af53a8a19d9..927c61f84ba 100644 --- a/jadx-core/src/main/java/jadx/api/ResourceFile.java +++ b/jadx-core/src/main/java/jadx/api/ResourceFile.java @@ -74,13 +74,20 @@ void setZipRef(ZipRef zipRef) { this.zipRef = zipRef; } - public void setAlias(ResourceEntry ri) { - int index = name.lastIndexOf('.'); - deobfName = String.format("res/%s%s/%s%s", - ri.getTypeName(), - ri.getConfig(), - ri.getKeyName(), - index == -1 ? "" : name.substring(index)); + public boolean setAlias(ResourceEntry ri) { + StringBuilder sb = new StringBuilder(); + sb.append("res/").append(ri.getTypeName()).append(ri.getConfig()); + sb.append("/").append(ri.getKeyName()); + int lastDot = name.lastIndexOf('.'); + if (lastDot != -1) { + sb.append(name.substring(lastDot)); + } + String alias = sb.toString(); + if (!alias.equals(name)) { + deobfName = alias; + return true; + } + return false; } public ZipRef getZipRef() { diff --git a/jadx-core/src/main/java/jadx/api/plugins/utils/ZipSecurity.java b/jadx-core/src/main/java/jadx/api/plugins/utils/ZipSecurity.java index bdea2028fdb..5522d45f341 100644 --- a/jadx-core/src/main/java/jadx/api/plugins/utils/ZipSecurity.java +++ b/jadx-core/src/main/java/jadx/api/plugins/utils/ZipSecurity.java @@ -71,6 +71,12 @@ public static boolean isValidZipEntryName(String entryName) { if (DISABLE_CHECKS) { return true; } + if (entryName.contains("..")) { // quick pre-check + if (entryName.contains("../") || entryName.contains("..\\")) { + LOG.error("Path traversal attack detected in entry: '{}'", entryName); + return false; + } + } try { File currentPath = CommonFileUtils.CWD; File canonical = new File(currentPath, entryName).getCanonicalFile(); 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 66eea3948dd..6cb41d3de1a 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 @@ -275,8 +275,9 @@ private void updateObfuscatedFiles(IResParser parser, List resourc for (ResourceFile resource : resources) { ResourceEntry resEntry = entryNames.get(resource.getOriginalName()); if (resEntry != null) { - resource.setAlias(resEntry); - renamedCount++; + if (resource.setAlias(resEntry)) { + renamedCount++; + } } } if (LOG.isDebugEnabled()) { 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 c16e44579de..d384020e56e 100644 --- a/jadx-core/src/main/java/jadx/core/xmlgen/ResTableParser.java +++ b/jadx-core/src/main/java/jadx/core/xmlgen/ResTableParser.java @@ -18,6 +18,7 @@ import jadx.api.ICodeInfo; import jadx.api.args.ResourceNameSource; +import jadx.api.plugins.utils.ZipSecurity; import jadx.core.deobf.NameMapper; import jadx.core.dex.attributes.AFlag; import jadx.core.dex.nodes.FieldNode; @@ -84,9 +85,14 @@ public ResTableParser(RootNode root, boolean useRawResNames) { @Override public void decode(InputStream inputStream) throws IOException { + long start = System.currentTimeMillis(); is = new ParserStream(inputStream); decodeTableChunk(); resStorage.finish(); + if (LOG.isDebugEnabled()) { + LOG.debug("Resource table parsed: size: {}, time: {}ms", + resStorage.size(), System.currentTimeMillis() - start); + } } public ResContainer decodeFiles(InputStream inputStream) throws IOException { @@ -356,20 +362,8 @@ private void parseEntry(PackageChunk pkg, int typeId, int entryId, String config int resRef = pkg.getId() << 24 | typeId << 16 | entryId; String typeName = pkg.getTypeStrings().get(typeId - 1); String origKeyName = pkg.getKeyStrings().get(key); - ResourceEntry newResEntry = new ResourceEntry(resRef, pkg.getName(), typeName, getResName(typeName, 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 newResEntry = buildResourceEntry(pkg, config, resRef, typeName, origKeyName); if ((flags & FLAG_COMPLEX) != 0 || size == 16) { int parentRef = is.readInt32(); int count = is.readInt32(); @@ -382,7 +376,33 @@ private void parseEntry(PackageChunk pkg, int typeId, int entryId, String config } else { newResEntry.setSimpleValue(parseValue()); } + } + + private static final ResourceEntry STUB_ENTRY = new ResourceEntry(-1, "stub", "stub", "stub", ""); + + private ResourceEntry buildResourceEntry(PackageChunk pkg, String config, int resRef, String typeName, String origKeyName) { + if (!ZipSecurity.isValidZipEntryName(origKeyName)) { + // malicious entry, ignore it + // can't return null here, return stub without adding it to storage + return STUB_ENTRY; + } + + String resName = getResName(typeName, resRef, origKeyName); + ResourceEntry newResEntry = new ResourceEntry(resRef, pkg.getName(), typeName, resName, 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); + } resStorage.add(newResEntry); + return newResEntry; } private String getResName(String typeName, int resRef, String origKeyName) { 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 c38bf5be4cb..5c7a2d23ea2 100644 --- a/jadx-core/src/main/java/jadx/core/xmlgen/ResourceStorage.java +++ b/jadx-core/src/main/java/jadx/core/xmlgen/ResourceStorage.java @@ -63,6 +63,10 @@ public void finish() { renames.clear(); } + public int size() { + return list.size(); + } + public Iterable getResources() { return list; }