From 528ca09e8ef8d11300a299c7d31a202e08aa5cd6 Mon Sep 17 00:00:00 2001 From: Sergey Toshin Date: Sun, 31 Dec 2017 01:51:25 +0300 Subject: [PATCH] Fixes for ZIP and XML processors --- .DS_Store | Bin 0 -> 10244 bytes jadx-core/.DS_Store | Bin 0 -> 12292 bytes .../main/java/jadx/api/ResourcesLoader.java | 9 +++- .../src/main/java/jadx/core/clsp/ClsSet.java | 3 +- .../java/jadx/core/utils/files/InputFile.java | 7 +++ .../jadx/core/utils/files/ZipSecurity.java | 47 ++++++++++++++++++ .../jadx/core/xmlgen/ManifestAttributes.java | 3 +- .../java/jadx/core/xmlgen/XmlSecurity.java | 26 ++++++++++ 8 files changed, 91 insertions(+), 4 deletions(-) create mode 100644 .DS_Store create mode 100644 jadx-core/.DS_Store create mode 100644 jadx-core/src/main/java/jadx/core/utils/files/ZipSecurity.java create mode 100644 jadx-core/src/main/java/jadx/core/xmlgen/XmlSecurity.java diff --git a/.DS_Store b/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..7c11c7830538270df87fa9f0bb7dc49941779d2d GIT binary patch literal 10244 zcmeI1Yitx%6vxjg1?CQgPHWk=z`{zct*}5}6p%-@51>RI)wcA(qwMYsbYMGEc4pf` zDQVOgqo9cjMpTTNNC3Y8^1&F5#>8i$Mw{@6#%R-Fxqu--Uc%T`}FVOG++a+Jy9plO{`yNo*?H z*3e=0G}vJ~VO59YTliId*zSn9`9{->4d=s=&AQgpuH@91h8@-oE$%edRgG@h+N>L@ z*;8#M4ApXer$JOW9knZYeSKx+i-Up6ivD1rue7XWX)v&?vb?{aNx6lkE7$Gl+_%5) zK>xw##H`SLGeG93SsqKCV$ABU5nB{#@u4YR{CkR1d~0&n*0n22!qOhlVqshF)@sAH zX0XZf^OETVzCB&zh<@ATCdG>ZI&6gYn&Tf zbZX1X+LdhHep8pNn})Sni(6zFJDo9o#+-uU1!YTCR1Hp@mZ`98mebs=TY4m>HTCGK z-O(DhqMD)V#;!Kxb`ofrCwm=cy6EgG>Xna6@mu~_3CdT^qv zMXXd(RMr#=N3@tIs$i9pavy8%7Sj@i%h+-$3xljBYIb(gxGksWm8*Qd?Y!spVawjs zr5QA;kEnY--f9|EE3d0kRX!%EAXF?KY(c0*WE3tM`4EIvum;*73I_DRAvg?Az%h6g zPQw{^3qF7k;S=~2zJN<`8NP;Z;3xP6{(!6SCjv5@jy}xA0N#lOco*J-Wmt}ju@aYK zEw0A3Sce<28C$Row_p@w*oj@Z7x&{s_!K^aNAM`VgeUP7p2pYlJia4FmNvxVI&nRe z;=7a^5pKkyyIq#noQg$1u~>9dSiF?M0nEz2BWL>Tg5qFlU34y9FH!pw2Oy+MQn6%!}cn@K(Oy<}~gZa66K7YVJS0X?Zamfn(1n7B5K~Y=i zpD#&D+93bo1q=O5nLzY1=d!Gv`IU(Ts^nZQS<0A_P9%%k6)UNB5-}@ktE#CMcucVr z&|MR`m*Et=Nw|Fp7YMTN;R<2)J6y$doJNS{;Y^%`e!Lrt2(oz?bRb)b%Usab5NP*f z12*Ai!fgjeP(=-QqDin>Xk!8o;s8EK*gcGg@kxU3X$QaK0V{Ag7EdGt*WC~| zVy3Z+N;#qoc0(9c=!FFoJ|@IL8?l3XODI6;by9#7QypIkWUd5q-H4a=|9$_b4CE9_ zF#LiWs|a$od_5IGPKw=sQUrO7BFOXPF^V9MRRlSeBFNWI5tyq4bD;uOKs~e)(p(9S zz)^UGbl@BX=KJs&d``jn6Cjly#hPDTl{C^%>194w~roKN9diuY1z zuEbSXgX?hvZo+yB&2}d=qqxTj$zI$?A^A8S!x!*HJdP*u44%bv_y)fHuaqD)ljA6Y zJcSfNF1o2q-Y8{||GP5C<12%lqzrPhGRSh0GEnVEWuV&8%D@~&NQK@>I1BF)c9-E> zg6#_YO1J^xmO*+jn?5=C2S*_;#6<*K8CE#3g|Ny6ZX*HLN*^8Da64hwfxDfL4+DD$ zx;{LB{rCtzijNU~hv+lpS%UC+2gBFIn}1wINCDm`We`8&rJe^o58SREm>?z=;QRlp zul@i3?K*vLUY-X$50D2iy|$sYnrE)z4!+kOpmPVE{NVQu2ukr<1>aY6 r_G_rj-L7%kg{Yjj9}t#ublmVi1HARWc!(*!1HJXXyV-r4umAr7tV`X7 literal 0 HcmV?d00001 diff --git a/jadx-core/.DS_Store b/jadx-core/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..2ae879b63a6d75356f99cc05105929f502eff189 GIT binary patch literal 12292 zcmeHNYj7J^6+XwW>|H1E+BmW#TaJjU#&PV#b{xlUY$HG7#58F_?8MHabtG#WFUTuL zE7?hmiy{5NFl`Eywlq*$Xa}ZE8HR$(05cSZ8K4hdB}~&onE|FSzz_HbWoY5tdsng~ z%b7qsfeE`ayLb2AJ=%M&&e`*Q=PUqVUoz4HP!9kK?Yz}i0~DzM{!+Tg*!Vc#mu-dA zgS3+z(76U+KolZTYoA$fsKtVh_9W|^( zbMs{^C@d;2S*0*m>Kd%vy=Pq0l9ql@kA@P)j6M)b=+?MaW=+OshbCg?fw(?n=(D2} zv1oG2q>=K8P;??mH|xF8=&*5Cx5g6s=>$JJYQ^G-esSZNL9)DWT&v*n`==8|%(M>c zaf@2UF0NkZU0+w<)Viaq_tKiRWg4qsm7_C;WrU;pp;;r6m^>1)CUi4mm?w`K5#3A} zw;Ot#l^0C15i>NU3n^EX%cE#@%2+aNoJtyrGncfrWoxQyTq1TF4t}R$$LXqV*j*m(yV?!kLUC4qgfr{ zW4)0GpOaD->VnPkL_>OUd!P||U=WVN1eh=jcfs9o9xlM6a1p)?--PeM_u)Bs z9)1EZ!z=JhcolvJe};d+oA6HrWVjApScQIkC)VK&xCvWv8*ayT+=&CY3wPrlyah*b z43FY5oWLkf;WVDYvv>|aj-SB$@P2$4AIB%~B0hcE>wsV-9LX52?g?I)#9V3?GOkWw>vhLoDaJMmtj)C2g4t<)Fs zY5ba0U}d#vypUYX0F%PwbT}&9YjnALsiodsmFn)i4)E3;EW>MQLiT^4 z<3gPJ{q}XxZWm-L#Q~)_p#5aD845~qK)<*hFeqE+=&XRV0?SvRgu1e^{}sRqEAcfR6~lqN1T34fr-IidJxm|8Ub5 zAJYnnEiiG}x{dj?Vj>2LxZKgfm{vs0L5_BH(`X5C1UcH?TYYrzuiKNvRiXY=w2SSO-uzfxfi$&At zq3~g1ljB-tf6PpT3{#H_VIqih%kfeAzhkp~v7{NX#P1Bt0phv}8`s=(bFD4g8~yEV z^Ns$wme%HuMt^7fw)uIcR0UePhfYkLIXn0M`8ytzilQB$piEw;j~ABIlNmBW0Uk)H zx5Tj3QqR!4)x8;H_^tBlybha(VW;^OXWG6EH*Rj;e$%@K1VL3R)PIv{lhtb;ucs#E zIJpkBoPpP__o!LEn`Yp3>ph-KN46QbTJLGdX5ea*x;2}Dds@}D90u;$p>{4~;GQnE zCl3P;_If-yuNTR{$^CcHtnxEe@j_uX{oVj=&;b=mlGRM5S^h91j77?r5?sQF)>YK7mSzY7K!>V{P#ZQ&COh9Z4+z@c9 z0vn5L1;24qgQ|*ziO5j!9zv{cnMe^i4Bkbk)tySHh|~q|B<$)=V{w@{;oaRmZg)Ob z#mAib-!-ZK<+H;qBDGPl|MZ`i^LJ(Oqf8^ZBCARhsgwohTDHu+2_CMkl6U*PsT>>6 zqp1NeCBqhFV$2)xHl#CbYLl0!e@M{^rJT+CwPJCp66fcM0j)@ma<)%as+GvmUbSC* zB%r|(jk=0*GpF(2pi(oZ@4uol^Pi~1{5m48!g8va%M@EJCD`hzz`Plo1c~pYGBYRf zymrpX)G&@vij9*gUTW3_nL34WO0jV=wSXU_6kCeW^Ay`dcr__D`< zzVZ7OWGh7IV3*^?m?*_lpZyh@=nK;ATy8NNf JyuR}P{{pLt0-68- literal 0 HcmV?d00001 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; + } + + +}