Null pointer checks for NoisyStructureBuilder and FillOutStructureCmd

This commit is contained in:
caheckman 2021-06-01 14:39:21 -04:00
parent 0a8db1e0ad
commit 703b368b78
4 changed files with 44 additions and 29 deletions

View file

@ -46,7 +46,6 @@ import ghidra.util.task.TaskMonitor;
*/ */
public class FillOutStructureCmd extends BackgroundCommand { public class FillOutStructureCmd extends BackgroundCommand {
/** /**
* Varnode with data-flow traceable to original pointer * Varnode with data-flow traceable to original pointer
*/ */
@ -75,8 +74,8 @@ public class FillOutStructureCmd extends BackgroundCommand {
private TaskMonitor monitor; private TaskMonitor monitor;
private PluginTool tool; private PluginTool tool;
private List<OffsetPcodeOpPair> storePcodeOps = new ArrayList<OffsetPcodeOpPair>(); private List<OffsetPcodeOpPair> storePcodeOps = new ArrayList<>();
private List<OffsetPcodeOpPair> loadPcodeOps = new ArrayList<OffsetPcodeOpPair>(); private List<OffsetPcodeOpPair> loadPcodeOps = new ArrayList<>();
/** /**
* Constructor. * Constructor.
@ -155,8 +154,8 @@ public class FillOutStructureCmd extends BackgroundCommand {
DataType pointerDT = new PointerDataType(structDT); DataType pointerDT = new PointerDataType(structDT);
// Delay adding to the manager until full structure is accumulated // Delay adding to the manager until full structure is accumulated
pointerDT = currentProgram.getDataTypeManager().addDataType(pointerDT, pointerDT = currentProgram.getDataTypeManager()
DataTypeConflictHandler.DEFAULT_HANDLER); .addDataType(pointerDT, DataTypeConflictHandler.DEFAULT_HANDLER);
commitVariable(var, pointerDT, isThisParam); commitVariable(var, pointerDT, isThisParam);
} }
catch (Exception e) { catch (Exception e) {
@ -225,7 +224,6 @@ public class FillOutStructureCmd extends BackgroundCommand {
return loadPcodeOps; return loadPcodeOps;
} }
/** /**
* Retrieve the (likely) storage address of a function parameter given * Retrieve the (likely) storage address of a function parameter given
* the inputs to a CALL p-code op and particular Varnode slot within the inputs. * the inputs to a CALL p-code op and particular Varnode slot within the inputs.
@ -379,15 +377,15 @@ public class FillOutStructureCmd extends BackgroundCommand {
sym = highFunc.getMappedSymbol(storageAddress, function.getEntryPoint()); sym = highFunc.getMappedSymbol(storageAddress, function.getEntryPoint());
} }
if (sym == null) { if (sym == null) {
sym = highFunc.getLocalSymbolMap().findLocal(storageAddress, sym = highFunc.getLocalSymbolMap()
function.getEntryPoint().subtractWrap(1L)); .findLocal(storageAddress, function.getEntryPoint().subtractWrap(1L));
} }
if (sym == null) { if (sym == null) {
sym = highFunc.getLocalSymbolMap().findLocal(storageAddress, null); sym = highFunc.getLocalSymbolMap().findLocal(storageAddress, null);
} }
if (sym == null) { if (sym == null) {
sym = highFunc.getLocalSymbolMap().findLocal(storageAddress, sym = highFunc.getLocalSymbolMap()
function.getEntryPoint()); .findLocal(storageAddress, function.getEntryPoint());
} }
if (sym == null) { if (sym == null) {
return null; return null;
@ -525,9 +523,8 @@ public class FillOutStructureCmd extends BackgroundCommand {
} }
String structName = createUniqueStructName(var, DEFAULT_CATEGORY, DEFAULT_BASENAME); String structName = createUniqueStructName(var, DEFAULT_CATEGORY, DEFAULT_BASENAME);
StructureDataType dt = StructureDataType dt = new StructureDataType(new CategoryPath(DEFAULT_CATEGORY), structName,
new StructureDataType(new CategoryPath(DEFAULT_CATEGORY), structName, size, size, f.getProgram().getDataTypeManager());
f.getProgram().getDataTypeManager());
return dt; return dt;
} }
@ -613,7 +610,7 @@ public class FillOutStructureCmd extends BackgroundCommand {
*/ */
private void fillOutStructureDef(HighVariable var) { private void fillOutStructureDef(HighVariable var) {
Varnode startVN = var.getRepresentative(); Varnode startVN = var.getRepresentative();
ArrayList<PointerRef> todoList = new ArrayList<PointerRef>(); ArrayList<PointerRef> todoList = new ArrayList<>();
HashSet<Varnode> doneList = new HashSet<>(); HashSet<Varnode> doneList = new HashSet<>();
todoList.add(new PointerRef(startVN, 0)); // Base Varnode on the todo list todoList.add(new PointerRef(startVN, 0)); // Base Varnode on the todo list
@ -661,8 +658,7 @@ public class FillOutStructureCmd extends BackgroundCommand {
if (!inputs[1].isConstant() || !inputs[2].isConstant()) { if (!inputs[1].isConstant() || !inputs[2].isConstant()) {
break; break;
} }
newOff = newOff = currentRef.offset + getSigned(inputs[1]) * inputs[2].getOffset();
currentRef.offset + getSigned(inputs[1]) * inputs[2].getOffset();
if (sanityCheck(newOff)) { // should this offset create a location in the structure? if (sanityCheck(newOff)) { // should this offset create a location in the structure?
putOnList(output, newOff, todoList, doneList); putOnList(output, newOff, todoList, doneList);
// Don't do componentMap.addReference() as data-type info here is likely uninformed // Don't do componentMap.addReference() as data-type info here is likely uninformed

View file

@ -72,15 +72,15 @@ public enum MetaDataType {
} }
public static DataType getMostSpecificDataType(DataType a, DataType b) { public static DataType getMostSpecificDataType(DataType a, DataType b) {
if (a == null) {
return b;
}
if (b == null) {
return a;
}
DataType aCopy = a; DataType aCopy = a;
DataType bCopy = b; DataType bCopy = b;
for (;;) { for (;;) {
if (a == null) {
return bCopy;
}
if (b == null) {
return aCopy;
}
MetaDataType aMeta = MetaDataType.getMeta(a); MetaDataType aMeta = MetaDataType.getMeta(a);
MetaDataType bMeta = MetaDataType.getMeta(b); MetaDataType bMeta = MetaDataType.getMeta(b);
int compare = aMeta.compareTo(bMeta); int compare = aMeta.compareTo(bMeta);

View file

@ -31,7 +31,7 @@ import java.util.TreeMap;
* the final field entries. * the final field entries.
*/ */
public class NoisyStructureBuilder { public class NoisyStructureBuilder {
private TreeMap<Long, DataType> offsetToDataTypeMap = new TreeMap<Long, DataType>(); private TreeMap<Long, DataType> offsetToDataTypeMap = new TreeMap<>();
private Structure structDT = null; private Structure structDT = null;
private long sizeOfStruct = 0; private long sizeOfStruct = 0;
@ -83,11 +83,14 @@ public class NoisyStructureBuilder {
computeMax(offset, 1); computeMax(offset, 1);
return; return;
} }
if (dt instanceof Pointer && ((Pointer) dt).getDataType().equals(structDT)) { if (dt instanceof Pointer) {
// Be careful of taking a pointer to the structure when the structure DataType baseType = ((Pointer) dt).getDataType();
// is not fully defined if (baseType != null && baseType.equals(structDT)) {
DataTypeManager manager = dt.getDataTypeManager(); // Be careful of taking a pointer to the structure when the structure
dt = manager.getPointer(DataType.DEFAULT, dt.getLength()); // is not fully defined
DataTypeManager manager = dt.getDataTypeManager();
dt = manager.getPointer(DataType.DEFAULT, dt.getLength());
}
} }
computeMax(offset, dt.getLength()); computeMax(offset, dt.getLength());
Entry<Long, DataType> firstEntry = checkForOverlap(offset, dt.getLength()); Entry<Long, DataType> firstEntry = checkForOverlap(offset, dt.getLength());
@ -127,7 +130,7 @@ public class NoisyStructureBuilder {
public void addReference(long offset, DataType dt) { public void addReference(long offset, DataType dt) {
if (dt != null && dt instanceof Pointer) { if (dt != null && dt instanceof Pointer) {
dt = ((Pointer) dt).getDataType(); dt = ((Pointer) dt).getDataType();
if (dt.equals(structDT)) { if (dt != null && dt.equals(structDT)) {
return; // Don't allow structure to contain itself return; // Don't allow structure to contain itself
} }
if (dt instanceof Structure) { if (dt instanceof Structure) {

View file

@ -98,4 +98,20 @@ public class NoisyStructureBuilderTest extends AbstractGTest {
testNextField(iter, 8, DWordDataType.dataType); testNextField(iter, 8, DWordDataType.dataType);
Assert.assertFalse(iter.hasNext()); Assert.assertFalse(iter.hasNext());
} }
@Test
public void testPointerNulls() {
NoisyStructureBuilder builder = new NoisyStructureBuilder();
DataType pointerNull = new Pointer32DataType(null);
builder.addDataType(4, Undefined4DataType.dataType);
builder.addDataType(8, Undefined4DataType.dataType);
builder.addDataType(4, pointerNull);
builder.addReference(16, pointerNull);
Iterator<Entry<Long, DataType>> iter = builder.iterator();
testNextField(iter, 4, pointerNull);
testNextField(iter, 8, Undefined4DataType.dataType);
Assert.assertFalse(iter.hasNext());
Assert.assertEquals(builder.getSize(), 17);
}
} }