fix(res): ignore resource table entries with '../' in name

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() {

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();

jadx-core/src/main/java/jadx/core/dex/nodes/RootNode.java

@@ -275,8 +275,9 @@ private void updateObfuscatedFiles(IResParser parser, List<ResourceFile> 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()) {

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) {

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<ResourceEntry> getResources() {
  return list;
  }