From ba150accda722df57396f79c0fb70df9fe9673db Mon Sep 17 00:00:00 2001 From: Paulo Casanova Date: Tue, 27 Dec 2016 16:16:59 +0000 Subject: Allow disabling version to extract in zip. Add an option to ZFile to ignore the "version to extract" in the central directory header and in the local header. This allows supporting some zip tools that write weird values in the field. Test: JUnit tests included Change-Id: If8f14737ddb50f30508937e13f376cb30bde71e2 --- .../com/android/apkzlib/zip/CentralDirectory.java | 10 +- .../java/com/android/apkzlib/zip/StoredEntry.java | 12 ++- src/main/java/com/android/apkzlib/zip/ZFile.java | 16 +++ .../java/com/android/apkzlib/zip/ZFileOptions.java | 25 +++++ .../java/com/android/apkzlib/zip/ZipField.java | 9 ++ .../java/com/android/apkzlib/zip/ZFileTest.java | 109 ++++++++++++++++++++- 6 files changed, 173 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/main/java/com/android/apkzlib/zip/CentralDirectory.java b/src/main/java/com/android/apkzlib/zip/CentralDirectory.java index 083bc65..e088298 100644 --- a/src/main/java/com/android/apkzlib/zip/CentralDirectory.java +++ b/src/main/java/com/android/apkzlib/zip/CentralDirectory.java @@ -18,6 +18,7 @@ package com.android.apkzlib.zip; import com.android.apkzlib.utils.CachedSupplier; import com.android.apkzlib.zip.utils.MsDosDateTimeUtils; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; @@ -28,7 +29,6 @@ import com.google.common.util.concurrent.ListenableFuture; import java.io.IOException; import java.io.UncheckedIOException; import java.nio.ByteBuffer; -import java.util.Collections; import java.util.List; import java.util.Map; import java.util.Set; @@ -53,7 +53,8 @@ class CentralDirectory { /** * Field in the central directory with the minimum version required to extract the entry. */ - private static final ZipField.F2 F_VERSION_EXTRACT = new ZipField.F2(F_MADE_BY.endOffset(), + @VisibleForTesting + static final ZipField.F2 F_VERSION_EXTRACT = new ZipField.F2(F_MADE_BY.endOffset(), "Version to extract", new ZipFieldInvariantNonNegative()); /** @@ -275,7 +276,8 @@ class CentralDirectory { long madeBy = F_MADE_BY.read(bytes); long versionNeededToExtract = F_VERSION_EXTRACT.read(bytes); - if (versionNeededToExtract > MAX_VERSION_TO_EXTRACT) { + if (versionNeededToExtract > MAX_VERSION_TO_EXTRACT + && !file.getSkipVersionToExtractValidation()) { throw new IOException("Unknown version needed to extract in zip directory entry: " + versionNeededToExtract + "."); } @@ -403,7 +405,7 @@ class CentralDirectory { private byte[] computeByteRepresentation() { List sorted = Lists.newArrayList(mEntries.values()); - Collections.sort(sorted, StoredEntry.COMPARE_BY_NAME); + sorted.sort(StoredEntry.COMPARE_BY_NAME); CentralDirectoryHeader[] cdhs = new CentralDirectoryHeader[mEntries.size()]; CentralDirectoryHeaderCompressInfo[] compressInfos = diff --git a/src/main/java/com/android/apkzlib/zip/StoredEntry.java b/src/main/java/com/android/apkzlib/zip/StoredEntry.java index a8ee56e..10289ca 100644 --- a/src/main/java/com/android/apkzlib/zip/StoredEntry.java +++ b/src/main/java/com/android/apkzlib/zip/StoredEntry.java @@ -18,6 +18,7 @@ package com.android.apkzlib.zip; import com.android.apkzlib.zip.utils.CloseableByteSource; import com.android.apkzlib.zip.utils.CloseableDelegateByteSource; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.base.Verify; import com.google.common.io.ByteSource; @@ -84,7 +85,8 @@ public class StoredEntry { /** * Local header field: version to extract, should match the CDH's. */ - private static final ZipField.F2 F_VERSION_EXTRACT = new ZipField.F2( + @VisibleForTesting + static final ZipField.F2 F_VERSION_EXTRACT = new ZipField.F2( F_LOCAL_SIGNATURE.endOffset(), "Version to extract", new ZipFieldInvariantNonNegative()); @@ -399,7 +401,13 @@ public class StoredEntry { ByteBuffer bytes = ByteBuffer.wrap(localHeader); F_LOCAL_SIGNATURE.verify(bytes); - F_VERSION_EXTRACT.verify(bytes, compressInfo.getVersionExtract()); + + if (mFile.getSkipVersionToExtractValidation()) { + F_VERSION_EXTRACT.skip(bytes); + } else { + F_VERSION_EXTRACT.verify(bytes, compressInfo.getVersionExtract()); + } + F_GP_BIT.verify(bytes, mCdh.getGpBit().getValue()); F_METHOD.verify(bytes, compressInfo.getMethod().methodCode); diff --git a/src/main/java/com/android/apkzlib/zip/ZFile.java b/src/main/java/com/android/apkzlib/zip/ZFile.java index 301b942..9fd1ef1 100644 --- a/src/main/java/com/android/apkzlib/zip/ZFile.java +++ b/src/main/java/com/android/apkzlib/zip/ZFile.java @@ -373,6 +373,12 @@ public class ZFile implements Closeable { */ private boolean mSkipDataDescriptorVerification; + /** + * Should the "version to extract" field validation be skipped? See + * {@link ZFileOptions#getSkipZipVersionToExtractValidation()}. + */ + private boolean mSkipVersionToExtractValidation; + /** * Creates a new zip file. If the zip file does not exist, then no file is created at this @@ -415,6 +421,7 @@ public class ZFile implements Closeable { mCoverEmptySpaceUsingExtraField = options.getCoverEmptySpaceUsingExtraField(); mAutoSortFiles = options.getAutoSortFiles(); mSkipDataDescriptorVerification = options.getSkipDataDescriptorValidation(); + mSkipVersionToExtractValidation = options.getSkipZipVersionToExtractValidation(); /* * These two values will be overwritten by openReadOnly() below if the file exists. @@ -2415,6 +2422,15 @@ public class ZFile implements Closeable { return mSkipDataDescriptorVerification; } + /** + * Checks whether the "version to extract" validation should be skipped. + * + * @return should it be skipped? + */ + boolean getSkipVersionToExtractValidation() { + return mSkipVersionToExtractValidation; + } + /** * Hint to where files should be positioned. */ diff --git a/src/main/java/com/android/apkzlib/zip/ZFileOptions.java b/src/main/java/com/android/apkzlib/zip/ZFileOptions.java index cc523c5..39da52a 100644 --- a/src/main/java/com/android/apkzlib/zip/ZFileOptions.java +++ b/src/main/java/com/android/apkzlib/zip/ZFileOptions.java @@ -65,6 +65,11 @@ public class ZFileOptions { */ private boolean mSkipDataDescriptionValidation; + /** + * Should validation of minimum zip version to extract be performed? + */ + private boolean mSkipZipVersionToExtractValidation; + /** * Creates a new options object. All options are set to their defaults. */ @@ -205,4 +210,24 @@ public class ZFileOptions { public void setSkipDataDescriptionValidation(boolean skip) { mSkipDataDescriptionValidation = skip; } + + /** + * Should the "minimum zip version to extract" field of the local and central headers be + * validated or not? By default they are, but some zip tools put weird values in there. + * + * @return should the "zip version to extract" field be ignored? + */ + public boolean getSkipZipVersionToExtractValidation() { + return mSkipZipVersionToExtractValidation; + } + + /** + * Sets whether the "zip version to extract" field validation should be skipped. See + * {@link #getSkipZipVersionToExtractValidation()}. + * + * @param skip should the "zip version to extract" field be ignored? + */ + public void setSkipZipVersionToExtractValidation(boolean skip) { + mSkipZipVersionToExtractValidation = skip; + } } diff --git a/src/main/java/com/android/apkzlib/zip/ZipField.java b/src/main/java/com/android/apkzlib/zip/ZipField.java index 3551e71..db6bb0b 100644 --- a/src/main/java/com/android/apkzlib/zip/ZipField.java +++ b/src/main/java/com/android/apkzlib/zip/ZipField.java @@ -246,6 +246,15 @@ abstract class ZipField { write(output, mExpected); } + /** + * Obtains the offset at which the field starts. + * + * @return the start offset + */ + int offset() { + return mOffset; + } + /** * Obtains the offset at which the field ends. This is the exact offset at which the next * field starts. diff --git a/src/test/java/com/android/apkzlib/zip/ZFileTest.java b/src/test/java/com/android/apkzlib/zip/ZFileTest.java index 742b9bd..265ef00 100644 --- a/src/test/java/com/android/apkzlib/zip/ZFileTest.java +++ b/src/test/java/com/android/apkzlib/zip/ZFileTest.java @@ -45,6 +45,7 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.RandomAccessFile; +import java.util.Locale; import java.util.Random; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -1394,8 +1395,6 @@ public class ZFileTest { /* * We should be complaining about the CRC32 somewhere... */ - boolean foundCrc32Complain = false; - assertTrue( Throwables.getCausalChain(e).stream() .map(Throwable::getMessage) @@ -1413,4 +1412,110 @@ public class ZFileTest { */ } } + + @Test + public void detectIncorrectVersionToExtractInCentralDirectory() throws Exception { + File zipFile = new File(mTemporaryFolder.getRoot(), "a.zip"); + + /* + * Create a valid zip file. + */ + try (ZFile zf = new ZFile(zipFile)) { + zf.add("foo", new ByteArrayInputStream(new byte[0])); + } + + /* + * Change the "version to extract" in the central directory to 0x7777. + */ + int versionToExtractOffset = + ZFileTestConstants.LOCAL_HEADER_SIZE + + 3 + + CentralDirectory.F_VERSION_EXTRACT.offset(); + byte[] allZipBytes = Files.toByteArray(zipFile); + allZipBytes[versionToExtractOffset] = 0x77; + allZipBytes[versionToExtractOffset + 1] = 0x77; + Files.write(allZipBytes, zipFile); + + /* + * Opening the file should fail. + */ + try { + new ZFile(zipFile); + fail(); + } catch (IOException e) { + /* + * We should complain about the version to extract somewhere... + */ + assertTrue( + Throwables.getCausalChain(e).stream() + .map(Throwable::getMessage) + .anyMatch(s -> s.toLowerCase(Locale.US).contains("version"))); + assertTrue( + Throwables.getCausalChain(e).stream() + .map(Throwable::getMessage) + .anyMatch(s -> s.toLowerCase(Locale.US).contains("extract"))); + } + + /* + * Setting the ignore version extract validation should allow the zip to be opened. + */ + ZFileOptions options = new ZFileOptions(); + options.setSkipZipVersionToExtractValidation(true); + try (ZFile zf = new ZFile(zipFile, options)) { + /* + * Nothing to do. + */ + } + } + @Test + public void detectIncorrectVersionToExtractInLocalHeader() throws Exception { + File zipFile = new File(mTemporaryFolder.getRoot(), "a.zip"); + + /* + * Create a valid zip file. + */ + try (ZFile zf = new ZFile(zipFile)) { + zf.add("foo", new ByteArrayInputStream(new byte[0])); + } + + /* + * Change the "version to extract" in the local header to 0x7777. + */ + int versionToExtractOffset = StoredEntry.F_VERSION_EXTRACT.offset(); + byte[] allZipBytes = Files.toByteArray(zipFile); + allZipBytes[versionToExtractOffset] = 0x77; + allZipBytes[versionToExtractOffset + 1] = 0x77; + Files.write(allZipBytes, zipFile); + + /* + * Opening the file should fail. + */ + try { + new ZFile(zipFile); + fail(); + } catch (IOException e) { + /* + * We should complain about the version to extract somewhere... + */ + assertTrue( + Throwables.getCausalChain(e).stream() + .map(Throwable::getMessage) + .anyMatch(s -> s.toLowerCase(Locale.US).contains("version"))); + assertTrue( + Throwables.getCausalChain(e).stream() + .map(Throwable::getMessage) + .anyMatch(s -> s.toLowerCase(Locale.US).contains("extract"))); + } + + /* + * Setting the ignore version extract validation should allow the zip to be opened. + */ + ZFileOptions options = new ZFileOptions(); + options.setSkipZipVersionToExtractValidation(true); + try (ZFile zf = new ZFile(zipFile, options)) { + /* + * Nothing to do. + */ + } + } } -- cgit v1.2.3