From 866bccfb3a2580ed500a8b262f91410a569069f8 Mon Sep 17 00:00:00 2001 From: David Srbecky Date: Thu, 16 Jul 2026 14:48:13 +0100 Subject: [PATCH] Fix CDEX containerSize and mapOffset validation in DexBackedDexFile Commit e99622b8 introduced validation of mapOffset against containerSize. For Compact DEX (CDEX) files: 1. containerSize was capped at fileSize (which covers only the unshared section header and ID tables, excluding the data section). 2. mapOffset is relative to baseDataOffset (the data section), but mapSize was eagerly read from dexBuffer instead of dataBuffer, reading garbage data from absolute offset mapOffset. Update containerSize calculation for CDEX headers to encompass dataStart + dataSize, use dataBuffer to read mapSize at mapOffset, and use overflow-safe bounds checking. TAG=agy CONV=63f64982-065a-4b9f-a2d3-6fa7ad565aa6 Bug: 535190835 Bug: 285129432 Test: ./gradlew :dexlib2:test --tests com.android.tools.smali.dexlib2.dexbacked.CDexTest Flag: EXEMPT BUGFIX Change-Id: I6249049af4d31d13cf0b66f36fbb0e2dd9dcb55a --- .../dexlib2/dexbacked/DexBackedDexFile.java | 14 ++- .../smali/dexlib2/dexbacked/CDexTest.java | 85 +++++++++++++++++++ 2 files changed, 97 insertions(+), 2 deletions(-) create mode 100644 dexlib2/src/test/java/com/android/tools/smali/dexlib2/dexbacked/CDexTest.java 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 4772f306..9f57db40 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 @@ -174,6 +174,10 @@ protected DexBackedDexFile(@Nullable Opcodes opcodes, "Invalid container_size %d (exceeds buffer length %d)", containerSize, buf.length - offset)); } + } else if (CDexBackedDexFile.isCdex(buf, offset + header_offset)) { + int dataStart = dexBuffer.readSmallUint(header_offset + HeaderItem.DATA_START_OFFSET); + int dataSize = dexBuffer.readSmallUint(header_offset + HeaderItem.DATA_SIZE_OFFSET); + this.containerSize = Math.min(buf.length - offset, Math.max(this.fileSize, dataStart + dataSize)); } else { this.containerSize = this.fileSize; } @@ -185,9 +189,15 @@ protected DexBackedDexFile(@Nullable Opcodes opcodes, throw new DexUtil.InvalidFile(String.format("Invalid mapOffset %d", mapOffset)); } + int baseDataOffset = getBaseDataOffset(); + if (baseDataOffset > containerSize - 4 - mapOffset) { + throw new DexUtil.InvalidFile(String.format("Invalid mapOffset %d", mapOffset)); + } + // Eagerly read mapSize to validate map bounds - int mapSize = dexBuffer.readSmallUint(mapOffset); - if (mapOffset + 4 + mapSize * MapItem.ITEM_SIZE > containerSize) { + int mapSize = dataBuffer.readSmallUint(mapOffset); + int remainingSize = containerSize - baseDataOffset - mapOffset - 4; + if (mapSize > remainingSize / MapItem.ITEM_SIZE) { throw new DexUtil.InvalidFile("Map extends beyond container bounds"); } } diff --git a/dexlib2/src/test/java/com/android/tools/smali/dexlib2/dexbacked/CDexTest.java b/dexlib2/src/test/java/com/android/tools/smali/dexlib2/dexbacked/CDexTest.java new file mode 100644 index 00000000..4838f174 --- /dev/null +++ b/dexlib2/src/test/java/com/android/tools/smali/dexlib2/dexbacked/CDexTest.java @@ -0,0 +1,85 @@ +/* + * 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.dexbacked.raw.HeaderItem; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public class CDexTest { + + @Test + public void testSyntheticCdexHeader() throws Exception { + // CDEX header layout (112 bytes min header): + // 0-7: magic "cdex001\0" + // 32-35: file_size = 112 (0x70) + // 36-39: header_size = 112 (0x70) + // 40-43: endian_tag = 0x12345678 + // 52-55: map_off = 120 (0x78) <- Notice map_off is relative to data_off (112) + // 104-107: data_size = 128 + // 108-111: data_off = 112 + // + // In CDEX, map item is at absolute offset data_off + map_off = 112 + 120 = 232. + // At absolute offset map_off (120), write invalid large uint (0xFFFFFFFF) to ensure + // mapSize is read from dataBuffer (offset 232) and NOT dexBuffer (offset 120). + byte[] buf = new byte[256]; + byte[] magic = new byte[]{'c', 'd', 'e', 'x', '0', '0', '1', 0}; + System.arraycopy(magic, 0, buf, 0, 8); + int fileSize = 112; + int dataStart = 112; + int dataSize = 128; + int mapOffset = 120; + + writeInt(buf, HeaderItem.FILE_SIZE_OFFSET, fileSize); + writeInt(buf, HeaderItem.HEADER_SIZE_OFFSET, fileSize); + writeInt(buf, HeaderItem.ENDIAN_TAG_OFFSET, HeaderItem.LITTLE_ENDIAN_TAG); + writeInt(buf, HeaderItem.MAP_OFFSET, mapOffset); + writeInt(buf, HeaderItem.DATA_SIZE_OFFSET, dataSize); + writeInt(buf, HeaderItem.DATA_START_OFFSET, dataStart); + + writeInt(buf, mapOffset, 0xFFFFFFFF); // Trap if reading from dexBuffer at mapOffset (120) + writeInt(buf, dataStart + mapOffset, 0); // Correct map_size = 0 at dataStart + mapOffset (232) + + CDexBackedDexFile cdex = new CDexBackedDexFile(null, buf, 0, false); + Assert.assertNotNull(cdex); + } + + 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); + } +}