diff options
6 files changed, 173 insertions, 8 deletions
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<StoredEntry> 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. @@ -2416,6 +2423,15 @@ public class ZFile implements Closeable { } /** + * 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. */ enum PositionHint { 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 @@ -66,6 +66,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. */ public ZFileOptions() { @@ -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 @@ -247,6 +247,15 @@ abstract class ZipField { } /** + * 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. + */ + } + } } |