Check crc and time of secondary dex files Protect extracted dex files from modifications by checking their crc and modification time. In case of change, proceed to a new extraction. Those checks are replacing the check of the zip integrity by opening it with a ZipFile. Test: SupportMultidexHostTest (from tradefed) Bug: 32159214 Change-Id: I09aa01550782f5f550bee6fc91709455e82c1057
diff --git a/library/src/android/support/multidex/MultiDex.java b/library/src/android/support/multidex/MultiDex.java index fad50d7..d35da96 100644 --- a/library/src/android/support/multidex/MultiDex.java +++ b/library/src/android/support/multidex/MultiDex.java
@@ -155,7 +155,8 @@ } File dexDir = getDexDir(context, applicationInfo); - List<File> files = MultiDexExtractor.load(context, applicationInfo, dexDir, false); + List<? extends File> files = + MultiDexExtractor.load(context, applicationInfo, dexDir, false); installSecondaryDexes(loader, dexDir, files); } @@ -217,7 +218,8 @@ return isMultidexCapable; } - private static void installSecondaryDexes(ClassLoader loader, File dexDir, List<File> files) + private static void installSecondaryDexes(ClassLoader loader, File dexDir, + List<? extends File> files) throws IllegalArgumentException, IllegalAccessException, NoSuchFieldException, InvocationTargetException, NoSuchMethodException, IOException { if (!files.isEmpty()) { @@ -374,7 +376,8 @@ */ private static final class V19 { - private static void install(ClassLoader loader, List<File> additionalClassPathEntries, + private static void install(ClassLoader loader, + List<? extends File> additionalClassPathEntries, File optimizedDirectory) throws IllegalArgumentException, IllegalAccessException, NoSuchFieldException, InvocationTargetException, NoSuchMethodException { @@ -439,7 +442,8 @@ */ private static final class V14 { - private static void install(ClassLoader loader, List<File> additionalClassPathEntries, + private static void install(ClassLoader loader, + List<? extends File> additionalClassPathEntries, File optimizedDirectory) throws IllegalArgumentException, IllegalAccessException, NoSuchFieldException, InvocationTargetException, NoSuchMethodException { @@ -473,7 +477,8 @@ * Installer for platform versions 4 to 13. */ private static final class V4 { - private static void install(ClassLoader loader, List<File> additionalClassPathEntries) + private static void install(ClassLoader loader, + List<? extends File> additionalClassPathEntries) throws IllegalArgumentException, IllegalAccessException, NoSuchFieldException, IOException { /* The patched class loader is expected to be a descendant of @@ -490,7 +495,7 @@ File[] extraFiles = new File[extraSize]; ZipFile[] extraZips = new ZipFile[extraSize]; DexFile[] extraDexs = new DexFile[extraSize]; - for (ListIterator<File> iterator = additionalClassPathEntries.listIterator(); + for (ListIterator<? extends File> iterator = additionalClassPathEntries.listIterator(); iterator.hasNext();) { File additionalEntry = iterator.next(); String entryPath = additionalEntry.getAbsolutePath();
diff --git a/library/src/android/support/multidex/MultiDexExtractor.java b/library/src/android/support/multidex/MultiDexExtractor.java index 998ccec..2d7402a 100644 --- a/library/src/android/support/multidex/MultiDexExtractor.java +++ b/library/src/android/support/multidex/MultiDexExtractor.java
@@ -21,7 +21,6 @@ import android.content.pm.ApplicationInfo; import android.os.Build; import android.util.Log; - import java.io.BufferedOutputStream; import java.io.Closeable; import java.io.File; @@ -36,7 +35,6 @@ import java.util.ArrayList; import java.util.List; import java.util.zip.ZipEntry; -import java.util.zip.ZipException; import java.util.zip.ZipFile; import java.util.zip.ZipOutputStream; @@ -46,6 +44,17 @@ */ final class MultiDexExtractor { + /** + * Zip file containing one secondary dex file. + */ + private static class ExtractedDex extends File { + public long crc = NO_VALUE; + + public ExtractedDex(File dexDir, String fileName) { + super(dexDir, fileName); + } + } + private static final String TAG = MultiDex.TAG; /** @@ -63,6 +72,8 @@ private static final String KEY_TIME_STAMP = "timestamp"; private static final String KEY_CRC = "crc"; private static final String KEY_DEX_NUMBER = "dex.number"; + private static final String KEY_DEX_CRC = "dex.crc."; + private static final String KEY_DEX_TIME = "dex.time."; /** * Size of reading buffers. @@ -82,7 +93,7 @@ * @throws IOException if encounters a problem while reading or writing * secondary dex files */ - static List<File> load(Context context, ApplicationInfo applicationInfo, File dexDir, + static List<? extends File> load(Context context, ApplicationInfo applicationInfo, File dexDir, boolean forceReload) throws IOException { Log.i(TAG, "MultiDexExtractor.load(" + applicationInfo.sourceDir + ", " + forceReload + ")"); final File sourceApk = new File(applicationInfo.sourceDir); @@ -94,7 +105,7 @@ RandomAccessFile lockRaf = new RandomAccessFile(lockFile, "rw"); FileChannel lockChannel = null; FileLock cacheLock = null; - List<File> files; + List<ExtractedDex> files; IOException releaseLockException = null; try { lockChannel = lockRaf.getChannel(); @@ -109,14 +120,12 @@ Log.w(TAG, "Failed to reload existing extracted secondary dex files," + " falling back to fresh extraction", ioe); files = performExtractions(sourceApk, dexDir); - putStoredApkInfo(context, - getTimeStamp(sourceApk), currentCrc, files.size() + 1); - + putStoredApkInfo(context, getTimeStamp(sourceApk), currentCrc, files); } } else { Log.i(TAG, "Detected that extraction must be performed."); files = performExtractions(sourceApk, dexDir); - putStoredApkInfo(context, getTimeStamp(sourceApk), currentCrc, files.size() + 1); + putStoredApkInfo(context, getTimeStamp(sourceApk), currentCrc, files); } } finally { if (cacheLock != null) { @@ -147,23 +156,35 @@ * Load previously extracted secondary dex files. Should be called only while owning the lock on * {@link #LOCK_FILENAME}. */ - private static List<File> loadExistingExtractions(Context context, File sourceApk, File dexDir) + private static List<ExtractedDex> loadExistingExtractions( + Context context, File sourceApk, File dexDir) throws IOException { Log.i(TAG, "loading existing secondary dex files"); final String extractedFilePrefix = sourceApk.getName() + EXTRACTED_NAME_EXT; - int totalDexNumber = getMultiDexPreferences(context).getInt(KEY_DEX_NUMBER, 1); - final List<File> files = new ArrayList<File>(totalDexNumber); + SharedPreferences multiDexPreferences = getMultiDexPreferences(context); + int totalDexNumber = multiDexPreferences.getInt(KEY_DEX_NUMBER, 1); + final List<ExtractedDex> files = new ArrayList<ExtractedDex>(totalDexNumber - 1); for (int secondaryNumber = 2; secondaryNumber <= totalDexNumber; secondaryNumber++) { String fileName = extractedFilePrefix + secondaryNumber + EXTRACTED_SUFFIX; - File extractedFile = new File(dexDir, fileName); + ExtractedDex extractedFile = new ExtractedDex(dexDir, fileName); if (extractedFile.isFile()) { - files.add(extractedFile); - if (!verifyZipFile(extractedFile)) { - Log.i(TAG, "Invalid zip file: " + extractedFile); - throw new IOException("Invalid ZIP file."); + extractedFile.crc = getZipCrc(extractedFile); + long expectedCrc = + multiDexPreferences.getLong(KEY_DEX_CRC + secondaryNumber, NO_VALUE); + long expectedModTime = + multiDexPreferences.getLong(KEY_DEX_TIME + secondaryNumber, NO_VALUE); + long lastModified = extractedFile.lastModified(); + if ((expectedModTime != lastModified) + || (expectedCrc != extractedFile.crc)) { + throw new IOException("Invalid extracted dex: " + extractedFile + + ", expected modification time: " + + expectedModTime + ", modification time: " + + lastModified + ", expected crc: " + + expectedCrc + ", file crc: " + extractedFile.crc); } + files.add(extractedFile); } else { throw new IOException("Missing extracted secondary dex file '" + extractedFile.getPath() + "'"); @@ -203,7 +224,7 @@ return computedValue; } - private static List<File> performExtractions(File sourceApk, File dexDir) + private static List<ExtractedDex> performExtractions(File sourceApk, File dexDir) throws IOException { final String extractedFilePrefix = sourceApk.getName() + EXTRACTED_NAME_EXT; @@ -214,7 +235,7 @@ // while another had created it. prepareDexDir(dexDir, extractedFilePrefix); - List<File> files = new ArrayList<File>(); + List<ExtractedDex> files = new ArrayList<ExtractedDex>(); final ZipFile apk = new ZipFile(sourceApk); try { @@ -224,7 +245,7 @@ ZipEntry dexFile = apk.getEntry(DEX_PREFIX + secondaryNumber + DEX_SUFFIX); while (dexFile != null) { String fileName = extractedFilePrefix + secondaryNumber + EXTRACTED_SUFFIX; - File extractedFile = new File(dexDir, fileName); + ExtractedDex extractedFile = new ExtractedDex(dexDir, fileName); files.add(extractedFile); Log.i(TAG, "Extraction is needed for file " + extractedFile); @@ -237,13 +258,19 @@ // (dexFile) from the apk. extract(apk, dexFile, extractedFile, extractedFilePrefix); - // Verify that the extracted file is indeed a zip file. - isExtractionSuccessful = verifyZipFile(extractedFile); + // Read zip crc of extracted dex + try { + extractedFile.crc = getZipCrc(extractedFile); + isExtractionSuccessful = true; + } catch (IOException e) { + isExtractionSuccessful = false; + Log.w(TAG, "Failed to read crc from " + extractedFile.getAbsolutePath(), e); + } - // Log the sha1 of the extracted zip file - Log.i(TAG, "Extraction " + (isExtractionSuccessful ? "success" : "failed") + + // Log size and crc of the extracted zip file + Log.i(TAG, "Extraction " + (isExtractionSuccessful ? "succeeded" : "failed") + " - length " + extractedFile.getAbsolutePath() + ": " + - extractedFile.length()); + extractedFile.length() + " - crc: " + extractedFile.crc); if (!isExtractionSuccessful) { // Delete the extracted file extractedFile.delete(); @@ -277,12 +304,19 @@ * {@link #LOCK_FILENAME}. */ private static void putStoredApkInfo(Context context, long timeStamp, long crc, - int totalDexNumber) { + List<ExtractedDex> extractedDexes) { SharedPreferences prefs = getMultiDexPreferences(context); SharedPreferences.Editor edit = prefs.edit(); edit.putLong(KEY_TIME_STAMP, timeStamp); edit.putLong(KEY_CRC, crc); - edit.putInt(KEY_DEX_NUMBER, totalDexNumber); + edit.putInt(KEY_DEX_NUMBER, extractedDexes.size() + 1); + + int extractedDexId = 2; + for (ExtractedDex dex : extractedDexes) { + edit.putLong(KEY_DEX_CRC + extractedDexId, dex.crc); + edit.putLong(KEY_DEX_TIME + extractedDexId, dex.lastModified()); + extractedDexId++; + } /* Use commit() and not apply() as advised by the doc because we need synchronous writing of * the editor content and apply is doing an "asynchronous commit to disk". */ @@ -372,26 +406,6 @@ } /** - * Returns whether the file is a valid zip file. - */ - private static boolean verifyZipFile(File file) { - try { - ZipFile zipFile = new ZipFile(file); - try { - zipFile.close(); - return true; - } catch (IOException e) { - Log.w(TAG, "Failed to close zip file: " + file.getAbsolutePath()); - } - } catch (ZipException ex) { - Log.w(TAG, "File " + file.getAbsolutePath() + " is not a valid zip file.", ex); - } catch (IOException ex) { - Log.w(TAG, "Got an IOException trying to open zip file: " + file.getAbsolutePath(), ex); - } - return false; - } - - /** * Closes the given {@code Closeable}. Suppresses any IO exceptions. */ private static void closeQuietly(Closeable closeable) {