From ddd8fdc8ad0583eb4a6172dc86c72c881485c55b Mon Sep 17 00:00:00 2001 From: Srikanth Reddy Lingala Date: Wed, 25 Jan 2023 16:37:20 -0500 Subject: [PATCH] #485 Calculate AES mac with cache and push back functionality --- .../lingala/zip4j/crypto/AESDecrypter.java | 4 +- .../zip4j/crypto/PBKDF2/MacBasedPRF.java | 41 +++++++++++++++--- .../io/inputstream/AesCipherInputStream.java | 8 ++-- .../io/inputstream/CipherInputStream.java | 2 +- .../inputstream/DecompressedInputStream.java | 7 +-- .../io/inputstream/InflaterInputStream.java | 7 +-- .../zip4j/io/inputstream/ZipInputStream.java | 4 +- .../java/net/lingala/zip4j/MiscZipFileIT.java | 10 +++++ .../io/inputstream/ZipInputStreamIT.java | 11 ++++- .../io/outputstream/ZipOutputStreamIT.java | 39 +++++++++++++++++ ...with_extra_data_record_and_corrupt_mac.zip | Bin 0 -> 4320 bytes 11 files changed, 110 insertions(+), 23 deletions(-) create mode 100644 src/test/resources/test-archives/aes_with_extra_data_record_and_corrupt_mac.zip diff --git a/src/main/java/net/lingala/zip4j/crypto/AESDecrypter.java b/src/main/java/net/lingala/zip4j/crypto/AESDecrypter.java index accb35ca..f2cf81bd 100755 --- a/src/main/java/net/lingala/zip4j/crypto/AESDecrypter.java +++ b/src/main/java/net/lingala/zip4j/crypto/AESDecrypter.java @@ -86,7 +86,7 @@ public int decryptData(byte[] buff, int start, int len) throws ZipException { return len; } - public byte[] getCalculatedAuthenticationBytes() { - return mac.doFinal(); + public byte[] getCalculatedAuthenticationBytes(int numberOfBytesPushedBack) { + return mac.doFinal(numberOfBytesPushedBack); } } diff --git a/src/main/java/net/lingala/zip4j/crypto/PBKDF2/MacBasedPRF.java b/src/main/java/net/lingala/zip4j/crypto/PBKDF2/MacBasedPRF.java index 31284337..ed1e84c6 100755 --- a/src/main/java/net/lingala/zip4j/crypto/PBKDF2/MacBasedPRF.java +++ b/src/main/java/net/lingala/zip4j/crypto/PBKDF2/MacBasedPRF.java @@ -16,11 +16,16 @@ package net.lingala.zip4j.crypto.PBKDF2; +import net.lingala.zip4j.util.InternalZipConstants; + import javax.crypto.Mac; import javax.crypto.spec.SecretKeySpec; +import java.io.ByteArrayOutputStream; import java.security.InvalidKeyException; import java.security.NoSuchAlgorithmException; +import static net.lingala.zip4j.util.InternalZipConstants.AES_BLOCK_SIZE; + /* * Source referred from Matthias Gartner's PKCS#5 implementation - * see http://rtner.de/software/PBKDF2.html @@ -30,9 +35,11 @@ public class MacBasedPRF implements PRF { private Mac mac; private int hLen; private String macAlgorithm; + private ByteArrayOutputStream macCache; public MacBasedPRF(String macAlgorithm) { this.macAlgorithm = macAlgorithm; + this.macCache = new ByteArrayOutputStream(InternalZipConstants.BUFF_SIZE); try { mac = Mac.getInstance(macAlgorithm); hLen = mac.getMacLength(); @@ -42,10 +49,20 @@ public MacBasedPRF(String macAlgorithm) { } public byte[] doFinal(byte[] M) { + if (macCache.size() > 0) { + doMacUpdate(0); + } return mac.doFinal(M); } public byte[] doFinal() { + return doFinal(0); + } + + public byte[] doFinal(int numberOfBytesToPushbackForMac) { + if (macCache.size() > 0) { + doMacUpdate(numberOfBytesToPushbackForMac); + } return mac.doFinal(); } @@ -61,19 +78,29 @@ public void init(byte[] P) { } } - public void update(byte[] U) { + public void update(byte[] u) { + update(u, 0, u.length); + } + + public void update(byte[] u, int start, int len) { try { - mac.update(U); + if (macCache.size() + len > InternalZipConstants.BUFF_SIZE) { + doMacUpdate(0); + } + macCache.write(u, start, len); } catch (IllegalStateException e) { throw new RuntimeException(e); } } - public void update(byte[] U, int start, int len) { - try { - mac.update(U, start, len); - } catch (IllegalStateException e) { - throw new RuntimeException(e); + private void doMacUpdate(int numberOfBytesToPushBack) { + byte[] macBytes = macCache.toByteArray(); + int numberOfBytesToRead = macBytes.length - numberOfBytesToPushBack; + int updateLength; + for (int i = 0; i < numberOfBytesToRead; i += InternalZipConstants.AES_BLOCK_SIZE) { + updateLength = (i + AES_BLOCK_SIZE) <= numberOfBytesToRead ? AES_BLOCK_SIZE : numberOfBytesToRead - i; + mac.update(macBytes, i, updateLength); } + macCache.reset(); } } diff --git a/src/main/java/net/lingala/zip4j/io/inputstream/AesCipherInputStream.java b/src/main/java/net/lingala/zip4j/io/inputstream/AesCipherInputStream.java index a58179f9..b1ab68b2 100644 --- a/src/main/java/net/lingala/zip4j/io/inputstream/AesCipherInputStream.java +++ b/src/main/java/net/lingala/zip4j/io/inputstream/AesCipherInputStream.java @@ -117,12 +117,12 @@ private void copyBytesFromBuffer(byte[] b, int off) { } @Override - protected void endOfEntryReached(InputStream inputStream) throws IOException { - verifyContent(readStoredMac(inputStream)); + protected void endOfEntryReached(InputStream inputStream, int numberOfBytesPushedBack) throws IOException { + verifyContent(readStoredMac(inputStream), numberOfBytesPushedBack); } - private void verifyContent(byte[] storedMac) throws IOException { - byte[] calculatedMac = getDecrypter().getCalculatedAuthenticationBytes(); + private void verifyContent(byte[] storedMac, int numberOfBytesPushedBack) throws IOException { + byte[] calculatedMac = getDecrypter().getCalculatedAuthenticationBytes(numberOfBytesPushedBack); byte[] first10BytesOfCalculatedMac = new byte[AES_AUTH_LENGTH]; System.arraycopy(calculatedMac, 0, first10BytesOfCalculatedMac, 0, InternalZipConstants.AES_AUTH_LENGTH); diff --git a/src/main/java/net/lingala/zip4j/io/inputstream/CipherInputStream.java b/src/main/java/net/lingala/zip4j/io/inputstream/CipherInputStream.java index 78caab65..5023a94e 100644 --- a/src/main/java/net/lingala/zip4j/io/inputstream/CipherInputStream.java +++ b/src/main/java/net/lingala/zip4j/io/inputstream/CipherInputStream.java @@ -80,7 +80,7 @@ public T getDecrypter() { return decrypter; } - protected void endOfEntryReached(InputStream inputStream) throws IOException { + protected void endOfEntryReached(InputStream inputStream, int numberOfBytesPushedBack) throws IOException { // is optional but useful for AES } diff --git a/src/main/java/net/lingala/zip4j/io/inputstream/DecompressedInputStream.java b/src/main/java/net/lingala/zip4j/io/inputstream/DecompressedInputStream.java index 2c0da223..f4a22029 100644 --- a/src/main/java/net/lingala/zip4j/io/inputstream/DecompressedInputStream.java +++ b/src/main/java/net/lingala/zip4j/io/inputstream/DecompressedInputStream.java @@ -39,12 +39,13 @@ public void close() throws IOException { cipherInputStream.close(); } - public void endOfEntryReached(InputStream inputStream) throws IOException { - cipherInputStream.endOfEntryReached(inputStream); + public void endOfEntryReached(InputStream inputStream, int numberOfBytesPushedBack) throws IOException { + cipherInputStream.endOfEntryReached(inputStream, numberOfBytesPushedBack); } - public void pushBackInputStreamIfNecessary(PushbackInputStream pushbackInputStream) throws IOException { + public int pushBackInputStreamIfNecessary(PushbackInputStream pushbackInputStream) throws IOException { // Do nothing by default + return 0; } protected byte[] getLastReadRawDataCache() { diff --git a/src/main/java/net/lingala/zip4j/io/inputstream/InflaterInputStream.java b/src/main/java/net/lingala/zip4j/io/inputstream/InflaterInputStream.java index df434d0e..572797e5 100644 --- a/src/main/java/net/lingala/zip4j/io/inputstream/InflaterInputStream.java +++ b/src/main/java/net/lingala/zip4j/io/inputstream/InflaterInputStream.java @@ -55,21 +55,22 @@ public int read(byte[] b, int off, int len) throws IOException { } @Override - public void endOfEntryReached(InputStream inputStream) throws IOException { + public void endOfEntryReached(InputStream inputStream, int numberOfBytesPushedBack) throws IOException { if (inflater != null) { inflater.end(); inflater = null; } - super.endOfEntryReached(inputStream); + super.endOfEntryReached(inputStream, numberOfBytesPushedBack); } @Override - public void pushBackInputStreamIfNecessary(PushbackInputStream pushbackInputStream) throws IOException { + public int pushBackInputStreamIfNecessary(PushbackInputStream pushbackInputStream) throws IOException { int n = inflater.getRemaining(); if (n > 0) { byte[] rawDataCache = getLastReadRawDataCache(); pushbackInputStream.unread(rawDataCache, len - n, n); } + return n; } @Override diff --git a/src/main/java/net/lingala/zip4j/io/inputstream/ZipInputStream.java b/src/main/java/net/lingala/zip4j/io/inputstream/ZipInputStream.java index a11d3a63..46acc8cf 100755 --- a/src/main/java/net/lingala/zip4j/io/inputstream/ZipInputStream.java +++ b/src/main/java/net/lingala/zip4j/io/inputstream/ZipInputStream.java @@ -231,10 +231,10 @@ public void setPassword(char[] password) { private void endOfCompressedDataReached() throws IOException { //With inflater, without knowing the compressed or uncompressed size, we over read necessary data //In such cases, we have to push back the inputstream to the end of data - decompressedInputStream.pushBackInputStreamIfNecessary(inputStream); + int numberOfBytesPushedBack = decompressedInputStream.pushBackInputStreamIfNecessary(inputStream); //First signal the end of data for this entry so that ciphers can read any header data if applicable - decompressedInputStream.endOfEntryReached(inputStream); + decompressedInputStream.endOfEntryReached(inputStream, numberOfBytesPushedBack); readExtendedLocalFileHeaderIfPresent(); verifyCrc(); diff --git a/src/test/java/net/lingala/zip4j/MiscZipFileIT.java b/src/test/java/net/lingala/zip4j/MiscZipFileIT.java index f113511d..8b8505df 100644 --- a/src/test/java/net/lingala/zip4j/MiscZipFileIT.java +++ b/src/test/java/net/lingala/zip4j/MiscZipFileIT.java @@ -673,6 +673,16 @@ public void testAddFileWithCustomLastModifiedFileTimeSetsInputTime() throws IOEx verifyLastModifiedFileTime(zipFile, fileToTestWith, expectedLastModifiedTimeInMillis); } + @Test + public void testExtractFileWithExtraDataRecordAndCorruptMac() throws ZipException { + ZipFile zipFile = new ZipFile(getTestArchiveFromResources("aes_with_extra_data_record_and_corrupt_mac.zip"), PASSWORD); + + expectedException.expect(ZipException.class); + expectedException.expectMessage("java.io.IOException: Reached end of data for this entry, but aes verification failed"); + + zipFile.extractAll(outputFolder.getPath()); + } + private void testAddAndExtractWithPasswordUtf8Encoding(boolean useUtf8ForPassword) throws IOException { char[] password = "hun 焰".toCharArray(); ZipFile zipFile = new ZipFile(generatedZipFile, password); diff --git a/src/test/java/net/lingala/zip4j/io/inputstream/ZipInputStreamIT.java b/src/test/java/net/lingala/zip4j/io/inputstream/ZipInputStreamIT.java index 8dbd2ec9..a4068d90 100644 --- a/src/test/java/net/lingala/zip4j/io/inputstream/ZipInputStreamIT.java +++ b/src/test/java/net/lingala/zip4j/io/inputstream/ZipInputStreamIT.java @@ -24,10 +24,10 @@ import java.io.InputStream; import java.io.OutputStream; import java.nio.file.Files; +import java.security.SecureRandom; import java.util.ArrayList; import java.util.HashSet; import java.util.List; -import java.security.SecureRandom; import java.util.Set; import java.util.zip.ZipEntry; import java.util.zip.ZipOutputStream; @@ -351,6 +351,15 @@ public void readingJarFileWithCompressedSizeNotZeroForDirectoryIsSuccessful() th } } + @Test + public void testExtractZipFileWithExtraDataRecordAndCorruptAesMacFails() throws IOException { + expectedException.expect(IOException.class); + expectedException.expectMessage("Reached end of data for this entry, but aes verification failed"); + + extractZipFileWithInputStreams(TestUtils.getTestArchiveFromResources("aes_with_extra_data_record_and_corrupt_mac.zip"), + PASSWORD, InternalZipConstants.BUFF_SIZE, false, 1); + } + private void extractZipFileWithInputStreams(File zipFile, char[] password) throws IOException { extractZipFileWithInputStreams(zipFile, password, InternalZipConstants.BUFF_SIZE); } diff --git a/src/test/java/net/lingala/zip4j/io/outputstream/ZipOutputStreamIT.java b/src/test/java/net/lingala/zip4j/io/outputstream/ZipOutputStreamIT.java index 83b48b06..f0d83d44 100644 --- a/src/test/java/net/lingala/zip4j/io/outputstream/ZipOutputStreamIT.java +++ b/src/test/java/net/lingala/zip4j/io/outputstream/ZipOutputStreamIT.java @@ -30,6 +30,8 @@ import java.io.OutputStream; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Paths; import java.util.ArrayList; import java.util.List; @@ -310,6 +312,28 @@ public void testLastModifiedTimeIsSetWhenItIsNotExplicitlySet() throws IOExcepti } } + @Test + public void testZipInputStreamWithDeflateAndAesEncryption() throws IOException { + byte[] buffer = new byte[InternalZipConstants.BUFF_SIZE]; + int readLen; + File fileToAdd = getTestFileFromResources("file_PDF_1MB.pdf"); + try (ZipOutputStream zipOutputStream = new ZipOutputStream(Files.newOutputStream(generatedZipFile.toPath()), PASSWORD)) { + ZipParameters zipParameters = new ZipParameters(); + zipParameters.setFileNameInZip(fileToAdd.getName()); + zipParameters.setEncryptFiles(true); + zipParameters.setEncryptionMethod(EncryptionMethod.AES); + zipOutputStream.putNextEntry(zipParameters); + try (InputStream inputStream = Files.newInputStream(fileToAdd.toPath())) { + while ((readLen = inputStream.read(buffer)) != -1) { + zipOutputStream.write(buffer, 0, readLen); + } + } + } + + verifyZipFileByExtractingAllFiles(generatedZipFile, PASSWORD, outputFolder, 1, true); + extractZipFileWithInputStream(generatedZipFile); + } + private void testZipOutputStream(CompressionMethod compressionMethod, boolean encrypt, EncryptionMethod encryptionMethod, AesKeyStrength aesKeyStrength, AesVersion aesVersion) @@ -513,4 +537,19 @@ private void putNextEntryAndCloseEntry(ZipOutputStream zipOutputStream, String f zipOutputStream.putNextEntry(zipParameters); zipOutputStream.closeEntry(); } + + private void extractZipFileWithInputStream(File zipFile) throws IOException { + byte[] buffer = new byte[InternalZipConstants.BUFF_SIZE]; + int readLen; + LocalFileHeader lfh; + try (ZipInputStream zipInputStream = new ZipInputStream(Files.newInputStream(zipFile.toPath()), PASSWORD)) { + while ((lfh = zipInputStream.getNextEntry()) != null) { + while ((readLen = zipInputStream.read(buffer)) != -1) { + try (OutputStream outputStream = Files.newOutputStream(Paths.get(outputFolder.getPath(), lfh.getFileName()))) { + outputStream.write(buffer, 0, readLen); + } + } + } + } + } } diff --git a/src/test/resources/test-archives/aes_with_extra_data_record_and_corrupt_mac.zip b/src/test/resources/test-archives/aes_with_extra_data_record_and_corrupt_mac.zip new file mode 100644 index 0000000000000000000000000000000000000000..ab8ff250c1005c48680379dad9e10a5c8fda1ffc GIT binary patch literal 4320 zcmai&Ra6v=)`o`xWN1V}VkqejfuW?k8IX_|9GU^??(QL^QA#=$hM_@}7)cpHN^t-Y zDe02!wcbP=jRyY zXNR!&ads5;3-ZG&Bmw{d%If$aKqLdhbNVVO4?GwIpuT!11<1fjbFg2^uaeS@uz>S+nk^3xKJu}sD6V` zBUEU(9TlIEQiOr;B2C~mRO@USDnTg`N=@_XWS8>ge2)CQRs-`|} zui;K@TGu6^EW-AP3^@po- zu=LoxKmF#JD^5{)AnAJ~-*slkqp-bCu1yu3zN;>+Ws! zK>d+l?D#G{{<@x-6XYvG_1duy{bZi$AgnBLg1_47cY@-nr<6m>M>si)R5Zq7OG^OH zH2X&-VLP`!Z-!dPEJ-THXXznA-=B}dH6{iMK^;iIGojUeqW^RbupN@~BF&8j4T1YE-&{uoI zR2K<^9VMAw1DmMm;aEAN#}%r^SMi$0yZ_hk?MKxF%foXUr&D&nC7WBhO3Wq5l9}hj z^n7h?uuc)m?Y(MVnJbE5eOJ9i2fOQ>j|0)ZAw2Z7c@29$V(RoYS?6pmP4bQ0A31#~ zn_PWrTXpD;Yiq6P$Lgs_U$!5O1PKWGzi;x@@*?xWKls+p!31`hWlIpNx;SHIh(V&~ z$qS)h-qjKb2IS`yPt=f|PL-;|n#*!UXki+{h4u|(Jx8jnhx%n`U z)1mho=E7Wg+x$aSE1Cnrdq>?eZcaCCcZpPaRSA5TL~k>(E$RN?ZF#sFGi{NxlHpW% z%gXb$cBNvoN5bC>{V1>@ebC+U3}qldXG4P|A$vc}cH5F=$*GL_x%4 zcwxP1t=0h8M@W~bpP9o7=9f9UA~UX&AJ3(in=!ftYy?r?j~C*se`s7Ga4138c1!v- zuR^#EEnpq->S&h+@v^s>2*kZWHK@kIxk_HF*R>)X4!^?yAY3Z=Et$q{>Z@8i3>*9; zZ>st&;>XU@Cy$LvKc2R4bC`3ylIvIDjDIn3&R_96BE5hJUxA>DCeKoF992_V^kBfs z&qnK=O!l^A1q&9%(yPM^1*a~jsnj+*qgTXRJ|?tQJ;-yc_rseaoRWk|UxXS7)VV2F zG>!#ZtELSGM+hl`~yZumtG+$4z$T4eHi{%y^QSEwED*O-f(iU0VIG zfoC-F_?KcM(%PGxOk^yqL&Lz%9f)nAYfQ&QZjhKuG>|hzYP(#DK8-*k@k!jRg*gY3 z%{9=4TE(Onr5$Y|Qq+997u(yZgx{@Ifc!n|-BCMsESK%aIK5X(VK%I)qrCUsMNN=> zRd|eJc%GE}J?zjMi_8`T@yOFl{um*@dd#;Ol{+XwDeS}gh2x)5UX}_aRj&QME?ZcYfd(#s7`Eti1=n~ z>Oe)j?uw`NtQN^5;dwswExkbwch{>lnI2$kRIR@9b8LyLUk;lZO5Xdt>Cxv=%aAQk z-KJ6C=t-xQ!LXqNiv(NE0_99{3?RVn}cWg@h`;a%~@@H8i~+gUc?%vC&P+&<7v zer616LOp=8N0C^IPH6y39T`pEiBD~22PQr}A=1ln-`U}+(b1!hEU5g}b&uVh$6vW7 ziq3}oLF=7Fk-)7@mMszy)S8I8EwynPw-TyAJ-p|;tphI0?OS#$PkT{itEx#cl6KUJ z{zMNQAxB_RU+>V}WSZj{anm<9#-zllXeCy2rQleW#OZ~%R4cBSrz{QSV)Led3dj&M z1`owwq`W=0XmZOV`ff6!(4XO`nG*5iL0YMFeSud8QK?HSXU-Qi5&0ne?OO`5+jRBu(Qq@rDQIBtJ_Do7J z+(5}kPw3tBUx5{z)~co9O#JB5>8Sg#g7J-ose5*x ztGm~09}kuDXbKrIme07zMp^{e7QK^e?h(HY`TrQ3MBU~8=)05C0NKnjcox4QfAEvbXI(uot(d=~ z{Ik&wqng}Xc)iYtzPdd~qWqzAw6(&wkO>LOE6q=6ApRrB_Xptni$DqOIYp+@#OwZu z`vVYu?H=x(6W6a?u|`vpqNu!O0>ono6fT~c${vXCb@tLpL;>vM=^i7cSa+0%xii)1 z3C}61$np?&O^kW-nySGNyE|%ZrJ(#H@2UNPycFN%OW_{l%YmiSmAqR8iG|x2O=YP+` z+l@axK!nSsBg$K>wkp~pZ+95>xL`P2JX-Onrc5+`Usj#643L`NCZZ#2@#*6sVRo6( zZ?t_x{;eh+)R&?*!z`I*vh#~=asdKUUe&-PDBML)g1<_8*; z``>Yy6&hW%+ih?KTy$c&e%P4m<0rLrBg@BD27KmPb}Vjh602Ze@sQVGB>oeNN;~6N zUncc45v>V@{%)=5wy#zmDjQm&q=h=kB+*%Ob2eK)H08*LT;GR|jG6KVjud#c1-6rW z#aso$=X`9jkr((c&nBBF-+S5>HdA-bcyGce^*zW+&HXsdu!asQ%SW05X^`+KybM+m z@9X};h0I$5h)EMU6~(I&fSw>ZFeDTUVO=uj|)T3+~s6^ zK1)8F+>P3YQBtdp3!Dwzc{mR@j)n~$(-#&SjZTT&i&4e8Fwk8jvwOD36mQqiHJU^U z3_>vs0w(MnGeS8l+h16nz(3HFBGfnIpMgoeaQ_~AGVb?Yyo?|b8W`JqmC#IOFvl4(@$uk6d6c6E@y z=b0AsK7BRvV-bav;PR-IEGN`t{C&S9=sjKfyxQUNln7Mw54mfg�xom9nv(xrG=g zFZZ$dxv_x=&rtbaimzWiVvkfGNBZ)e$k#vH1})XRI5(gMi@%uOO27Njk{C;H=2ejHHnu^@yCY&(%MJnDL=Y0XSJkB&Zz9@(8 zp6pWdX;m7ATWZ3^P8^THZRGjKOBVd=m7+~a>+A^Pq-tfJo%Qfu|y8~50A ztTdETq#<7t%MZhPjwHzcrdG$6{$Q}lZI#tFuiwqYBVC)L!zR8Ct)EJ=R| z6&6d-kzdhp7Ym`NlxYu}^H8osEi%^R}4AtbZHdZL_8G0r$H&tUg7{&14rDCZ+wA%JAl%pbe@xj-A8SrK!(LTZmr5VM3js zVi3-87wND{M!4f8;td1?rxOx>$;>`=_2nqM@&qj_GDn#+`2s==7FB+qzL|Kn23t^< zUM@evB9@aOK%ofs$%Xt_(Q<{d=V;MF-w#}zUcQ$0!o@?BMiFYOzRQanBA+D+PBrewbi!=?a8}XGpo4}@#V`!|*WLQS>#X)7 zXm;=A{1miAaCNsg^SJ7^f5s%Wo8sB@s|`&-YANeAK?9q?voNpZFgDi$+dhKdGn{IR zdkkFQG3$4PnXX@Slja>%lkzpB_tQC_1+ctsNRq|mFRKSxVo4UIZU%mmeo7cL(j@}@ zD;R{S004z#Jb;ld9#8`RzwN_+x_|A%e|Qu9Z~9*@!bq3!-oJNv|BS&uJM5n?0KoqN DsB;+M literal 0 HcmV?d00001