From bb744d6cbaa3bd9f48c00a995684307457e15d53 Mon Sep 17 00:00:00 2001 From: David Srbecky Date: Wed, 24 Jun 2026 13:37:47 +0100 Subject: [PATCH 1/7] Revert "Ignore parsing invalid Dex files (#91)" Replaced by the "Harden DexBackedDexFile against malformed DEX" commit. This reverts commit 491ae88c89c9bc950a0d1a4c1adcb6e8eaf6a081. --- .../android/tools/smali/dexlib2/dexbacked/ZipDexContainer.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/dexlib2/src/main/java/com/android/tools/smali/dexlib2/dexbacked/ZipDexContainer.java b/dexlib2/src/main/java/com/android/tools/smali/dexlib2/dexbacked/ZipDexContainer.java index 6199db05..50d0bdfc 100644 --- a/dexlib2/src/main/java/com/android/tools/smali/dexlib2/dexbacked/ZipDexContainer.java +++ b/dexlib2/src/main/java/com/android/tools/smali/dexlib2/dexbacked/ZipDexContainer.java @@ -105,8 +105,6 @@ private Map getEntries() throws IOException { entries.put(entry.getName() + (i > 1 ? ("/" + i) : ""), dex); offset += dex.getFileSize(); }; - } catch (ArrayIndexOutOfBoundsException ex) { - // This is expected if the dex file is invalid. } } From 5e2de1d0ff9f0ffc035804d74f47fcd64efbfe05 Mon Sep 17 00:00:00 2001 From: David Srbecky Date: Thu, 18 Jun 2026 13:20:29 +0100 Subject: [PATCH 2/7] Fix non-determinism in the writer Ensure callSiteComparator tie-breaks deterministically on call site name when encoded array offsets match, and align CallSiteReference equality. Standardize data section sorting to lexicographical order across writeEncodedArrays and CollectionUtils.compareAsSet, resolving a bug in compareAsSet where pre-sorting size comparisons could crash or error. Additionally, replace ArraySortedSet.of with copyOf in CollectionUtils and PoolClassDef to ensure collections are genuinely sorted when wrapped. Flag: EXEMPT BUGFIX Test: repeated builds produce the same file/hash now Change-Id: I85d39888f69a5e4aff90e273e9d6f4092f676215 TAG=agy CONV=396b0ad6-b46c-4693-90c5-77ac4f18be23 --- .../base/reference/BaseCallSiteReference.java | 3 +- .../iface/reference/CallSiteReference.java | 2 +- .../tools/smali/dexlib2/writer/DexWriter.java | 80 ++++++++++++++++--- .../tools/smali/util/CollectionUtils.java | 26 +----- 4 files changed, 76 insertions(+), 35 deletions(-) diff --git a/dexlib2/src/main/java/com/android/tools/smali/dexlib2/base/reference/BaseCallSiteReference.java b/dexlib2/src/main/java/com/android/tools/smali/dexlib2/base/reference/BaseCallSiteReference.java index 29962070..48d2f84a 100644 --- a/dexlib2/src/main/java/com/android/tools/smali/dexlib2/base/reference/BaseCallSiteReference.java +++ b/dexlib2/src/main/java/com/android/tools/smali/dexlib2/base/reference/BaseCallSiteReference.java @@ -48,7 +48,8 @@ public int hashCode() { public boolean equals(Object o) { if (o != null && o instanceof CallSiteReference) { CallSiteReference other = (CallSiteReference) o; - return getMethodHandle().equals(other.getMethodHandle()) && + return getName().equals(other.getName()) && + getMethodHandle().equals(other.getMethodHandle()) && getMethodName().equals(other.getMethodName()) && getMethodProto().equals(other.getMethodProto()) && getExtraArguments().equals(other.getExtraArguments()); diff --git a/dexlib2/src/main/java/com/android/tools/smali/dexlib2/iface/reference/CallSiteReference.java b/dexlib2/src/main/java/com/android/tools/smali/dexlib2/iface/reference/CallSiteReference.java index c122bfb4..832ce415 100644 --- a/dexlib2/src/main/java/com/android/tools/smali/dexlib2/iface/reference/CallSiteReference.java +++ b/dexlib2/src/main/java/com/android/tools/smali/dexlib2/iface/reference/CallSiteReference.java @@ -99,7 +99,7 @@ public interface CallSiteReference extends Reference { * Compares this CallSiteReference to another CallSiteReference for equality. * * This CallSiteReference is equal to another CallSiteReference if all of its fields are equal. That is, if - * the return values of getMethodHandle(), getMethodName(), getMethodProto() and getExtraArguments() are all equal. + * the return values of getName(), getMethodHandle(), getMethodName(), getMethodProto() and getExtraArguments() are all equal. * * @param o The object to be compared for equality with this CallSiteReference * @return true if the specified object is equal to this CallSiteReference diff --git a/dexlib2/src/main/java/com/android/tools/smali/dexlib2/writer/DexWriter.java b/dexlib2/src/main/java/com/android/tools/smali/dexlib2/writer/DexWriter.java index 42424bae..1f26d24d 100644 --- a/dexlib2/src/main/java/com/android/tools/smali/dexlib2/writer/DexWriter.java +++ b/dexlib2/src/main/java/com/android/tools/smali/dexlib2/writer/DexWriter.java @@ -264,7 +264,9 @@ protected abstract void writeEncodedValue(@Nonnull InternalEncodedValueWriter wr public int compare(Entry o1, Entry o2) { int offset1 = encodedArraySection.getItemOffset(callSiteSection.getEncodedCallSite(o1.getKey())); int offset2 = encodedArraySection.getItemOffset(callSiteSection.getEncodedCallSite(o2.getKey())); - return Integer.compare(offset1, offset2); + int res = Integer.compare(offset1, offset2); + if (res != 0) return res; + return o1.getKey().getName().compareTo(o2.getKey().getName()); } }; @@ -780,9 +782,14 @@ private void writeCallSites(DexDataWriter writer) throws IOException { private void writeMethodHandles(DexDataWriter writer) throws IOException { methodHandleSectionOffset = writer.getPosition(); + List> methodHandleEntries = + new ArrayList<>(methodHandleSection.getItems()); + Collections.sort(methodHandleEntries, + DexWriter.comparableKeyComparator()); + int index = 0; - for (Entry entry: methodHandleSection.getItems()) { + for (Map.Entry entry: methodHandleEntries) { entry.setValue(index++); MethodHandleKey methodHandleReference = entry.getKey(); writer.writeUshort(methodHandleReference.getMethodHandleType()); @@ -814,7 +821,8 @@ private void writeMethodHandles(DexDataWriter writer) throws IOException { } } - private void writeEncodedFields(@Nonnull DexDataWriter writer, @Nonnull Collection fields) + private void writeEncodedFields(@Nonnull DexDataWriter writer, + @Nonnull Collection fields) throws IOException { int prevIndex = 0; for (FieldKey key: fields) { @@ -828,7 +836,8 @@ private void writeEncodedFields(@Nonnull DexDataWriter writer, @Nonnull Collecti } } - private void writeEncodedMethods(@Nonnull DexDataWriter writer, @Nonnull Collection methods) + private void writeEncodedMethods(@Nonnull DexDataWriter writer, + @Nonnull Collection methods) throws IOException { int prevIndex = 0; for (MethodKey key: methods) { @@ -846,7 +855,21 @@ private void writeEncodedMethods(@Nonnull DexDataWriter writer, @Nonnull Collect private void writeTypeLists(@Nonnull DexDataWriter writer) throws IOException { writer.align(); typeListSectionOffset = writer.getPosition(); - for (Map.Entry entry: typeListSection.getItems()) { + List> typeListEntries = + new ArrayList<>(typeListSection.getItems()); + Collections.sort(typeListEntries, + new Comparator>() { + @Override + public int compare( + Map.Entry o1, + Map.Entry o2) { + return CollectionUtils.compareAsIterable( + CollectionUtils.usingToStringOrdering(), + typeListSection.getTypes(o1.getKey()), + typeListSection.getTypes(o2.getKey())); + } + }); + for (Map.Entry entry: typeListEntries) { writer.align(); entry.setValue(writer.getPosition()); @@ -861,10 +884,26 @@ private void writeTypeLists(@Nonnull DexDataWriter writer) throws IOException { private void writeEncodedArrays(@Nonnull DexDataWriter writer) throws IOException { InternalEncodedValueWriter encodedValueWriter = new InternalEncodedValueWriter(writer); encodedArraySectionOffset = writer.getPosition(); + List> encodedArrayEntries = + new ArrayList<>(encodedArraySection.getItems()); + Collections.sort(encodedArrayEntries, + new Comparator>() { + @Override + public int compare( + Map.Entry o1, + Map.Entry o2) { + List list1 = encodedArraySection.getEncodedValueList(o1.getKey()); + List list2 = encodedArraySection.getEncodedValueList(o2.getKey()); + return CollectionUtils.compareAsIterable( + (List) list1, + (List) list2); + } + }); - for (Map.Entry entry: encodedArraySection.getItems()) { + for (Map.Entry entry: encodedArrayEntries) { entry.setValue(writer.getPosition()); - List encodedArray = encodedArraySection.getEncodedValueList(entry.getKey()); + List encodedArray = + encodedArraySection.getEncodedValueList(entry.getKey()); writer.writeUleb128(encodedArray.size()); for (EncodedValue value: encodedArray) { writeEncodedValue(encodedValueWriter, value); @@ -876,7 +915,11 @@ private void writeAnnotations(@Nonnull DexDataWriter writer) throws IOException InternalEncodedValueWriter encodedValueWriter = new InternalEncodedValueWriter(writer); annotationSectionOffset = writer.getPosition(); - for (Map.Entry entry: annotationSection.getItems()) { + List> annotationEntries = + new ArrayList<>(annotationSection.getItems()); + Collections.sort(annotationEntries, + DexWriter.comparableKeyComparator()); + for (Map.Entry entry: annotationEntries) { entry.setValue(writer.getPosition()); AnnotationKey key = entry.getKey(); @@ -890,8 +933,10 @@ private void writeAnnotations(@Nonnull DexDataWriter writer) throws IOException writer.writeUleb128(elements.size()); for (AnnotationElement element: elements) { - writer.writeUleb128(stringSection.getItemIndex(annotationSection.getElementName(element))); - writeEncodedValue(encodedValueWriter, annotationSection.getElementValue(element)); + writer.writeUleb128(stringSection.getItemIndex( + annotationSection.getElementName(element))); + writeEncodedValue(encodedValueWriter, + annotationSection.getElementValue(element)); } } } @@ -902,7 +947,20 @@ private void writeAnnotationSets(@Nonnull DexDataWriter writer) throws IOExcepti if (shouldCreateEmptyAnnotationSet()) { writer.writeInt(0); } - for (Map.Entry entry: annotationSetSection.getItems()) { + List> annotationSetEntries = + new ArrayList<>(annotationSetSection.getItems()); + Collections.sort(annotationSetEntries, + new Comparator>() { + @Override + public int compare( + Map.Entry o1, + Map.Entry o2) { + return CollectionUtils.compareAsSet( + annotationSetSection.getAnnotations(o1.getKey()), + annotationSetSection.getAnnotations(o2.getKey())); + } + }); + for (Map.Entry entry: annotationSetEntries) { Collection annotations = CollectionUtils.immutableSortedCopy( annotationSetSection.getAnnotations(entry.getKey()), BaseAnnotation.BY_TYPE); diff --git a/dexlib2/src/main/java/com/android/tools/smali/util/CollectionUtils.java b/dexlib2/src/main/java/com/android/tools/smali/util/CollectionUtils.java index 7883dcfa..f56b8d53 100644 --- a/dexlib2/src/main/java/com/android/tools/smali/util/CollectionUtils.java +++ b/dexlib2/src/main/java/com/android/tools/smali/util/CollectionUtils.java @@ -164,7 +164,7 @@ private static SortedSet toNaturalSortedSet(@Nonnull Collection if (isNaturalSortedSet(collection)) { return (SortedSet)collection; } - SortedSet sortedSet = ArraySortedSet.of(NaturalOrdering.INSTANCE, collection.toArray()); + SortedSet sortedSet = ArraySortedSet.copyOf((Comparator)NaturalOrdering.INSTANCE, collection); return Collections.unmodifiableSortedSet(sortedSet); } @@ -179,7 +179,7 @@ private static SortedSet toSortedSet(@Nonnull Comparator list1, Collection list2) public static > int compareAsSet(@Nonnull Collection set1, @Nonnull Collection set2) { - int res = Integer.compare(set1.size(), set2.size()); - if (res != 0) return res; - SortedSet sortedSet1 = toNaturalSortedSet(set1); SortedSet sortedSet2 = toNaturalSortedSet(set2); - - Iterator elements2 = set2.iterator(); - for (T element1: set1) { - res = element1.compareTo(elements2.next()); - if (res != 0) return res; - } - return 0; + return compareAsIterable(sortedSet1, sortedSet2); } public static int compareAsSet(@Nonnull Comparator elementComparator, @Nonnull Collection list1, @Nonnull Collection list2) { - int res = Integer.compare(list1.size(), list2.size()); - if (res != 0) return res; - SortedSet set1 = toSortedSet(elementComparator, list1); SortedSet set2 = toSortedSet(elementComparator, list2); - - Iterator elements2 = set2.iterator(); - for (T element1: set1) { - res = elementComparator.compare(element1, elements2.next()); - if (res != 0) return res; - } - return 0; + return compareAsIterable(elementComparator, set1, set2); } public static List immutableSortedCopy( From efbcb06a6626eb7f25795e089c86f9c404b05184 Mon Sep 17 00:00:00 2001 From: David Srbecky Date: Fri, 19 Jun 2026 00:13:57 +0100 Subject: [PATCH 3/7] Fix logic inversion bugs in ArraySortedSet Resolve pre-existing bugs in ArraySortedSet where isEmpty() returned true for non-empty sets and toArray(T[] a) had inverted array boundary checks. Add unit tests for both methods in ArraySortedSetTest. Flag: EXEMPT BUGFIX Test: ./gradlew :dexlib2:test --tests com.android.tools.smali.util.ArraySortedSetTest TAG=agy CONV=92fcec37-cdaa-44ee-9f03-128bec8492e3 Change-Id: I1c375e8bddbb1373081b2688b18593f18a601ee5 --- .../tools/smali/util/ArraySortedSet.java | 13 +++++--- .../tools/smali/util/ArraySortedSetTest.java | 32 +++++++++++++++++++ 2 files changed, 40 insertions(+), 5 deletions(-) diff --git a/dexlib2/src/main/java/com/android/tools/smali/util/ArraySortedSet.java b/dexlib2/src/main/java/com/android/tools/smali/util/ArraySortedSet.java index 7bc16138..b243d4a2 100644 --- a/dexlib2/src/main/java/com/android/tools/smali/util/ArraySortedSet.java +++ b/dexlib2/src/main/java/com/android/tools/smali/util/ArraySortedSet.java @@ -87,7 +87,7 @@ public int size() { @Override public boolean isEmpty() { - return arr.length > 0; + return arr.length == 0; } @Override @@ -110,11 +110,14 @@ public Object[] toArray() { @Override @SuppressWarnings("unchecked") public T[] toArray(T[] a) { - if (a.length <= arr.length) { - System.arraycopy(arr, 0, (Object[])a, 0, arr.length); - return a; + if (a.length < arr.length) { + return (T[]) Arrays.copyOf(arr, arr.length, a.getClass()); } - return Arrays.copyOf((T[])arr, arr.length); + System.arraycopy(arr, 0, (Object[])a, 0, arr.length); + if (a.length > arr.length) { + a[arr.length] = null; + } + return a; } @Override diff --git a/dexlib2/src/test/java/com/android/tools/smali/util/ArraySortedSetTest.java b/dexlib2/src/test/java/com/android/tools/smali/util/ArraySortedSetTest.java index 658ad4dd..a69789bc 100644 --- a/dexlib2/src/test/java/com/android/tools/smali/util/ArraySortedSetTest.java +++ b/dexlib2/src/test/java/com/android/tools/smali/util/ArraySortedSetTest.java @@ -87,6 +87,15 @@ public void testIterator() { Assert.assertEquals(it.next(), (Integer) 3); } + @Test + public void testIsEmpty() { + ArraySortedSet set = ArraySortedSet.of(Comparator.naturalOrder(), new Integer[] { 1 }); + Assert.assertFalse(set.isEmpty()); + + ArraySortedSet emptySet = ArraySortedSet.of(Comparator.naturalOrder(), new Integer[0]); + Assert.assertTrue(emptySet.isEmpty()); + } + @Test public void testToArray() { ArraySortedSet set = ArraySortedSet.of(Comparator.naturalOrder(), @@ -96,6 +105,29 @@ public void testToArray() { }); } + @Test + public void testToArrayWithArg() { + ArraySortedSet set = ArraySortedSet.of(Comparator.naturalOrder(), + Arrays.asList(1, 2, 3)); + + // Array smaller than set + Integer[] small = new Integer[0]; + Assert.assertArrayEquals(set.toArray(small), new Integer[] { 1, 2, 3 }); + + // Array exact size + Integer[] exact = new Integer[3]; + Assert.assertArrayEquals(set.toArray(exact), new Integer[] { 1, 2, 3 }); + + // Array larger than set + Integer[] large = new Integer[] { 9, 9, 9, 9 }; + Integer[] res = set.toArray(large); + Assert.assertSame(res, large); + Assert.assertEquals(res[0], (Integer) 1); + Assert.assertEquals(res[1], (Integer) 2); + Assert.assertEquals(res[2], (Integer) 3); + Assert.assertNull(res[3]); + } + @Test(expected = UnsupportedOperationException.class) public void testAdd() { ArraySortedSet set = ArraySortedSet.of(Comparator.naturalOrder(), From 50d8457678a296f73a4952e838666bd8debd4801 Mon Sep 17 00:00:00 2001 From: David Srbecky Date: Mon, 22 Jun 2026 15:00:06 +0100 Subject: [PATCH 4/7] Add CRC32 checksum retrieval to ZipDexContainer entries Introduce ZipDexEntry interface extending DexEntry to expose the uncompressed ZIP entry CRC-32 checksum via getCrc(). Update ZipDexContainer to instantiate ZipDexEntry objects when reading ZIP archives, allowing callers to inspect entry checksums. Add unit test coverage in ZipDexContainerTest. Flag: EXEMPT PURE_REFACTOR Test: ./gradlew :dexlib2:test --tests com.android.tools.smali.dexlib2.dexbacked.ZipDexContainerTest TAG=agy CONV=92fcec37-cdaa-44ee-9f03-128bec8492e3 Change-Id: If4205d80fccafa4e80be7684ec81b3a6e46c173e --- .../dexlib2/dexbacked/ZipDexContainer.java | 74 ++++++++++------ .../dexbacked/ZipDexContainerTest.java | 86 +++++++++++++++++++ 2 files changed, 132 insertions(+), 28 deletions(-) create mode 100644 dexlib2/src/test/java/com/android/tools/smali/dexlib2/dexbacked/ZipDexContainerTest.java diff --git a/dexlib2/src/main/java/com/android/tools/smali/dexlib2/dexbacked/ZipDexContainer.java b/dexlib2/src/main/java/com/android/tools/smali/dexlib2/dexbacked/ZipDexContainer.java index 50d0bdfc..f5b57f41 100644 --- a/dexlib2/src/main/java/com/android/tools/smali/dexlib2/dexbacked/ZipDexContainer.java +++ b/dexlib2/src/main/java/com/android/tools/smali/dexlib2/dexbacked/ZipDexContainer.java @@ -59,9 +59,22 @@ */ public class ZipDexContainer implements MultiDexContainer { + /** + * Represents a dex entry in a zip container, including the uncompressed CRC-32 checksum + * of the zip entry. + */ + public interface ZipDexEntry extends DexEntry { + /** + * Gets the uncompressed CRC-32 checksum of the zip entry containing this dex file. + * + * @return The uncompressed CRC-32 checksum of the zip entry + */ + long getCrc(); + } + private final File zipFilePath; @Nullable private final Opcodes opcodes; - private TreeMap entries; + private TreeMap entries; /** * Constructs a new ZipDexContainer for the given zip file @@ -82,11 +95,11 @@ public ZipDexContainer(@Nonnull File zipFilePath, @Nullable Opcodes opcodes) { return new ArrayList<>(getEntries().keySet()); } - private Map getEntries() throws IOException { + private Map getEntries() throws IOException { if (entries != null) { return entries; } - entries = new TreeMap(); + entries = new TreeMap(); try (ZipFile zipFile = getZipFile()) { Enumeration entriesEnumeration = zipFile.entries(); @@ -97,12 +110,39 @@ private Map getEntries() throws IOException { continue; } + long entryCrc = entry.getCrc(); + // There might be several dex files in zip entry since DEX v41. try (InputStream inputStream = zipFile.getInputStream(entry)) { byte[] buf = InputStreamUtil.toByteArray(inputStream); for (int offset = 0, i = 1; offset < buf.length; i++) { DexBackedDexFile dex = new DexBackedDexFile(opcodes, buf, 0, true, offset); - entries.put(entry.getName() + (i > 1 ? ("/" + i) : ""), dex); + String entryName = entry.getName() + (i > 1 ? ("/" + i) : ""); + ZipDexEntry dexEntry = new ZipDexEntry() { + @Nonnull + @Override + public String getEntryName() { + return entryName; + } + + @Nonnull + @Override + public DexBackedDexFile getDexFile() { + return dex; + } + + @Nonnull + @Override + public MultiDexContainer getContainer() { + return ZipDexContainer.this; + } + + @Override + public long getCrc() { + return entryCrc; + } + }; + entries.put(entryName, dexEntry); offset += dex.getFileSize(); }; } @@ -119,30 +159,8 @@ private Map getEntries() throws IOException { * @return A DexEntry, or null if there is no entry with the given name * @throws NotADexFile If the entry isn't a dex file */ - @Nullable @Override public DexEntry getEntry(@Nonnull String entryName) throws IOException { - DexFile dexFile = getEntries().get(entryName); - if (dexFile == null) { - return null; - } - return new DexEntry() { - @Nonnull - @Override - public String getEntryName() { - return entryName; - } - - @Nonnull - @Override - public DexFile getDexFile() { - return dexFile; - } - - @Nonnull - @Override - public MultiDexContainer getContainer() { - return ZipDexContainer.this; - } - }; + @Nullable @Override public ZipDexEntry getEntry(@Nonnull String entryName) throws IOException { + return getEntries().get(entryName); } public boolean isZipFile() { diff --git a/dexlib2/src/test/java/com/android/tools/smali/dexlib2/dexbacked/ZipDexContainerTest.java b/dexlib2/src/test/java/com/android/tools/smali/dexlib2/dexbacked/ZipDexContainerTest.java new file mode 100644 index 00000000..c2e22e29 --- /dev/null +++ b/dexlib2/src/test/java/com/android/tools/smali/dexlib2/dexbacked/ZipDexContainerTest.java @@ -0,0 +1,86 @@ +/* + * Copyright 2026, Google LLC + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google LLC nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package com.android.tools.smali.dexlib2.dexbacked; + +import com.android.tools.smali.dexlib2.Opcodes; +import com.android.tools.smali.dexlib2.immutable.ImmutableClassDef; +import com.android.tools.smali.dexlib2.immutable.ImmutableDexFile; +import com.android.tools.smali.dexlib2.writer.io.MemoryDataStore; +import com.android.tools.smali.dexlib2.writer.pool.DexPool; +import com.google.common.collect.ImmutableSet; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +import java.io.File; +import java.io.FileOutputStream; +import java.util.List; +import java.util.zip.CRC32; +import java.util.zip.ZipEntry; +import java.util.zip.ZipOutputStream; + +@RunWith(JUnit4.class) +public class ZipDexContainerTest { + @Test + public void testZipDexEntryCrc() throws Exception { + ImmutableClassDef classDef = new ImmutableClassDef("Lorg/test/blah;", + 0, "Ljava/lang/Object;", null, null, ImmutableSet.of(), null, null); + + MemoryDataStore dataStore = new MemoryDataStore(); + DexPool.writeTo(dataStore, new ImmutableDexFile(Opcodes.getDefault(), ImmutableSet.of(classDef))); + byte[] dexBytes = dataStore.getData(); + + CRC32 crc32 = new CRC32(); + crc32.update(dexBytes); + long expectedCrc = crc32.getValue(); + + File tempZip = File.createTempFile("test", ".apk"); + tempZip.deleteOnExit(); + + try (ZipOutputStream zos = new ZipOutputStream(new FileOutputStream(tempZip))) { + ZipEntry entry = new ZipEntry("classes.dex"); + zos.putNextEntry(entry); + zos.write(dexBytes); + zos.closeEntry(); + } + + ZipDexContainer container = new ZipDexContainer(tempZip, Opcodes.getDefault()); + List entries = container.getDexEntryNames(); + Assert.assertEquals(1, entries.size()); + Assert.assertEquals("classes.dex", entries.get(0)); + + ZipDexContainer.ZipDexEntry entry = container.getEntry("classes.dex"); + Assert.assertNotNull(entry); + Assert.assertEquals("classes.dex", entry.getEntryName()); + Assert.assertEquals(expectedCrc, entry.getCrc()); + } +} From d499afec80e74cf59fa99cce1cffbefde6eb2416 Mon Sep 17 00:00:00 2001 From: David Srbecky Date: Wed, 24 Jun 2026 13:54:30 +0100 Subject: [PATCH 5/7] Don't refer to method_stack directly, but use $method instead Update smaliTreeWalker grammar definition to access totalMethodRegisters and methodParameterRegisters via ANTLR $method dynamic attribute scoping rather than referencing the generated internal method_stack stack structure directly. Change-Id: I8bceb392e69e0b71700c0da195964efacfe6680d --- third_party/smali/src/main/antlr/smaliTreeWalker.g | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/third_party/smali/src/main/antlr/smaliTreeWalker.g b/third_party/smali/src/main/antlr/smaliTreeWalker.g index 32585714..2b23afd0 100644 --- a/third_party/smali/src/main/antlr/smaliTreeWalker.g +++ b/third_party/smali/src/main/antlr/smaliTreeWalker.g @@ -98,8 +98,8 @@ import java.util.*; private byte parseRegister_nibble(String register) throws SemanticException { - int totalMethodRegisters = method_stack.peek().totalMethodRegisters; - int methodParameterRegisters = method_stack.peek().methodParameterRegisters; + int totalMethodRegisters = $method::totalMethodRegisters; + int methodParameterRegisters = $method::methodParameterRegisters; //register should be in the format "v12" int val = Byte.parseByte(register.substring(1)); @@ -116,8 +116,8 @@ import java.util.*; //return a short, because java's byte is signed private short parseRegister_byte(String register) throws SemanticException { - int totalMethodRegisters = method_stack.peek().totalMethodRegisters; - int methodParameterRegisters = method_stack.peek().methodParameterRegisters; + int totalMethodRegisters = $method::totalMethodRegisters; + int methodParameterRegisters = $method::methodParameterRegisters; //register should be in the format "v123" int val = Short.parseShort(register.substring(1)); if (register.charAt(0) == 'p') { @@ -132,8 +132,8 @@ import java.util.*; //return an int because java's short is signed private int parseRegister_short(String register) throws SemanticException { - int totalMethodRegisters = method_stack.peek().totalMethodRegisters; - int methodParameterRegisters = method_stack.peek().methodParameterRegisters; + int totalMethodRegisters = $method::totalMethodRegisters; + int methodParameterRegisters = $method::methodParameterRegisters; //register should be in the format "v12345" int val = Integer.parseInt(register.substring(1)); if (register.charAt(0) == 'p') { From db85de0731a19a0fb309a1ce0b8706414f58ca3c Mon Sep 17 00:00:00 2001 From: David Srbecky Date: Mon, 22 Jun 2026 15:39:25 +0100 Subject: [PATCH 6/7] Enforce deterministic ZipDexContainer entry order Update ZipDexContainer.getEntries() to store entries in insertion order via LinkedHashMap. Scan ZIP archives to load classes.dex, classes2.dex, etc. sequentially until classesN.dex is missing, then load remaining DEX files in lexicographically sorted order. When loading virtual DEX entries from container DEX files, check whether the entry is both isDexContainerFirstEntry and isDexContainerLastEntry; if so, omit the entry suffix (/0, /1, etc.), otherwise append the suffix. Load DEX entries only up to the container range and ignore any trailing garbage, aligning with ART's loader behavior. Add unit test coverage in ZipDexContainerTest. Flag: EXEMPT PURE_REFACTOR Test: ./gradlew :dexlib2:test --tests com.android.tools.smali.dexlib2.dexbacked.ZipDexContainerTest TAG=agy CONV=de2c422c-7039-42f4-a565-4aeb48b07bc7 Change-Id: I5550a749deda3dc283744dc46924177a2a63f0c4 --- .../dexlib2/dexbacked/DexBackedDexFile.java | 27 ++- .../dexlib2/dexbacked/ZipDexContainer.java | 114 +++++++----- .../dexbacked/ZipDexContainerTest.java | 172 +++++++++++++++++- 3 files changed, 268 insertions(+), 45 deletions(-) diff --git a/dexlib2/src/main/java/com/android/tools/smali/dexlib2/dexbacked/DexBackedDexFile.java b/dexlib2/src/main/java/com/android/tools/smali/dexlib2/dexbacked/DexBackedDexFile.java index 260ae6cd..63c26e22 100644 --- a/dexlib2/src/main/java/com/android/tools/smali/dexlib2/dexbacked/DexBackedDexFile.java +++ b/dexlib2/src/main/java/com/android/tools/smali/dexlib2/dexbacked/DexBackedDexFile.java @@ -90,8 +90,11 @@ public class DexBackedDexFile implements DexFile { private final int classStartOffset; private final int mapOffset; private final int hiddenApiRestrictionsOffset; + private final int headerOffset; + private final int containerSize; - protected DexBackedDexFile(@Nullable Opcodes opcodes, @Nonnull byte[] buf, int offset, boolean verifyMagic) { + protected DexBackedDexFile(@Nullable Opcodes opcodes, @Nonnull byte[] buf, int offset, + boolean verifyMagic) { this(opcodes, buf, offset, verifyMagic, 0); } @@ -133,9 +136,15 @@ protected DexBackedDexFile(@Nullable Opcodes opcodes, hiddenApiRestrictionsOffset = DexWriter.NO_OFFSET; } + this.headerOffset = header_offset; int container_off = 0; if (dexVersion >= 41) { - container_off = dexBuffer.readSmallUint(header_offset + HeaderItem.CONTAINER_OFF_OFFSET); + container_off = dexBuffer.readSmallUint( + header_offset + HeaderItem.CONTAINER_OFF_OFFSET); + this.containerSize = dexBuffer.readSmallUint( + header_offset + HeaderItem.CONTAINER_SIZE_OFFSET); + } else { + this.containerSize = this.fileSize; } if (container_off != header_offset) { throw new DexUtil.InvalidFile(String.format("Unexpected container offset in header")); @@ -157,6 +166,20 @@ public int getFileSize() { return fileSize; } + /** + * @return True if this is the first entry in a DEX container (or classic DEX). + */ + public boolean isDexContainerFirstEntry() { + return headerOffset == 0; + } + + /** + * @return True if this is the last entry in a DEX container, ignoring trailing garbage. + */ + public boolean isDexContainerLastEntry() { + return headerOffset + fileSize >= containerSize; + } + protected int getVersion(byte[] buf, int offset, boolean verifyMagic) { if (verifyMagic) { return DexUtil.verifyDexHeader(buf, offset); diff --git a/dexlib2/src/main/java/com/android/tools/smali/dexlib2/dexbacked/ZipDexContainer.java b/dexlib2/src/main/java/com/android/tools/smali/dexlib2/dexbacked/ZipDexContainer.java index f5b57f41..4d245486 100644 --- a/dexlib2/src/main/java/com/android/tools/smali/dexlib2/dexbacked/ZipDexContainer.java +++ b/dexlib2/src/main/java/com/android/tools/smali/dexlib2/dexbacked/ZipDexContainer.java @@ -45,10 +45,12 @@ import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; +import java.util.Collections; import java.util.Enumeration; +import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; -import java.util.TreeMap; import java.util.zip.ZipEntry; import java.util.zip.ZipFile; import javax.annotation.Nonnull; @@ -74,7 +76,7 @@ public interface ZipDexEntry extends DexEntry { private final File zipFilePath; @Nullable private final Opcodes opcodes; - private TreeMap entries; + private Map entries; /** * Constructs a new ZipDexContainer for the given zip file @@ -88,6 +90,10 @@ public ZipDexContainer(@Nonnull File zipFilePath, @Nullable Opcodes opcodes) { /** * Gets a list of the names of dex files in this zip file. + *

+ * The returned list follows a strict deterministic order: it starts with all dex files in + * classes.dex, then classes2.dex, etc. sequentially until classesN.dex is missing, followed by + * any remaining dex entries in lexicographically sorted order. * * @return A list of the names of dex files in this zip file */ @@ -99,59 +105,82 @@ private Map getEntries() throws IOException { if (entries != null) { return entries; } - entries = new TreeMap(); + // LinkedHashMap is used to preserve the strict deterministic insertion ordering of entries. + entries = new LinkedHashMap(); try (ZipFile zipFile = getZipFile()) { + Map validZipEntries = new HashMap<>(); Enumeration entriesEnumeration = zipFile.entries(); while (entriesEnumeration.hasMoreElements()) { ZipEntry entry = entriesEnumeration.nextElement(); - - if (!isDex(zipFile, entry)) { - continue; + if (isDex(zipFile, entry)) { + validZipEntries.put(entry.getName(), entry); } + } - long entryCrc = entry.getCrc(); - - // There might be several dex files in zip entry since DEX v41. - try (InputStream inputStream = zipFile.getInputStream(entry)) { - byte[] buf = InputStreamUtil.toByteArray(inputStream); - for (int offset = 0, i = 1; offset < buf.length; i++) { - DexBackedDexFile dex = new DexBackedDexFile(opcodes, buf, 0, true, offset); - String entryName = entry.getName() + (i > 1 ? ("/" + i) : ""); - ZipDexEntry dexEntry = new ZipDexEntry() { - @Nonnull - @Override - public String getEntryName() { - return entryName; - } - - @Nonnull - @Override - public DexBackedDexFile getDexFile() { - return dex; - } - - @Nonnull - @Override - public MultiDexContainer getContainer() { - return ZipDexContainer.this; - } - - @Override - public long getCrc() { - return entryCrc; - } - }; - entries.put(entryName, dexEntry); - offset += dex.getFileSize(); - }; + for (int n = 1; ; n++) { + String targetName = (n == 1) ? "classes.dex" : ("classes" + n + ".dex"); + ZipEntry entry = validZipEntries.remove(targetName); + if (entry == null) { + break; } + loadDexFilesFromZipEntry(zipFile, entry); + } + + List remainingNames = new ArrayList<>(validZipEntries.keySet()); + Collections.sort(remainingNames); + for (String remainingName : remainingNames) { + ZipEntry entry = validZipEntries.get(remainingName); + loadDexFilesFromZipEntry(zipFile, entry); } return entries; } } + private void loadDexFilesFromZipEntry(@Nonnull ZipFile zipFile, @Nonnull ZipEntry entry) + throws IOException { + long entryCrc = entry.getCrc(); + try (InputStream inputStream = zipFile.getInputStream(entry)) { + byte[] buf = InputStreamUtil.toByteArray(inputStream); + for (int offset = 0, i = 0; offset < buf.length; i++) { + DexBackedDexFile dex = new DexBackedDexFile(opcodes, buf, 0, true, offset); + boolean hasSingleEntry = dex.isDexContainerFirstEntry() && + dex.isDexContainerLastEntry(); + String entryName = entry.getName() + (hasSingleEntry ? "" : ("/" + i)); + ZipDexEntry dexEntry = new ZipDexEntry() { + @Nonnull + @Override + public String getEntryName() { + return entryName; + } + + @Nonnull + @Override + public DexBackedDexFile getDexFile() { + return dex; + } + + @Nonnull + @Override + public MultiDexContainer getContainer() { + return ZipDexContainer.this; + } + + @Override + public long getCrc() { + return entryCrc; + } + }; + entries.put(entryName, dexEntry); + offset += dex.getFileSize(); + if (dex.isDexContainerLastEntry()) { + break; + } + }; + } + } + /** * Loads a dex file from a specific named entry. * @@ -174,7 +203,8 @@ public boolean isZipFile() { // just eat it } - protected boolean isDex(@Nonnull ZipFile zipFile, @Nonnull ZipEntry zipEntry) throws IOException { + protected boolean isDex(@Nonnull ZipFile zipFile, @Nonnull ZipEntry zipEntry) + throws IOException { try (InputStream inputStream = new BufferedInputStream(zipFile.getInputStream(zipEntry))) { DexUtil.verifyDexHeader(inputStream); } catch (NotADexFile ex) { diff --git a/dexlib2/src/test/java/com/android/tools/smali/dexlib2/dexbacked/ZipDexContainerTest.java b/dexlib2/src/test/java/com/android/tools/smali/dexlib2/dexbacked/ZipDexContainerTest.java index c2e22e29..0445650e 100644 --- a/dexlib2/src/test/java/com/android/tools/smali/dexlib2/dexbacked/ZipDexContainerTest.java +++ b/dexlib2/src/test/java/com/android/tools/smali/dexlib2/dexbacked/ZipDexContainerTest.java @@ -31,6 +31,7 @@ package com.android.tools.smali.dexlib2.dexbacked; import com.android.tools.smali.dexlib2.Opcodes; +import com.android.tools.smali.dexlib2.dexbacked.raw.HeaderItem; import com.android.tools.smali.dexlib2.immutable.ImmutableClassDef; import com.android.tools.smali.dexlib2.immutable.ImmutableDexFile; import com.android.tools.smali.dexlib2.writer.io.MemoryDataStore; @@ -56,7 +57,8 @@ public void testZipDexEntryCrc() throws Exception { 0, "Ljava/lang/Object;", null, null, ImmutableSet.of(), null, null); MemoryDataStore dataStore = new MemoryDataStore(); - DexPool.writeTo(dataStore, new ImmutableDexFile(Opcodes.getDefault(), ImmutableSet.of(classDef))); + DexPool.writeTo(dataStore, + new ImmutableDexFile(Opcodes.getDefault(), ImmutableSet.of(classDef))); byte[] dexBytes = dataStore.getData(); CRC32 crc32 = new CRC32(); @@ -83,4 +85,172 @@ public void testZipDexEntryCrc() throws Exception { Assert.assertEquals("classes.dex", entry.getEntryName()); Assert.assertEquals(expectedCrc, entry.getCrc()); } + + @Test + public void testZipDexEntryDeterministicOrder() throws Exception { + ImmutableClassDef classDef = new ImmutableClassDef("Lorg/test/blah;", + 0, "Ljava/lang/Object;", null, null, ImmutableSet.of(), null, null); + + MemoryDataStore dataStore = new MemoryDataStore(); + DexPool.writeTo(dataStore, + new ImmutableDexFile(Opcodes.getDefault(), ImmutableSet.of(classDef))); + byte[] dexBytes = dataStore.getData(); + + File tempZip = File.createTempFile("test_order", ".apk"); + tempZip.deleteOnExit(); + + // Write entries out of order: z.dex, classes2.dex, classes4.dex, classes.dex, a.dex + try (ZipOutputStream zos = new ZipOutputStream(new FileOutputStream(tempZip))) { + for (String name : new String[]{ + "z.dex", "classes2.dex", "classes4.dex", "classes.dex", "a.dex"}) { + ZipEntry entry = new ZipEntry(name); + zos.putNextEntry(entry); + zos.write(dexBytes); + zos.closeEntry(); + } + } + + ZipDexContainer container = new ZipDexContainer(tempZip, Opcodes.getDefault()); + List entries = container.getDexEntryNames(); + + // Expected order: classes.dex, classes2.dex (classes3 missing), then a, classes4, z.dex + String[] expectedOrder = new String[]{ + "classes.dex", "classes2.dex", "a.dex", "classes4.dex", "z.dex"}; + Assert.assertEquals(expectedOrder.length, entries.size()); + + for (int i = 0; i < expectedOrder.length; i++) { + Assert.assertEquals(expectedOrder[i], entries.get(i)); + ZipDexContainer.ZipDexEntry entry = container.getEntry(expectedOrder[i]); + Assert.assertNotNull(entry); + } + } + + @Test + public void testZipDexContainerContainerDex() throws Exception { + ImmutableClassDef classDef = new ImmutableClassDef("Lorg/test/blah;", + 0, "Ljava/lang/Object;", null, null, ImmutableSet.of(), null, null); + + MemoryDataStore dataStore = new MemoryDataStore(); + DexPool.writeTo(dataStore, + new ImmutableDexFile(Opcodes.getDefault(), ImmutableSet.of(classDef))); + byte[] dexBytes = dataStore.getData(); + + byte[] containerBytes = new byte[dexBytes.length * 2]; + System.arraycopy(dexBytes, 0, containerBytes, 0, dexBytes.length); + System.arraycopy(dexBytes, 0, containerBytes, dexBytes.length, dexBytes.length); + + for (int headerOffset : new int[]{0, dexBytes.length}) { + containerBytes[headerOffset + 4] = '0'; + containerBytes[headerOffset + 5] = '4'; + containerBytes[headerOffset + 6] = '1'; + writeInt(containerBytes, + headerOffset + HeaderItem.CONTAINER_SIZE_OFFSET, containerBytes.length); + writeInt(containerBytes, + headerOffset + HeaderItem.CONTAINER_OFF_OFFSET, headerOffset); + } + + File tempZip = File.createTempFile("test_container", ".apk"); + tempZip.deleteOnExit(); + + try (ZipOutputStream zos = new ZipOutputStream(new FileOutputStream(tempZip))) { + ZipEntry entry = new ZipEntry("classes.dex"); + zos.putNextEntry(entry); + zos.write(containerBytes); + zos.closeEntry(); + } + + ZipDexContainer container = new ZipDexContainer(tempZip, Opcodes.forDexVersion(41)); + List entries = container.getDexEntryNames(); + Assert.assertEquals(2, entries.size()); + Assert.assertEquals("classes.dex/0", entries.get(0)); + Assert.assertEquals("classes.dex/1", entries.get(1)); + + Assert.assertNotNull(container.getEntry("classes.dex/0")); + Assert.assertNotNull(container.getEntry("classes.dex/1")); + } + + @Test + public void testZipDexContainerIgnoreTrailingGarbage() throws Exception { + ImmutableClassDef classDef = new ImmutableClassDef("Lorg/test/blah;", + 0, "Ljava/lang/Object;", null, null, ImmutableSet.of(), null, null); + + MemoryDataStore dataStore = new MemoryDataStore(); + DexPool.writeTo(dataStore, + new ImmutableDexFile(Opcodes.getDefault(), ImmutableSet.of(classDef))); + byte[] dexBytes = dataStore.getData(); + + byte[] containerBytesWithGarbage = new byte[dexBytes.length * 2 + 50]; + System.arraycopy(dexBytes, 0, containerBytesWithGarbage, 0, dexBytes.length); + System.arraycopy(dexBytes, 0, + containerBytesWithGarbage, dexBytes.length, dexBytes.length); + + for (int headerOffset : new int[]{0, dexBytes.length}) { + containerBytesWithGarbage[headerOffset + 4] = '0'; + containerBytesWithGarbage[headerOffset + 5] = '4'; + containerBytesWithGarbage[headerOffset + 6] = '1'; + writeInt(containerBytesWithGarbage, + headerOffset + HeaderItem.CONTAINER_SIZE_OFFSET, dexBytes.length * 2); + writeInt(containerBytesWithGarbage, + headerOffset + HeaderItem.CONTAINER_OFF_OFFSET, headerOffset); + } + + File tempZip = File.createTempFile("test_garbage", ".apk"); + tempZip.deleteOnExit(); + + try (ZipOutputStream zos = new ZipOutputStream(new FileOutputStream(tempZip))) { + ZipEntry entry = new ZipEntry("classes.dex"); + zos.putNextEntry(entry); + zos.write(containerBytesWithGarbage); + zos.closeEntry(); + } + + ZipDexContainer container = new ZipDexContainer(tempZip, Opcodes.forDexVersion(41)); + List entries = container.getDexEntryNames(); + Assert.assertEquals(2, entries.size()); + Assert.assertEquals("classes.dex/0", entries.get(0)); + Assert.assertEquals("classes.dex/1", entries.get(1)); + } + + @Test + public void testZipDexContainerSingleEntryContainerDex() throws Exception { + ImmutableClassDef classDef = new ImmutableClassDef("Lorg/test/blah;", + 0, "Ljava/lang/Object;", null, null, ImmutableSet.of(), null, null); + + MemoryDataStore dataStore = new MemoryDataStore(); + DexPool.writeTo(dataStore, + new ImmutableDexFile(Opcodes.getDefault(), ImmutableSet.of(classDef))); + byte[] dexBytes = dataStore.getData(); + + byte[] containerBytes = new byte[dexBytes.length]; + System.arraycopy(dexBytes, 0, containerBytes, 0, dexBytes.length); + + containerBytes[4] = '0'; + containerBytes[5] = '4'; + containerBytes[6] = '1'; + writeInt(containerBytes, HeaderItem.CONTAINER_SIZE_OFFSET, containerBytes.length); + writeInt(containerBytes, HeaderItem.CONTAINER_OFF_OFFSET, 0); + + File tempZip = File.createTempFile("test_single_container", ".apk"); + tempZip.deleteOnExit(); + + try (ZipOutputStream zos = new ZipOutputStream(new FileOutputStream(tempZip))) { + ZipEntry entry = new ZipEntry("classes.dex"); + zos.putNextEntry(entry); + zos.write(containerBytes); + zos.closeEntry(); + } + + ZipDexContainer container = new ZipDexContainer(tempZip, Opcodes.forDexVersion(41)); + List entries = container.getDexEntryNames(); + Assert.assertEquals(1, entries.size()); + Assert.assertEquals("classes.dex", entries.get(0)); + Assert.assertNotNull(container.getEntry("classes.dex")); + } + + private static void writeInt(byte[] buf, int offset, int value) { + buf[offset] = (byte) value; + buf[offset + 1] = (byte) (value >> 8); + buf[offset + 2] = (byte) (value >> 16); + buf[offset + 3] = (byte) (value >> 24); + } } From 022c58df1bfef9131a83e19d295cd14c87e4da8c Mon Sep 17 00:00:00 2001 From: David Srbecky Date: Tue, 23 Jun 2026 18:46:13 +0100 Subject: [PATCH 7/7] Harden DexBackedDexFile against malformed DEX Harden DexBackedDexFile against malformed and corrupted DEX files to prevent unchecked runtime crashes (such as OutOfBounds exceptions). Ensure buffer capacity checks guard magic parsing and version checking. Align constructor validation to enforce minimum header sizes and bounds checking relative to header_offset. Uphold specification constraints on container DEX files (v41+) and lazy load map items on demand. Test: ./gradlew :dexlib2:test Flag: EXEMPT BUGFIX TAG=agy CONV=af5f83f4-d40c-419f-9452-99168f7eb18f Change-Id: I15a7c9226aa61ec44d086c2e5daa6cf34b9f0987 --- .../dexlib2/dexbacked/DexBackedDexFile.java | 79 ++++++++++++++----- .../dexlib2/dexbacked/raw/HeaderItem.java | 8 +- .../tools/smali/dexlib2/util/DexUtil.java | 14 +++- .../dexbacked/ZipDexContainerTest.java | 6 +- 4 files changed, 81 insertions(+), 26 deletions(-) diff --git a/dexlib2/src/main/java/com/android/tools/smali/dexlib2/dexbacked/DexBackedDexFile.java b/dexlib2/src/main/java/com/android/tools/smali/dexlib2/dexbacked/DexBackedDexFile.java index 63c26e22..b3d24bc9 100644 --- a/dexlib2/src/main/java/com/android/tools/smali/dexlib2/dexbacked/DexBackedDexFile.java +++ b/dexlib2/src/main/java/com/android/tools/smali/dexlib2/dexbacked/DexBackedDexFile.java @@ -89,7 +89,7 @@ public class DexBackedDexFile implements DexFile { private final int classCount; private final int classStartOffset; private final int mapOffset; - private final int hiddenApiRestrictionsOffset; + private int hiddenApiRestrictionsOffset = -1; private final int headerOffset; private final int containerSize; @@ -103,10 +103,20 @@ protected DexBackedDexFile(@Nullable Opcodes opcodes, int offset, boolean verifyMagic, int header_offset) { + if (offset < 0 || offset > buf.length) { + throw new IndexOutOfBoundsException( + String.format("Invalid offset %d for buffer of length %d", offset, buf.length)); + } + if (header_offset < 0 || buf.length - offset < header_offset) { + throw new IndexOutOfBoundsException(String.format( + "Invalid header_offset %d for buffer of length %d at offset %d", + header_offset, buf.length, offset)); + } + dexBuffer = new DexBuffer(buf, offset); dataBuffer = new DexBuffer(buf, offset + getBaseDataOffset()); - int dexVersion = getVersion(buf, offset, verifyMagic); + int dexVersion = getVersion(buf, offset + header_offset, verifyMagic); if (opcodes == null) { this.opcodes = getDefaultOpcodes(dexVersion); @@ -114,7 +124,24 @@ protected DexBackedDexFile(@Nullable Opcodes opcodes, this.opcodes = opcodes; } + int requiredHeaderSize = (dexVersion >= 41) ? 120 : 112; + if (buf.length - offset - header_offset < requiredHeaderSize) { + throw new DexUtil.InvalidFile(String.format( + "File is too short to contain a valid dex header (size: %d, required: %d)", + buf.length - offset - header_offset, requiredHeaderSize)); + } + fileSize = dexBuffer.readSmallUint(header_offset + HeaderItem.FILE_SIZE_OFFSET); + if (fileSize < requiredHeaderSize) { + throw new DexUtil.InvalidFile(String.format( + "Invalid file_size %d (smaller than required header size %d)", + fileSize, requiredHeaderSize)); + } + if (fileSize > buf.length - offset - header_offset) { + throw new DexUtil.InvalidFile(String.format( + "Invalid file_size %d (exceeds remaining buffer length %d)", + fileSize, buf.length - offset - header_offset)); + } stringCount = dexBuffer.readSmallUint(header_offset + HeaderItem.STRING_COUNT_OFFSET); stringStartOffset = dexBuffer.readSmallUint(header_offset + HeaderItem.STRING_START_OFFSET); typeCount = dexBuffer.readSmallUint(header_offset + HeaderItem.TYPE_COUNT_OFFSET); @@ -129,25 +156,28 @@ protected DexBackedDexFile(@Nullable Opcodes opcodes, classStartOffset = dexBuffer.readSmallUint(header_offset + HeaderItem.CLASS_START_OFFSET); mapOffset = dexBuffer.readSmallUint(header_offset + HeaderItem.MAP_OFFSET); - MapItem mapItem = getMapItemForSection(ItemType.HIDDENAPI_CLASS_DATA_ITEM); - if (mapItem != null) { - hiddenApiRestrictionsOffset = mapItem.getOffset(); - } else { - hiddenApiRestrictionsOffset = DexWriter.NO_OFFSET; - } - this.headerOffset = header_offset; int container_off = 0; if (dexVersion >= 41) { - container_off = dexBuffer.readSmallUint( - header_offset + HeaderItem.CONTAINER_OFF_OFFSET); - this.containerSize = dexBuffer.readSmallUint( - header_offset + HeaderItem.CONTAINER_SIZE_OFFSET); + container_off = dexBuffer.readSmallUint( + header_offset + HeaderItem.HEADER_OFFSET_OFFSET); + this.containerSize = dexBuffer.readSmallUint( + header_offset + HeaderItem.CONTAINER_SIZE_OFFSET); + if (this.containerSize < header_offset + fileSize) { + throw new DexUtil.InvalidFile(String.format( + "DEX entry (header_offset: %d, file_size: %d) exceeds container_size %d", + header_offset, fileSize, containerSize)); + } + if (this.containerSize > buf.length - offset) { + throw new DexUtil.InvalidFile(String.format( + "Invalid container_size %d (exceeds buffer length %d)", + containerSize, buf.length - offset)); + } } else { - this.containerSize = this.fileSize; + this.containerSize = this.fileSize; } if (container_off != header_offset) { - throw new DexUtil.InvalidFile(String.format("Unexpected container offset in header")); + throw new DexUtil.InvalidFile(String.format("Unexpected container offset in header")); } } @@ -586,20 +616,33 @@ protected DexBackedMethodImplementation createMethodImplementation( return new DexBackedMethodImplementation(dexFile, method, codeOffset); } + private int getHiddenApiRestrictionsOffset() { + if (hiddenApiRestrictionsOffset == -1) { + MapItem mapItem = getMapItemForSection(ItemType.HIDDENAPI_CLASS_DATA_ITEM); + if (mapItem != null) { + hiddenApiRestrictionsOffset = mapItem.getOffset(); + } else { + hiddenApiRestrictionsOffset = DexWriter.NO_OFFSET; + } + } + return hiddenApiRestrictionsOffset; + } + private int readHiddenApiRestrictionsOffset(int classIndex) { - if (hiddenApiRestrictionsOffset == DexWriter.NO_OFFSET) { + int restrictionsOffset = getHiddenApiRestrictionsOffset(); + if (restrictionsOffset == DexWriter.NO_OFFSET) { return DexWriter.NO_OFFSET; } int offset = dexBuffer.readInt( - hiddenApiRestrictionsOffset + + restrictionsOffset + HiddenApiClassDataItem.OFFSETS_LIST_OFFSET + classIndex * HiddenApiClassDataItem.OFFSET_ITEM_SIZE); if (offset == DexWriter.NO_OFFSET) { return DexWriter.NO_OFFSET; } - return hiddenApiRestrictionsOffset + offset; + return restrictionsOffset + offset; } public static abstract class OptionalIndexedSection extends IndexedSection { diff --git a/dexlib2/src/main/java/com/android/tools/smali/dexlib2/dexbacked/raw/HeaderItem.java b/dexlib2/src/main/java/com/android/tools/smali/dexlib2/dexbacked/raw/HeaderItem.java index 56bb992f..2f6d04ec 100644 --- a/dexlib2/src/main/java/com/android/tools/smali/dexlib2/dexbacked/raw/HeaderItem.java +++ b/dexlib2/src/main/java/com/android/tools/smali/dexlib2/dexbacked/raw/HeaderItem.java @@ -43,6 +43,7 @@ public class HeaderItem { public static final int ITEM_SIZE = 0x70; + public static final int MAGIC_SIZE = 8; private static final byte[] MAGIC_VALUE = new byte[] { 0x64, 0x65, 0x78, 0x0a, 0x00, 0x00, 0x00, 0x00 }; @@ -88,7 +89,7 @@ public class HeaderItem { public static final int DATA_START_OFFSET = 108; public static final int CONTAINER_SIZE_OFFSET = 112; - public static final int CONTAINER_OFF_OFFSET = 116; + public static final int HEADER_OFFSET_OFFSET = 116; @Nonnull private DexBackedDexFile dexFile; @@ -267,7 +268,7 @@ public static byte[] getMagicForDexVersion(int dexVersion) { * @return True if the magic value is valid */ public static boolean verifyMagic(byte[] buf, int offset) { - if (buf.length - offset < 8) { + if (offset < 0 || offset > buf.length || buf.length - offset < MAGIC_SIZE) { return false; } @@ -297,6 +298,9 @@ public static boolean verifyMagic(byte[] buf, int offset) { * @return The dex version if the header is valid or -1 if the header is invalid */ public static int getVersion(byte[] buf, int offset) { + if (offset < 0 || offset > buf.length || buf.length - offset < MAGIC_SIZE) { + throw new DexBackedDexFile.NotADexFile("File is too short"); + } if (!verifyMagic(buf, offset)) { return -1; } diff --git a/dexlib2/src/main/java/com/android/tools/smali/dexlib2/util/DexUtil.java b/dexlib2/src/main/java/com/android/tools/smali/dexlib2/util/DexUtil.java index 9bbfc11d..24c546b5 100644 --- a/dexlib2/src/main/java/com/android/tools/smali/dexlib2/util/DexUtil.java +++ b/dexlib2/src/main/java/com/android/tools/smali/dexlib2/util/DexUtil.java @@ -83,17 +83,25 @@ public static int verifyDexHeader(@Nonnull InputStream inputStream) throws IOExc * @throws UnsupportedFile If the dex header is valid, but uses unsupported functionality */ public static int verifyDexHeader(@Nonnull byte[] buf, int offset) { + if (offset < 0 || offset > buf.length || buf.length - offset < HeaderItem.MAGIC_SIZE) { + throw new NotADexFile("File is too short"); + } int dexVersion = HeaderItem.getVersion(buf, offset); if (dexVersion == -1) { StringBuilder sb = new StringBuilder("Not a valid dex magic value:"); - for (int i=0; i<8; i++) { - sb.append(String.format(" %02x", buf[i])); + for (int i=0; i