Cause dex verification to fail if the class_defs section contains more
than one definition for any given class.
Bonus: Killed off a bunch of trailing whitespace, to reduce the gerrit
bloodbath.
Change-Id: Idc599364f1a38887fb4b9f91dc91b9b53343ca03
Bug: 2382215
diff --git a/libdex/DexSwapVerify.c b/libdex/DexSwapVerify.c
index bc6f51f..400e766 100644
--- a/libdex/DexSwapVerify.c
+++ b/libdex/DexSwapVerify.c
@@ -69,11 +69,19 @@
*/
typedef struct CheckState {
const DexHeader* pHeader;
- const u1* fileStart;
+ const u1* fileStart;
const u1* fileEnd; // points to fileStart + fileLen
u4 fileLen;
DexDataMap* pDataMap; // set after map verification
const DexFile* pDexFile; // set after intraitem verification
+
+ /*
+ * bitmap of type_id indices that have been used to define classes;
+ * initialized immediately before class_def cross-verification, and
+ * freed immediately after it
+ */
+ u4* pDefinedClassBits;
+
const void* previousItem; // set during section iteration
} CheckState;
@@ -235,12 +243,36 @@
}
/*
+ * Calculate the required size (in elements) of the array pointed at by
+ * pDefinedClassBits.
+ */
+static size_t calcDefinedClassBitsSize(const CheckState* state)
+{
+ // Divide typeIdsSize by 32 (0x20), rounding up.
+ return (state->pHeader->typeIdsSize + 0x1f) >> 5;
+}
+
+/*
+ * Set the given bit in pDefinedClassBits, returning its former value.
+ */
+static bool setDefinedClassBit(const CheckState* state, u4 typeIdx) {
+ u4 arrayIdx = typeIdx >> 5;
+ u4 bit = 1 << (typeIdx & 0x1f);
+ u4* element = &state->pDefinedClassBits[arrayIdx];
+ bool result = (*element & bit) != 0;
+
+ *element |= bit;
+
+ return result;
+}
+
+/*
* Swap the header_item.
*/
static bool swapDexHeader(const CheckState* state, DexHeader* pHeader)
{
CHECK_PTR_RANGE(pHeader, pHeader + 1);
-
+
// magic is ok
SWAP_FIELD4(pHeader->checksum);
// signature is ok
@@ -371,7 +403,7 @@
SWAP_FIELD4(pMap->size);
count = pMap->size;
-
+
CHECK_LIST_SIZE(item, count, sizeof(DexMapItem));
while (count--) {
@@ -485,7 +517,7 @@
LOGE("Unable to allocate data map (size 0x%x)\n", dataItemCount);
return false;
}
-
+
return true;
}
@@ -644,7 +676,7 @@
item->parametersOff, kDexTypeTypeList)) {
return NULL;
}
-
+
if (!shortyDescMatch(*shorty,
dexStringByTypeIdx(state->pDexFile, item->returnTypeIdx),
true)) {
@@ -681,7 +713,7 @@
LOGE("Shorty is too long\n");
return NULL;
}
-
+
const DexProtoId* item0 = state->previousItem;
if (item0 != NULL) {
// Check ordering. This relies on type_ids being in order.
@@ -699,7 +731,7 @@
for (;;) {
u4 idx0 = dexParameterIteratorNextIndex(&iterator0);
u4 idx1 = dexParameterIteratorNextIndex(&iterator);
-
+
if (idx1 == kDexNoIndex) {
badOrder = true;
break;
@@ -743,7 +775,7 @@
static void* crossVerifyFieldIdItem(const CheckState* state, void* ptr) {
const DexFieldId* item = ptr;
const char* s;
-
+
s = dexStringByTypeIdx(state->pDexFile, item->classIdx);
if (!dexIsClassDescriptor(s)) {
LOGE("Invalid descriptor for class_idx: '%s'\n", s);
@@ -802,7 +834,7 @@
/* Perform byte-swapping and intra-item verification on method_id_item. */
static void* swapMethodIdItem(const CheckState* state, void* ptr) {
DexMethodId* item = ptr;
-
+
CHECK_PTR_RANGE(item, item + 1);
SWAP_INDEX2(item->classIdx, state->pHeader->typeIdsSize);
SWAP_INDEX2(item->protoIdx, state->pHeader->protoIdsSize);
@@ -910,7 +942,7 @@
*/
u4 dataDefiner = findFirstClassDataDefiner(state, classData);
bool result = (dataDefiner == definerIdx) || (dataDefiner == kDexNoIndex);
-
+
free(classData);
return result;
}
@@ -933,14 +965,19 @@
/* Perform cross-item verification of class_def_item. */
static void* crossVerifyClassDefItem(const CheckState* state, void* ptr) {
const DexClassDef* item = ptr;
- const char* descriptor =
- dexStringByTypeIdx(state->pDexFile, item->classIdx);
+ u4 classIdx = item->classIdx;
+ const char* descriptor = dexStringByTypeIdx(state->pDexFile, classIdx);
if (!dexIsClassDescriptor(descriptor)) {
LOGE("Invalid class: '%s'\n", descriptor);
return NULL;
}
+ if (setDefinedClassBit(state, classIdx)) {
+ LOGE("Duplicate class definition: '%s'\n", descriptor);
+ return NULL;
+ }
+
bool okay =
dexDataMapVerify0Ok(state->pDataMap,
item->interfacesOff, kDexTypeTypeList)
@@ -1209,7 +1246,7 @@
static u4 findFirstAnnotationsDirectoryDefiner(const CheckState* state,
const DexAnnotationsDirectoryItem* dir) {
if (dir->fieldsSize != 0) {
- const DexFieldAnnotationsItem* fields =
+ const DexFieldAnnotationsItem* fields =
dexGetFieldAnnotations(state->pDexFile, dir);
const DexFieldId* field =
dexGetFieldId(state->pDexFile, fields[0].fieldIdx);
@@ -1217,7 +1254,7 @@
}
if (dir->methodsSize != 0) {
- const DexMethodAnnotationsItem* methods =
+ const DexMethodAnnotationsItem* methods =
dexGetMethodAnnotations(state->pDexFile, dir);
const DexMethodId* method =
dexGetMethodId(state->pDexFile, methods[0].methodIdx);
@@ -1225,7 +1262,7 @@
}
if (dir->parametersSize != 0) {
- const DexParameterAnnotationsItem* parameters =
+ const DexParameterAnnotationsItem* parameters =
dexGetParameterAnnotations(state->pDexFile, dir);
const DexMethodId* method =
dexGetMethodId(state->pDexFile, parameters[0].methodIdx);
@@ -1362,7 +1399,7 @@
const u1* data = item->annotation;
return readUnsignedLeb128(&data);
}
-
+
/* Perform cross-item verification of annotation_set_item. */
static void* crossVerifyAnnotationSetItem(const CheckState* state, void* ptr) {
const DexAnnotationSetItem* set = ptr;
@@ -1376,7 +1413,7 @@
dexGetAnnotationOff(set, i), kDexTypeAnnotationItem)) {
return NULL;
}
-
+
const DexAnnotationItem* annotation =
dexGetAnnotationItem(state->pDexFile, set, i);
u4 idx = annotationItemTypeIdx(annotation);
@@ -1443,7 +1480,7 @@
return false;
}
- if (((accessFlags & ~ACC_METHOD_MASK) != 0)
+ if (((accessFlags & ~ACC_METHOD_MASK) != 0)
|| (isSynchronized && !allowSynchronized)) {
LOGE("Bogus method access flags %x @ %d\n", accessFlags, i);
return false;
@@ -1469,7 +1506,7 @@
static bool verifyClassDataItem0(const CheckState* state,
DexClassData* classData) {
bool okay;
-
+
okay = verifyFields(state, classData->header.staticFieldsSize,
classData->staticFields, true);
@@ -1477,7 +1514,7 @@
LOGE("Trouble with static fields\n");
return false;
}
-
+
verifyFields(state, classData->header.instanceFieldsSize,
classData->instanceFields, false);
@@ -1488,7 +1525,7 @@
okay = verifyMethods(state, classData->header.directMethodsSize,
classData->directMethods, true);
-
+
if (!okay) {
LOGE("Trouble with direct methods\n");
return false;
@@ -1496,7 +1533,7 @@
okay = verifyMethods(state, classData->header.virtualMethodsSize,
classData->virtualMethods, false);
-
+
if (!okay) {
LOGE("Trouble with virtual methods\n");
return false;
@@ -1522,7 +1559,7 @@
if (!okay) {
return NULL;
}
-
+
return (void*) data;
}
@@ -1557,7 +1594,7 @@
return kDexNoIndex;
}
-
+
/* Perform cross-item verification of class_data_item. */
static void* crossVerifyClassDataItem(const CheckState* state, void* ptr) {
const u1* data = ptr;
@@ -1593,13 +1630,13 @@
kDexTypeCodeItem)
&& verifyMethodDefiner(state, definingClass, meth->methodIdx);
}
-
+
free(classData);
if (!okay) {
return NULL;
}
-
+
return (void*) data;
}
@@ -1635,7 +1672,7 @@
} else {
catchAll = false;
}
-
+
handlerOffs[i] = offset;
while (size-- > 0) {
@@ -1700,9 +1737,9 @@
LOGE("Invalid handlers_size: %d\n", handlersSize);
return NULL;
}
-
+
u4 handlerOffs[handlersSize]; // list of valid handlerOff values
- u4 endOffset = setHandlerOffsAndVerify(state, code,
+ u4 endOffset = setHandlerOffsAndVerify(state, code,
encodedPtr - encodedHandlers,
handlersSize, handlerOffs);
@@ -1747,11 +1784,11 @@
lastEnd = tries->startAddr + tries->insnCount;
if (lastEnd > code->insnsSize) {
- LOGE("Invalid insn_count: 0x%x (end addr 0x%x)\n",
+ LOGE("Invalid insn_count: 0x%x (end addr 0x%x)\n",
tries->insnCount, lastEnd);
return NULL;
}
-
+
tries++;
}
@@ -1816,7 +1853,7 @@
LOGE("String data would go beyond end-of-file\n");
return NULL;
}
-
+
u1 byte1 = *(data++);
// Switch on the high four bits.
@@ -2037,7 +2074,7 @@
return NULL;
}
}
-
+
return (void*) data;
}
@@ -2056,13 +2093,13 @@
u4 i;
CHECK_PTR_RANGE(data, data + size);
-
+
for (i = 0; i < size; i++) {
result |= ((u4) *(data++)) << (i * 8);
}
*pData = data;
- return result;
+ return result;
}
/* Helper for *VerifyAnnotationItem() and *VerifyEncodedArrayItem(), which
@@ -2093,7 +2130,7 @@
static const u1* verifyEncodedValue(const CheckState* state,
const u1* data, bool crossVerify) {
CHECK_PTR_RANGE(data, data + 1);
-
+
u1 headerByte = *(data++);
u4 valueType = headerByte & kDexAnnotationValueTypeMask;
u4 valueArg = headerByte >> kDexAnnotationValueArgShift;
@@ -2230,7 +2267,7 @@
return NULL;
}
}
-
+
u4 size = readAndVerifyUnsignedLeb128(&data, fileEnd, &okay);
u4 lastIdx = 0;
bool first = true;
@@ -2242,7 +2279,7 @@
while (size--) {
idx = readAndVerifyUnsignedLeb128(&data, fileEnd, &okay);
-
+
if (!okay) {
LOGE("Bogus encoded_annotation name_idx\n");
return NULL;
@@ -2287,7 +2324,7 @@
const u1* data = ptr;
CHECK_PTR_RANGE(data, data + 1);
-
+
switch (*(data++)) {
case kDexVisibilityBuild:
case kDexVisibilityRuntime:
@@ -2333,11 +2370,11 @@
u4 i;
state->previousItem = NULL;
-
+
for (i = 0; i < count; i++) {
u4 newOffset = (offset + alignmentMask) & ~alignmentMask;
u1* ptr = filePointer(state, newOffset);
-
+
if (offset < newOffset) {
ptr = filePointer(state, offset);
if (offset < newOffset) {
@@ -2540,7 +2577,7 @@
break;
}
case kDexTypeMapList: {
- /*
+ /*
* The map section was swapped early on, but do some
* additional sanity checking here.
*/
@@ -2674,8 +2711,16 @@
break;
}
case kDexTypeClassDefItem: {
+ // Allocate (on the stack) the "observed class_def" bits.
+ size_t arraySize = calcDefinedClassBitsSize(state);
+ u4 definedClassBits[arraySize];
+ memset(definedClassBits, 0, arraySize * sizeof(u4));
+ state->pDefinedClassBits = definedClassBits;
+
okay = iterateSection(state, sectionOffset, sectionCount,
crossVerifyClassDefItem, sizeof(u4), NULL);
+
+ state->pDefinedClassBits = NULL;
break;
}
case kDexTypeAnnotationSetRefList: {
@@ -2799,6 +2844,7 @@
state.fileLen = len;
state.pDexFile = NULL;
state.pDataMap = NULL;
+ state.pDefinedClassBits = NULL;
state.previousItem = NULL;
/*
@@ -2850,6 +2896,6 @@
if (state.pDataMap != NULL) {
dexDataMapFree(state.pDataMap);
}
-
+
return !okay; // 0 == success
}