diff --git a/.DS_Store b/.DS_Store new file mode 100644 index 000000000..7c11c7830 Binary files /dev/null and b/.DS_Store differ diff --git a/jadx-core/.DS_Store b/jadx-core/.DS_Store new file mode 100644 index 000000000..2ae879b63 Binary files /dev/null and b/jadx-core/.DS_Store differ diff --git a/jadx-core/src/main/java/jadx/api/ResourcesLoader.java b/jadx-core/src/main/java/jadx/api/ResourcesLoader.java index 4f7e39f97..dc21fb3b6 100644 --- a/jadx-core/src/main/java/jadx/api/ResourcesLoader.java +++ b/jadx-core/src/main/java/jadx/api/ResourcesLoader.java @@ -5,6 +5,7 @@ import jadx.core.codegen.CodeWriter; import jadx.core.utils.Utils; import jadx.core.utils.exceptions.JadxException; import jadx.core.utils.files.InputFile; +import jadx.core.utils.files.ZipSecurity; import jadx.core.xmlgen.ResContainer; import jadx.core.xmlgen.ResTableParser; @@ -64,6 +65,10 @@ public final class ResourcesLoader { if (entry == null) { throw new IOException("Zip entry not found: " + zipRef); } + if(!ZipSecurity.isValidZipEntry(entry)) { + return null; + } + inputStream = new BufferedInputStream(zipFile.getInputStream(entry)); result = decoder.decode(entry.getSize(), inputStream); } catch (Exception e) { @@ -129,7 +134,9 @@ public final class ResourcesLoader { Enumeration entries = zip.entries(); while (entries.hasMoreElements()) { ZipEntry entry = entries.nextElement(); - addEntry(list, file, entry); + if(ZipSecurity.isValidZipEntry(entry)) { + addEntry(list, file, entry); + } } } catch (IOException e) { LOG.debug("Not a zip file: {}", file.getAbsolutePath()); diff --git a/jadx-core/src/main/java/jadx/core/clsp/ClsSet.java b/jadx-core/src/main/java/jadx/core/clsp/ClsSet.java index bb9ab61f8..76c190a1b 100644 --- a/jadx-core/src/main/java/jadx/core/clsp/ClsSet.java +++ b/jadx-core/src/main/java/jadx/core/clsp/ClsSet.java @@ -6,6 +6,7 @@ import jadx.core.dex.nodes.RootNode; import jadx.core.utils.exceptions.DecodeException; import jadx.core.utils.exceptions.JadxRuntimeException; import jadx.core.utils.files.FileUtils; +import jadx.core.utils.files.ZipSecurity; import java.io.BufferedOutputStream; import java.io.DataInputStream; @@ -173,7 +174,7 @@ public class ClsSet { try { ZipEntry entry = in.getNextEntry(); while (entry != null) { - if (entry.getName().endsWith(CLST_EXTENSION)) { + if (entry.getName().endsWith(CLST_EXTENSION) && ZipSecurity.isValidZipEntry(entry)) { load(in); } entry = in.getNextEntry(); diff --git a/jadx-core/src/main/java/jadx/core/utils/files/InputFile.java b/jadx-core/src/main/java/jadx/core/utils/files/InputFile.java index b0b29df8e..75aa5ee22 100644 --- a/jadx-core/src/main/java/jadx/core/utils/files/InputFile.java +++ b/jadx-core/src/main/java/jadx/core/utils/files/InputFile.java @@ -88,6 +88,13 @@ public class InputFile { if (entry == null) { break; } + + // security check + if(!ZipSecurity.isValidZipEntry(entry)) { + index++; + continue; + } + InputStream inputStream = zf.getInputStream(entry); try { if (ext.equals(".dex")) { diff --git a/jadx-core/src/main/java/jadx/core/utils/files/ZipSecurity.java b/jadx-core/src/main/java/jadx/core/utils/files/ZipSecurity.java new file mode 100644 index 000000000..13eb9ff8d --- /dev/null +++ b/jadx-core/src/main/java/jadx/core/utils/files/ZipSecurity.java @@ -0,0 +1,47 @@ +package jadx.core.utils.files; + +import java.io.File; +import java.util.zip.ZipEntry; + +public class ZipSecurity { + // 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 boolean isInSubDirectory(File base, File file) { + if (file == null) { + return false; + } + if (file.equals(base)) { + return true; + } + + return isInSubDirectory(base, file.getParentFile()); + } + + // checks that entry name contains no any traversals + // and prevents cases like "../classes.dex", to limit output only to the specified directory + public static boolean isValidZipEntryName(String entryName) { + try { + File currentPath = new File(".").getCanonicalFile(); + File canonical = new File(currentPath, entryName).getCanonicalFile(); + return isInSubDirectory(currentPath, canonical); + } + catch(Exception e) { + return false; + } + } + + public static boolean isZipBomb(ZipEntry entry) { + long compressedSize = entry.getCompressedSize(); + long uncompressedSize = entry.getSize(); + if(compressedSize < 0 || uncompressedSize < 0) { + return true; + } + return compressedSize * MAX_SIZE_DIFF < uncompressedSize; + } + + public static boolean isValidZipEntry(ZipEntry entry) { + return isValidZipEntryName(entry.getName()) + && !isZipBomb(entry); + } +} diff --git a/jadx-core/src/main/java/jadx/core/xmlgen/ManifestAttributes.java b/jadx-core/src/main/java/jadx/core/xmlgen/ManifestAttributes.java index 7709442c0..7c3886ff5 100644 --- a/jadx-core/src/main/java/jadx/core/xmlgen/ManifestAttributes.java +++ b/jadx-core/src/main/java/jadx/core/xmlgen/ManifestAttributes.java @@ -3,7 +3,6 @@ package jadx.core.xmlgen; import jadx.core.utils.exceptions.JadxException; import javax.xml.parsers.DocumentBuilder; -import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; import java.io.IOException; import java.io.InputStream; @@ -72,7 +71,7 @@ public class ManifestAttributes { if (xmlStream == null) { throw new JadxException(xml + " not found in classpath"); } - DocumentBuilder dBuilder = DocumentBuilderFactory.newInstance().newDocumentBuilder(); + DocumentBuilder dBuilder = XmlSecurity.getSecureDbf().newDocumentBuilder(); doc = dBuilder.parse(xmlStream); } finally { close(xmlStream); diff --git a/jadx-core/src/main/java/jadx/core/xmlgen/XmlSecurity.java b/jadx-core/src/main/java/jadx/core/xmlgen/XmlSecurity.java new file mode 100644 index 000000000..485cdaa9d --- /dev/null +++ b/jadx-core/src/main/java/jadx/core/xmlgen/XmlSecurity.java @@ -0,0 +1,26 @@ +package jadx.core.xmlgen; + +import javax.xml.parsers.DocumentBuilderFactory; +import javax.xml.parsers.ParserConfigurationException; + +public class XmlSecurity { + private static DocumentBuilderFactory secureDbf = null; + + public static DocumentBuilderFactory getSecureDbf() throws ParserConfigurationException { + synchronized(XmlSecurity.class) { + if(secureDbf == null) { + secureDbf = DocumentBuilderFactory.newInstance(); + secureDbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true); + secureDbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + secureDbf.setFeature("http://xml.org/sax/features/external-general-entities", false); + secureDbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false); + secureDbf.setFeature("http://apache.org/xml/features/dom/create-entity-ref-nodes", false); + secureDbf.setXIncludeAware(false); + secureDbf.setExpandEntityReferences(false); + } + } + return secureDbf; + } + + +}