Commit 7d3caa28 authored by Sergey Toshin's avatar Sergey Toshin

Adds checks for resources, and logs detected attacks

parent 418546a6
package jadx.api; package jadx.api;
import jadx.core.utils.files.ZipSecurity;
import jadx.core.xmlgen.ResContainer; import jadx.core.xmlgen.ResContainer;
import java.io.File; import java.io.File;
...@@ -34,7 +35,7 @@ public class ResourceFile { ...@@ -34,7 +35,7 @@ public class ResourceFile {
private final ResourceType type; private final ResourceType type;
private ZipRef zipRef; private ZipRef zipRef;
ResourceFile(JadxDecompiler decompiler, String name, ResourceType type) { protected ResourceFile(JadxDecompiler decompiler, String name, ResourceType type) {
this.decompiler = decompiler; this.decompiler = decompiler;
this.name = name; this.name = name;
this.type = type; this.type = type;
...@@ -64,4 +65,11 @@ public class ResourceFile { ...@@ -64,4 +65,11 @@ public class ResourceFile {
public String toString() { public String toString() {
return "ResourceFile{name='" + name + '\'' + ", type=" + type + "}"; return "ResourceFile{name='" + name + '\'' + ", type=" + type + "}";
} }
public static ResourceFile createResourceFileInstance(JadxDecompiler decompiler, String name, ResourceType type) {
if(!ZipSecurity.isValidZipEntryName(name)) {
return null;
}
return new ResourceFile(decompiler, name, type);
}
} }
package jadx.api; package jadx.api;
import jadx.core.codegen.CodeWriter; import jadx.core.codegen.CodeWriter;
import jadx.core.utils.files.ZipSecurity;
import jadx.core.xmlgen.ResContainer; import jadx.core.xmlgen.ResContainer;
public class ResourceFileContent extends ResourceFile { public class ResourceFileContent extends ResourceFile {
private final CodeWriter content; private final CodeWriter content;
public ResourceFileContent(String name, ResourceType type, CodeWriter content) { private ResourceFileContent(String name, ResourceType type, CodeWriter content) {
super(null, name, type); super(null, name, type);
this.content = content; this.content = content;
} }
...@@ -16,4 +17,11 @@ public class ResourceFileContent extends ResourceFile { ...@@ -16,4 +17,11 @@ public class ResourceFileContent extends ResourceFile {
public ResContainer loadContent() { public ResContainer loadContent() {
return ResContainer.singleFile(getName(), content); return ResContainer.singleFile(getName(), content);
} }
public static ResourceFileContent createResourceFileContentInstance(String name, ResourceType type, CodeWriter content) {
if(!ZipSecurity.isValidZipEntryName(name)) {
return null;
}
return new ResourceFileContent(name, type, content);
}
} }
...@@ -157,8 +157,10 @@ public final class ResourcesLoader { ...@@ -157,8 +157,10 @@ public final class ResourcesLoader {
private void addResourceFile(List<ResourceFile> list, File file) { private void addResourceFile(List<ResourceFile> list, File file) {
String name = file.getAbsolutePath(); String name = file.getAbsolutePath();
ResourceType type = ResourceType.getFileType(name); ResourceType type = ResourceType.getFileType(name);
ResourceFile rf = new ResourceFile(jadxRef, name, type); ResourceFile rf = ResourceFile.createResourceFileInstance(jadxRef, name, type);
list.add(rf); if(rf != null) {
list.add(rf);
}
} }
private void addEntry(List<ResourceFile> list, File zipFile, ZipEntry entry) { private void addEntry(List<ResourceFile> list, File zipFile, ZipEntry entry) {
...@@ -167,9 +169,11 @@ public final class ResourcesLoader { ...@@ -167,9 +169,11 @@ public final class ResourcesLoader {
} }
String name = entry.getName(); String name = entry.getName();
ResourceType type = ResourceType.getFileType(name); ResourceType type = ResourceType.getFileType(name);
ResourceFile rf = new ResourceFile(jadxRef, name, type); ResourceFile rf = ResourceFile.createResourceFileInstance(jadxRef, name, type);
rf.setZipRef(new ZipRef(zipFile, name)); if(rf != null) {
list.add(rf); rf.setZipRef(new ZipRef(zipFile, name));
list.add(rf);
}
} }
public static CodeWriter loadToCodeWriter(InputStream is) throws IOException { public static CodeWriter loadToCodeWriter(InputStream is) throws IOException {
......
...@@ -3,7 +3,12 @@ package jadx.core.utils.files; ...@@ -3,7 +3,12 @@ package jadx.core.utils.files;
import java.io.File; import java.io.File;
import java.util.zip.ZipEntry; import java.util.zip.ZipEntry;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
public class ZipSecurity { public class ZipSecurity {
private static final Logger LOG = LoggerFactory.getLogger(ZipSecurity.class);
// size of uncompressed zip entry shouldn't be bigger of compressed in MAX_SIZE_DIFF times // size of uncompressed zip entry shouldn't be bigger of compressed in MAX_SIZE_DIFF times
private static final int MAX_SIZE_DIFF = 5; private static final int MAX_SIZE_DIFF = 5;
...@@ -24,9 +29,14 @@ public class ZipSecurity { ...@@ -24,9 +29,14 @@ public class ZipSecurity {
try { try {
File currentPath = new File(".").getCanonicalFile(); File currentPath = new File(".").getCanonicalFile();
File canonical = new File(currentPath, entryName).getCanonicalFile(); File canonical = new File(currentPath, entryName).getCanonicalFile();
return isInSubDirectory(currentPath, canonical); if(isInSubDirectory(currentPath, canonical)) {
return true;
}
LOG.debug("Path traversal attack detected, invalid name: {}", entryName);
return false;
} }
catch(Exception e) { catch(Exception e) {
LOG.debug("Path traversal attack detected, invalid name: {}", entryName);
return false; return false;
} }
} }
...@@ -37,7 +47,11 @@ public class ZipSecurity { ...@@ -37,7 +47,11 @@ public class ZipSecurity {
if(compressedSize < 0 || uncompressedSize < 0) { if(compressedSize < 0 || uncompressedSize < 0) {
return true; return true;
} }
return compressedSize * MAX_SIZE_DIFF < uncompressedSize; if(compressedSize * MAX_SIZE_DIFF < uncompressedSize) {
LOG.debug("Zip bomp attack detected, invalid sizes: compressed {}, uncompressed {}", compressedSize, uncompressedSize);
return true;
}
return false;
} }
public static boolean isValidZipEntry(ZipEntry entry) { public static boolean isValidZipEntry(ZipEntry entry) {
......
...@@ -101,8 +101,10 @@ public class JResource extends JNode implements Comparable<JResource> { ...@@ -101,8 +101,10 @@ public class JResource extends JNode implements Comparable<JResource> {
String resName = rc.getName(); String resName = rc.getName();
String[] path = resName.split("/"); String[] path = resName.split("/");
String resShortName = path.length == 0 ? resName : path[path.length - 1]; String resShortName = path.length == 0 ? resName : path[path.length - 1];
ResourceFileContent fileContent = new ResourceFileContent(resShortName, ResourceType.XML, cw); ResourceFileContent fileContent = ResourceFileContent.createResourceFileContentInstance(resShortName, ResourceType.XML, cw);
addPath(path, root, new JResource(fileContent, resName, resShortName, JResType.FILE)); if(fileContent != null) {
addPath(path, root, new JResource(fileContent, resName, resShortName, JResType.FILE));
}
} }
} }
List<ResContainer> subFiles = rc.getSubFiles(); List<ResContainer> subFiles = rc.getSubFiles();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment