From 4f2620415d505a35d2d14b866cde10a54b1b7c8c Mon Sep 17 00:00:00 2001 From: Ben Gruver Date: Thu, 5 Dec 2013 14:23:35 -0800 Subject: [PATCH] Add better handling for various types of invalid instructions --- .../jf/baksmali/Adaptors/ClassDefinition.java | 12 +- .../Format/InstructionMethodItem.java | 188 ++++++++++++------ .../baksmali/Adaptors/MethodDefinition.java | 62 +++++- .../baksmali/Adaptors/ReferenceFormatter.java | 3 + .../java/org/jf/baksmali/baksmaliOptions.java | 1 + .../src/main/java/org/jf/baksmali/main.java | 1 + .../org/jf/dexlib2/VerificationError.java | 2 + .../builder/MutableMethodImplementation.java | 8 + .../jf/dexlib2/util/InstructionOffsetMap.java | 30 ++- 9 files changed, 235 insertions(+), 72 deletions(-) diff --git a/baksmali/src/main/java/org/jf/baksmali/Adaptors/ClassDefinition.java b/baksmali/src/main/java/org/jf/baksmali/Adaptors/ClassDefinition.java index 60ef448c..a4eb6913 100644 --- a/baksmali/src/main/java/org/jf/baksmali/Adaptors/ClassDefinition.java +++ b/baksmali/src/main/java/org/jf/baksmali/Adaptors/ClassDefinition.java @@ -32,6 +32,7 @@ import com.google.common.collect.Lists; import org.jf.baksmali.baksmaliOptions; import org.jf.dexlib2.AccessFlags; import org.jf.dexlib2.dexbacked.DexBackedClassDef; +import org.jf.dexlib2.dexbacked.DexBackedDexFile.InvalidItemIndex; import org.jf.dexlib2.iface.*; import org.jf.dexlib2.iface.instruction.Instruction; import org.jf.dexlib2.iface.instruction.formats.Instruction21c; @@ -79,8 +80,15 @@ public class ClassDefinition { case SPUT_SHORT: case SPUT_WIDE: { Instruction21c ins = (Instruction21c)instruction; - FieldReference fieldRef = (FieldReference)ins.getReference(); - if (fieldRef.getDefiningClass().equals((classDef.getType()))) { + FieldReference fieldRef = null; + try { + fieldRef = (FieldReference)ins.getReference(); + } catch (InvalidItemIndex ex) { + // just ignore it for now. We'll deal with it later, when processing the instructions + // themselves + } + if (fieldRef != null && + fieldRef.getDefiningClass().equals((classDef.getType()))) { fieldsSetInStaticConstructor.add(ReferenceUtil.getShortFieldDescriptor(fieldRef)); } break; diff --git a/baksmali/src/main/java/org/jf/baksmali/Adaptors/Format/InstructionMethodItem.java b/baksmali/src/main/java/org/jf/baksmali/Adaptors/Format/InstructionMethodItem.java index e0a969cc..300efd81 100644 --- a/baksmali/src/main/java/org/jf/baksmali/Adaptors/Format/InstructionMethodItem.java +++ b/baksmali/src/main/java/org/jf/baksmali/Adaptors/Format/InstructionMethodItem.java @@ -29,16 +29,21 @@ package org.jf.baksmali.Adaptors.Format; import org.jf.baksmali.Adaptors.MethodDefinition; +import org.jf.baksmali.Adaptors.MethodDefinition.InvalidSwitchPayload; import org.jf.baksmali.Adaptors.MethodItem; -import org.jf.baksmali.Adaptors.ReferenceFormatter; import org.jf.baksmali.Renderers.LongRenderer; +import org.jf.baksmali.baksmaliOptions; +import org.jf.dexlib2.Opcode; import org.jf.dexlib2.ReferenceType; import org.jf.dexlib2.VerificationError; import org.jf.dexlib2.dexbacked.DexBackedDexFile.InvalidItemIndex; import org.jf.dexlib2.iface.instruction.*; import org.jf.dexlib2.iface.instruction.formats.Instruction20bc; +import org.jf.dexlib2.iface.instruction.formats.Instruction31t; import org.jf.dexlib2.iface.instruction.formats.UnknownInstruction; import org.jf.dexlib2.iface.reference.Reference; +import org.jf.dexlib2.util.ReferenceUtil; +import org.jf.util.ExceptionWithContext; import org.jf.util.IndentingWriter; import javax.annotation.Nonnull; @@ -60,29 +65,105 @@ public class InstructionMethodItem extends MethodItem { return 100; } + private boolean isAllowedOdex(@Nonnull Opcode opcode) { + baksmaliOptions options = methodDef.classDef.options; + if (options.allowOdex) { + return true; + } + + if (methodDef.classDef.options.apiLevel >= 14) { + return false; + } + + return opcode.isOdexedInstanceVolatile() || opcode.isOdexedStaticVolatile() || + opcode == Opcode.THROW_VERIFICATION_ERROR; + } + @Override public boolean writeTo(IndentingWriter writer) throws IOException { - boolean invalidReference = false; - if (instruction instanceof ReferenceInstruction) { - try { - Reference reference = ((ReferenceInstruction)instruction).getReference(); - } catch (InvalidItemIndex ex) { - invalidReference = true; + Opcode opcode = instruction.getOpcode(); + String verificationErrorName = null; + String referenceString = null; - writer.write("#invalid "); - writer.write(ReferenceType.toString(instruction.getOpcode().referenceType)); - writer.write(" index: "); - writer.printSignedIntAsDec(ex.getInvalidIndex()); - writer.write("\n#"); + boolean commentOutInstruction = false; + + if (instruction instanceof Instruction20bc) { + int verificationError = ((Instruction20bc)instruction).getVerificationError(); + verificationErrorName = VerificationError.getVerificationErrorName(verificationError); + if (verificationErrorName == null) { + writer.write("#was invalid verification error type: "); + writer.printSignedIntAsDec(verificationError); + writer.write("\n"); + verificationErrorName = "generic-error"; } } + if (instruction instanceof ReferenceInstruction) { + ReferenceInstruction referenceInstruction = (ReferenceInstruction)instruction; + try { + Reference reference = referenceInstruction.getReference(); + referenceString = ReferenceUtil.getReferenceString(reference); + assert referenceString != null; + } catch (InvalidItemIndex ex) { + writer.write("#"); + writer.write(ex.getMessage()); + writer.write("\n"); + commentOutInstruction = true; + + referenceString = String.format("%s@%d", + ReferenceType.toString(referenceInstruction.getReferenceType()), + ex.getInvalidIndex()); + } catch (ReferenceType.InvalidReferenceTypeException ex) { + writer.write("#invalid reference type: "); + writer.printSignedIntAsDec(ex.getReferenceType()); + commentOutInstruction = true; + + referenceString = "invalid_reference"; + } + } + + if (instruction instanceof Instruction31t) { + Opcode payloadOpcode; + switch (instruction.getOpcode()) { + case PACKED_SWITCH: + payloadOpcode = Opcode.PACKED_SWITCH_PAYLOAD; + break; + case SPARSE_SWITCH: + payloadOpcode = Opcode.SPARSE_SWITCH_PAYLOAD; + break; + case FILL_ARRAY_DATA: + payloadOpcode = Opcode.ARRAY_PAYLOAD; + break; + default: + throw new ExceptionWithContext("Invalid 31t opcode: %s", instruction.getOpcode()); + } + + try { + methodDef.findSwitchPayload(this.codeAddress + ((Instruction31t)instruction).getCodeOffset(), + payloadOpcode); + } catch (InvalidSwitchPayload ex) { + writer.write("#invalid payload reference"); + commentOutInstruction = true; + } + } + + if (opcode.odexOnly()) { + if (!isAllowedOdex(opcode)) { + writer.write("#disallowed odex opcode\n"); + commentOutInstruction = true; + } + } + + if (commentOutInstruction) { + writer.write("#"); + } + switch (instruction.getOpcode().format) { case Format10t: writeOpcode(writer); writer.write(' '); writeTargetLabel(writer); - return true; + break; case Format10x: if (instruction instanceof UnknownInstruction) { writer.write("#unknown opcode: 0x"); @@ -90,47 +171,47 @@ public class InstructionMethodItem extends MethodItem { writer.write('\n'); } writeOpcode(writer); - return true; + break; case Format11n: writeOpcode(writer); writer.write(' '); writeFirstRegister(writer); writer.write(", "); writeLiteral(writer); - return true; + break; case Format11x: writeOpcode(writer); writer.write(' '); writeFirstRegister(writer); - return true; + break; case Format12x: writeOpcode(writer); writer.write(' '); writeFirstRegister(writer); writer.write(", "); writeSecondRegister(writer); - return true; + break; case Format20bc: writeOpcode(writer); writer.write(' '); - writeVerificationErrorType(writer); + writer.write(verificationErrorName); writer.write(", "); - writeReference(writer); - return true; + writer.write(referenceString); + break; case Format20t: case Format30t: writeOpcode(writer); writer.write(' '); writeTargetLabel(writer); - return true; + break; case Format21c: case Format31c: writeOpcode(writer); writer.write(' '); writeFirstRegister(writer); writer.write(", "); - writeReference(writer); - return true; + writer.write(referenceString); + break; case Format21ih: case Format21lh: case Format21s: @@ -143,7 +224,7 @@ public class InstructionMethodItem extends MethodItem { writeLiteral(writer); if (instruction.getOpcode().setsWideRegister() == false) writeResourceId(writer); - return true; + break; case Format21t: case Format31t: writeOpcode(writer); @@ -151,7 +232,7 @@ public class InstructionMethodItem extends MethodItem { writeFirstRegister(writer); writer.write(", "); writeTargetLabel(writer); - return true; + break; case Format22b: case Format22s: writeOpcode(writer); @@ -161,7 +242,7 @@ public class InstructionMethodItem extends MethodItem { writeSecondRegister(writer); writer.write(", "); writeLiteral(writer); - return true; + break; case Format22c: writeOpcode(writer); writer.write(' '); @@ -169,8 +250,8 @@ public class InstructionMethodItem extends MethodItem { writer.write(", "); writeSecondRegister(writer); writer.write(", "); - writeReference(writer); - return true; + writer.write(referenceString); + break; case Format22cs: writeOpcode(writer); writer.write(' '); @@ -179,7 +260,7 @@ public class InstructionMethodItem extends MethodItem { writeSecondRegister(writer); writer.write(", "); writeFieldOffset(writer); - return true; + break; case Format22t: writeOpcode(writer); writer.write(' '); @@ -188,7 +269,7 @@ public class InstructionMethodItem extends MethodItem { writeSecondRegister(writer); writer.write(", "); writeTargetLabel(writer); - return true; + break; case Format22x: case Format32x: writeOpcode(writer); @@ -196,7 +277,7 @@ public class InstructionMethodItem extends MethodItem { writeFirstRegister(writer); writer.write(", "); writeSecondRegister(writer); - return true; + break; case Format23x: writeOpcode(writer); writer.write(' '); @@ -205,52 +286,59 @@ public class InstructionMethodItem extends MethodItem { writeSecondRegister(writer); writer.write(", "); writeThirdRegister(writer); - return true; + break; case Format35c: writeOpcode(writer); writer.write(' '); writeInvokeRegisters(writer); writer.write(", "); - writeReference(writer); - return true; + writer.write(referenceString); + break; case Format35mi: writeOpcode(writer); writer.write(' '); writeInvokeRegisters(writer); writer.write(", "); writeInlineIndex(writer); - return true; + break; case Format35ms: writeOpcode(writer); writer.write(' '); writeInvokeRegisters(writer); writer.write(", "); writeVtableIndex(writer); - return true; + break; case Format3rc: writeOpcode(writer); writer.write(' '); writeInvokeRangeRegisters(writer); writer.write(", "); - writeReference(writer); - return true; + writer.write(referenceString); + break; case Format3rmi: writeOpcode(writer); writer.write(' '); writeInvokeRangeRegisters(writer); writer.write(", "); writeInlineIndex(writer); - return true; + break; case Format3rms: writeOpcode(writer); writer.write(' '); writeInvokeRangeRegisters(writer); writer.write(", "); writeVtableIndex(writer); - return true; + break; + default: + assert false; + return false; } - assert false; - return false; + + if (commentOutInstruction) { + writer.write("\nnop"); + } + + return true; } protected void writeOpcode(IndentingWriter writer) throws IOException { @@ -367,20 +455,4 @@ public class InstructionMethodItem extends MethodItem { writer.write("vtable@"); writer.printSignedIntAsDec(((VtableIndexInstruction)instruction).getVtableIndex()); } - - protected void writeReference(IndentingWriter writer) throws IOException { - try { - ReferenceFormatter.writeReference(writer, instruction.getOpcode().referenceType, - ((ReferenceInstruction)instruction).getReference()); - } catch (InvalidItemIndex ex) { - writer.write(ReferenceType.toString(instruction.getOpcode().referenceType)); - writer.write("@"); - writer.printSignedIntAsDec(ex.getInvalidIndex()); - } - } - - protected void writeVerificationErrorType(IndentingWriter writer) throws IOException { - int verificationError = ((Instruction20bc)instruction).getVerificationError(); - writer.write(VerificationError.getVerificationErrorName(verificationError)); - } } diff --git a/baksmali/src/main/java/org/jf/baksmali/Adaptors/MethodDefinition.java b/baksmali/src/main/java/org/jf/baksmali/Adaptors/MethodDefinition.java index 8ebd1e78..5ce971f5 100644 --- a/baksmali/src/main/java/org/jf/baksmali/Adaptors/MethodDefinition.java +++ b/baksmali/src/main/java/org/jf/baksmali/Adaptors/MethodDefinition.java @@ -39,6 +39,7 @@ import org.jf.dexlib2.ReferenceType; import org.jf.dexlib2.analysis.AnalysisException; import org.jf.dexlib2.analysis.AnalyzedInstruction; import org.jf.dexlib2.analysis.MethodAnalyzer; +import org.jf.dexlib2.dexbacked.DexBackedDexFile.InvalidItemIndex; import org.jf.dexlib2.iface.*; import org.jf.dexlib2.iface.debug.DebugItem; import org.jf.dexlib2.iface.instruction.Instruction; @@ -46,6 +47,7 @@ import org.jf.dexlib2.iface.instruction.OffsetInstruction; import org.jf.dexlib2.iface.instruction.ReferenceInstruction; import org.jf.dexlib2.iface.reference.MethodReference; import org.jf.dexlib2.util.InstructionOffsetMap; +import org.jf.dexlib2.util.InstructionOffsetMap.InvalidInstructionOffset; import org.jf.dexlib2.util.ReferenceUtil; import org.jf.dexlib2.util.SyntheticAccessorResolver; import org.jf.dexlib2.util.TypeUtils; @@ -92,15 +94,31 @@ public class MethodDefinition { Opcode opcode = instruction.getOpcode(); if (opcode == Opcode.PACKED_SWITCH) { + boolean valid = true; int codeOffset = instructionOffsetMap.getInstructionCodeOffset(i); int targetOffset = codeOffset + ((OffsetInstruction)instruction).getCodeOffset(); - targetOffset = findSwitchPayload(targetOffset, Opcode.PACKED_SWITCH_PAYLOAD); - packedSwitchMap.append(targetOffset, codeOffset); + try { + targetOffset = findSwitchPayload(targetOffset, Opcode.PACKED_SWITCH_PAYLOAD); + } catch (InvalidSwitchPayload ex) { + valid = false; + } + if (valid) { + packedSwitchMap.append(targetOffset, codeOffset); + } } else if (opcode == Opcode.SPARSE_SWITCH) { + boolean valid = true; int codeOffset = instructionOffsetMap.getInstructionCodeOffset(i); int targetOffset = codeOffset + ((OffsetInstruction)instruction).getCodeOffset(); - targetOffset = findSwitchPayload(targetOffset, Opcode.SPARSE_SWITCH_PAYLOAD); - sparseSwitchMap.append(targetOffset, codeOffset); + try { + targetOffset = findSwitchPayload(targetOffset, Opcode.SPARSE_SWITCH_PAYLOAD); + } catch (InvalidSwitchPayload ex) { + valid = false; + // The offset to the payload instruction was invalid. Nothing to do, except that we won't + // add this instruction to the map. + } + if (valid) { + sparseSwitchMap.append(targetOffset, codeOffset); + } } } }catch (Exception ex) { @@ -187,8 +205,13 @@ public class MethodDefinition { writer.write(".end method\n"); } - private int findSwitchPayload(int targetOffset, Opcode type) { - int targetIndex = instructionOffsetMap.getInstructionIndexAtCodeOffset(targetOffset); + public int findSwitchPayload(int targetOffset, Opcode type) { + int targetIndex; + try { + targetIndex = instructionOffsetMap.getInstructionIndexAtCodeOffset(targetOffset); + } catch (InvalidInstructionOffset ex) { + throw new InvalidSwitchPayload(targetOffset); + } //TODO: does dalvik let you pad with multiple nops? //TODO: does dalvik let a switch instruction point to a non-payload instruction? @@ -205,7 +228,7 @@ public class MethodDefinition { } } } - throw new ExceptionWithContext("No switch payload at offset 0x%x", targetOffset); + throw new InvalidSwitchPayload(targetOffset); } else { return targetOffset; } @@ -337,10 +360,16 @@ public class MethodDefinition { Opcode opcode = instruction.getOpcode(); if (opcode.referenceType == ReferenceType.METHOD) { - MethodReference methodReference = - (MethodReference)((ReferenceInstruction)instruction).getReference(); + MethodReference methodReference = null; + try { + methodReference = (MethodReference)((ReferenceInstruction)instruction).getReference(); + } catch (InvalidItemIndex ex) { + // just ignore it for now. We'll deal with it later, when processing the instructions + // themselves + } - if (SyntheticAccessorResolver.looksLikeSyntheticAccessor(methodReference.getName())) { + if (methodReference != null && + SyntheticAccessorResolver.looksLikeSyntheticAccessor(methodReference.getName())) { SyntheticAccessorResolver.AccessedMember accessedMember = classDef.options.syntheticAccessorResolver.getAccessedMember(methodReference); if (accessedMember != null) { @@ -510,4 +539,17 @@ public class MethodDefinition { return labels.values(); } } + + public static class InvalidSwitchPayload extends ExceptionWithContext { + private final int payloadOffset; + + public InvalidSwitchPayload(int payloadOffset) { + super("No switch payload at offset: %d", payloadOffset); + this.payloadOffset = payloadOffset; + } + + public int getPayloadOffset() { + return payloadOffset; + } + } } diff --git a/baksmali/src/main/java/org/jf/baksmali/Adaptors/ReferenceFormatter.java b/baksmali/src/main/java/org/jf/baksmali/Adaptors/ReferenceFormatter.java index a39b66d6..91d142a8 100644 --- a/baksmali/src/main/java/org/jf/baksmali/Adaptors/ReferenceFormatter.java +++ b/baksmali/src/main/java/org/jf/baksmali/Adaptors/ReferenceFormatter.java @@ -57,6 +57,9 @@ public class ReferenceFormatter { return; case ReferenceType.FIELD: ReferenceUtil.writeFieldDescriptor(writer, (FieldReference)reference); + return; + default: + throw new IllegalStateException("Unknown reference type"); } } } diff --git a/baksmali/src/main/java/org/jf/baksmali/baksmaliOptions.java b/baksmali/src/main/java/org/jf/baksmali/baksmaliOptions.java index 197841c6..48a9146b 100644 --- a/baksmali/src/main/java/org/jf/baksmali/baksmaliOptions.java +++ b/baksmali/src/main/java/org/jf/baksmali/baksmaliOptions.java @@ -67,6 +67,7 @@ public class baksmaliOptions { public boolean outputDebugInfo = true; public boolean addCodeOffsets = false; public boolean noAccessorComments = false; + public boolean allowOdex = false; public boolean deodex = false; public boolean ignoreErrors = false; public boolean checkPackagePrivateAccess = false; diff --git a/baksmali/src/main/java/org/jf/baksmali/main.java b/baksmali/src/main/java/org/jf/baksmali/main.java index 2442ff24..26aaf78a 100644 --- a/baksmali/src/main/java/org/jf/baksmali/main.java +++ b/baksmali/src/main/java/org/jf/baksmali/main.java @@ -255,6 +255,7 @@ public class main { System.err.println("Warning: You are disassembling an odex file without deodexing it. You"); System.err.println("won't be able to re-assemble the results unless you deodex it with the -x"); System.err.println("option"); + options.allowOdex = true; } } else { options.deodex = false; diff --git a/dexlib2/src/main/java/org/jf/dexlib2/VerificationError.java b/dexlib2/src/main/java/org/jf/dexlib2/VerificationError.java index 4e9ccd05..d0aa0297 100644 --- a/dexlib2/src/main/java/org/jf/dexlib2/VerificationError.java +++ b/dexlib2/src/main/java/org/jf/dexlib2/VerificationError.java @@ -34,6 +34,7 @@ package org.jf.dexlib2; import com.google.common.collect.Maps; import org.jf.util.ExceptionWithContext; +import javax.annotation.Nullable; import java.util.HashMap; public class VerificationError { @@ -61,6 +62,7 @@ public class VerificationError { verificationErrorNames.put("instantiation-error", INSTANTIATION_ERROR); } + @Nullable public static String getVerificationErrorName(int verificationError) { switch (verificationError) { case GENERIC: diff --git a/dexlib2/src/main/java/org/jf/dexlib2/builder/MutableMethodImplementation.java b/dexlib2/src/main/java/org/jf/dexlib2/builder/MutableMethodImplementation.java index 8cdf1f99..a3bfbd89 100644 --- a/dexlib2/src/main/java/org/jf/dexlib2/builder/MutableMethodImplementation.java +++ b/dexlib2/src/main/java/org/jf/dexlib2/builder/MutableMethodImplementation.java @@ -435,6 +435,14 @@ public class MutableMethodImplementation implements MethodImplementation { } case SPARSE_SWITCH_PAYLOAD: case PACKED_SWITCH_PAYLOAD: + if (((BuilderSwitchPayload)instruction).referrer == null) { + // if the switch payload isn't referenced, just remove it + removeInstruction(index); + index--; + madeChanges = true; + break; + } + // intentional fall-through case ARRAY_PAYLOAD: { if ((location.codeAddress & 0x01) != 0) { int previousIndex = location.index - 1; diff --git a/dexlib2/src/main/java/org/jf/dexlib2/util/InstructionOffsetMap.java b/dexlib2/src/main/java/org/jf/dexlib2/util/InstructionOffsetMap.java index 3675a5d4..a817523d 100644 --- a/dexlib2/src/main/java/org/jf/dexlib2/util/InstructionOffsetMap.java +++ b/dexlib2/src/main/java/org/jf/dexlib2/util/InstructionOffsetMap.java @@ -59,7 +59,7 @@ public class InstructionOffsetMap { int index = Arrays.binarySearch(instructionCodeOffsets, codeOffset); if (index < 0) { if (exact) { - throw new ExceptionWithContext("No instruction at offset %d", codeOffset); + throw new InvalidInstructionOffset(codeOffset); } else { // This calculation would be incorrect if index was -1 (i.e. insertion point of 0). Luckily, we can // ignore this case, because codeOffset will always be non-negative, and the code offset of the first @@ -72,8 +72,34 @@ public class InstructionOffsetMap { public int getInstructionCodeOffset(int index) { if (index < 0 || index >= instructionCodeOffsets.length) { - throw new ExceptionWithContext("Index out of bounds: %d", index); + throw new InvalidInstructionIndex(index); } return instructionCodeOffsets[index]; } + + public static class InvalidInstructionOffset extends ExceptionWithContext { + private final int instructionOffset; + + public InvalidInstructionOffset(int instructionOffset) { + super("No instruction at offset %d", instructionOffset); + this.instructionOffset = instructionOffset; + } + + public int getInstructionOffset() { + return instructionOffset; + } + } + + public static class InvalidInstructionIndex extends ExceptionWithContext { + private final int instructionIndex; + + public InvalidInstructionIndex(int instructionIndex) { + super("Instruction index out of bounds: %d", instructionIndex); + this.instructionIndex = instructionIndex; + } + + public int getInstructionIndex() { + return instructionIndex; + } + } }