From 48aaa697e7bc1a6a44cbd5f75c6a4c25a7cd0dd3 Mon Sep 17 00:00:00 2001 From: emteere <47253321+emteere@users.noreply.github.com> Date: Wed, 6 Nov 2019 09:38:02 -0500 Subject: [PATCH] GT-3261_emteere code review updates --- .../util/bin/format/macho/commands/NList.java | 103 ++++++------ .../util/opinion/DyldCacheProgramBuilder.java | 146 +++++++++--------- 2 files changed, 125 insertions(+), 124 deletions(-) diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/macho/commands/NList.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/macho/commands/NList.java index 07399000a2..26505e5923 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/macho/commands/NList.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/macho/commands/NList.java @@ -20,9 +20,7 @@ import java.io.IOException; import ghidra.app.util.bin.StructConverter; import ghidra.app.util.bin.format.FactoryBundledWithBinaryReader; import ghidra.app.util.bin.format.macho.MachConstants; -import ghidra.program.model.data.CategoryPath; -import ghidra.program.model.data.DataType; -import ghidra.program.model.data.StructureDataType; +import ghidra.program.model.data.*; import ghidra.util.exception.AssertException; import ghidra.util.exception.DuplicateNameException; @@ -32,34 +30,36 @@ import ghidra.util.exception.DuplicateNameException; * @see mach-o/nlist.h */ public class NList implements StructConverter { - private int n_strx; - private byte n_type; - private byte n_sect; - private short n_desc; - private long n_value; + private int n_strx; + private byte n_type; + private byte n_sect; + private short n_desc; + private long n_value; private String string; private boolean is32bit; - public static NList createNList(FactoryBundledWithBinaryReader reader, - boolean is32bit) throws IOException { - NList nList = (NList) reader.getFactory().create(NList.class); - nList.initNList(reader, is32bit); - return nList; - } + public static NList createNList(FactoryBundledWithBinaryReader reader, boolean is32bit) + throws IOException { + NList nList = (NList) reader.getFactory().create(NList.class); + nList.initNList(reader, is32bit); + return nList; + } - /** - * DO NOT USE THIS CONSTRUCTOR, USE create*(GenericFactory ...) FACTORY METHODS INSTEAD. - */ - public NList() {} + /** + * DO NOT USE THIS CONSTRUCTOR, USE create*(GenericFactory ...) FACTORY METHODS INSTEAD. + */ + public NList() { + } - private void initNList(FactoryBundledWithBinaryReader reader, boolean is32bit) throws IOException { + private void initNList(FactoryBundledWithBinaryReader reader, boolean is32bit) + throws IOException { this.is32bit = is32bit; - n_strx = reader.readNextInt(); - n_type = reader.readNextByte(); - n_sect = reader.readNextByte(); - n_desc = reader.readNextShort(); + n_strx = reader.readNextInt(); + n_type = reader.readNextByte(); + n_sect = reader.readNextByte(); + n_desc = reader.readNextShort(); if (is32bit) { n_value = reader.readNextInt() & 0xffffffffL; } @@ -67,12 +67,12 @@ public class NList implements StructConverter { n_value = reader.readNextLong(); } } - - /**] + + /** * Initialize the string from the string table. - * + *

* You MUST call this method after the NLIST element is created! - * + *

* Reading a large NList table can cause a large performance issue if the strings * are initialized as the NList entry is created. The string table indexes are * scattered. Initializing the strings linearly from the string table is much @@ -92,19 +92,19 @@ public class NList implements StructConverter { @Override public DataType toDataType() throws DuplicateNameException, IOException { - StructureDataType struct = new StructureDataType("nlist", 0); - struct.add(DWORD, "n_strx", null); - struct.add( BYTE, "n_type", null); - struct.add( BYTE, "n_sect", null); - struct.add( WORD, "n_desc", null); - if (is32bit) { - struct.add(DWORD, "n_value", null); - } - else { - struct.add(QWORD, "n_value", null); - } - struct.setCategoryPath(new CategoryPath(MachConstants.DATA_TYPE_CATEGORY)); - return struct; + StructureDataType struct = new StructureDataType("nlist", 0); + struct.add(DWORD, "n_strx", null); + struct.add(BYTE, "n_type", null); + struct.add(BYTE, "n_sect", null); + struct.add(WORD, "n_desc", null); + if (is32bit) { + struct.add(DWORD, "n_value", null); + } + else { + struct.add(QWORD, "n_value", null); + } + struct.setCategoryPath(new CategoryPath(MachConstants.DATA_TYPE_CATEGORY)); + return struct; } /** @@ -126,6 +126,7 @@ public class NList implements StructConverter { public int getStringTableIndex() { return n_strx; } + /** * Returns the symbol type flag. * @return the symbol type flag @@ -135,34 +136,40 @@ public class NList implements StructConverter { } public boolean isTypeUndefined() { - return n_sect == NListConstants.NO_SECT && - (n_type & NListConstants.MASK_N_TYPE) == NListConstants.TYPE_N_UNDF; + return n_sect == NListConstants.NO_SECT && + (n_type & NListConstants.MASK_N_TYPE) == NListConstants.TYPE_N_UNDF; } + public boolean isTypeAbsolute() { - return n_sect == NListConstants.NO_SECT && - (n_type & NListConstants.MASK_N_TYPE) == NListConstants.TYPE_N_ABS; + return n_sect == NListConstants.NO_SECT && + (n_type & NListConstants.MASK_N_TYPE) == NListConstants.TYPE_N_ABS; } + public boolean isTypePreboundUndefined() { - return n_sect == NListConstants.NO_SECT && - (n_type & NListConstants.MASK_N_TYPE) == NListConstants.TYPE_N_PBUD; + return n_sect == NListConstants.NO_SECT && + (n_type & NListConstants.MASK_N_TYPE) == NListConstants.TYPE_N_PBUD; } public boolean isSymbolicDebugging() { return (n_type & NListConstants.MASK_N_STAB) != 0; } + public boolean isPrivateExternal() { return (n_type & NListConstants.MASK_N_PEXT) != 0; } + public boolean isExternal() { return (n_type & NListConstants.MASK_N_EXT) != 0; } + public boolean isLazyBind() { - return ( n_desc & NListConstants.REFERENCE_TYPE ) != 0; + return (n_desc & NListConstants.REFERENCE_TYPE) != 0; } public boolean isThumbSymbol() { return (n_desc & NListConstants.DESC_N_ARM_THUMB_DEF) != 0; } + /** * An integer specifying the number of the section that this * symbol can be found in, or NO_SECT if @@ -172,6 +179,7 @@ public class NList implements StructConverter { public byte getSection() { return n_sect; } + /** * A 16-bit value providing additional information about this symbol. * @return a 16-bit value providing additional information about this symbol @@ -179,6 +187,7 @@ public class NList implements StructConverter { public short getDescription() { return n_desc; } + /** * An integer that contains the value of this symbol. * The format of this value is different for each type of symbol. diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/DyldCacheProgramBuilder.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/DyldCacheProgramBuilder.java index 63a92ccfcc..ccdd163130 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/DyldCacheProgramBuilder.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/DyldCacheProgramBuilder.java @@ -17,10 +17,7 @@ package ghidra.app.util.opinion; import java.io.File; import java.io.IOException; -import java.util.ArrayList; -import java.util.Iterator; -import java.util.List; -import java.util.TreeSet; +import java.util.*; import ghidra.app.util.MemoryBlockUtils; import ghidra.app.util.bin.BinaryReader; @@ -28,21 +25,14 @@ import ghidra.app.util.bin.ByteProvider; import ghidra.app.util.bin.format.macho.MachException; import ghidra.app.util.bin.format.macho.MachHeader; import ghidra.app.util.bin.format.macho.commands.NList; -import ghidra.app.util.bin.format.macho.dyld.DyldCacheHeader; -import ghidra.app.util.bin.format.macho.dyld.DyldCacheImageInfo; -import ghidra.app.util.bin.format.macho.dyld.DyldCacheLocalSymbolsInfo; -import ghidra.app.util.bin.format.macho.dyld.DyldCacheMappingInfo; -import ghidra.app.util.bin.format.macho.dyld.DyldCacheSlideInfo2; -import ghidra.app.util.bin.format.macho.dyld.DyldCacheSlideInfoCommon; +import ghidra.app.util.bin.format.macho.dyld.*; import ghidra.app.util.importer.MessageLog; import ghidra.app.util.importer.MessageLogContinuesFactory; import ghidra.program.database.mem.FileBytes; import ghidra.program.model.address.Address; import ghidra.program.model.address.AddressSpace; import ghidra.program.model.data.Pointer64DataType; -import ghidra.program.model.listing.CodeUnit; -import ghidra.program.model.listing.Program; -import ghidra.program.model.listing.ProgramFragment; +import ghidra.program.model.listing.*; import ghidra.program.model.mem.MemoryAccessException; import ghidra.program.model.symbol.SourceType; import ghidra.program.model.symbol.SymbolUtilities; @@ -60,7 +50,7 @@ public class DyldCacheProgramBuilder extends MachoProgramBuilder { private static final int CHAIN_OFFSET_MASK = 0x3fff; private static final int DYLD_CACHE_SLIDE_PAGE_ATTR_NO_REBASE = 0x4000; private static final int DYLD_CACHE_SLIDE_PAGE_ATTR_EXTRA = 0x8000; - + protected DyldCacheHeader dyldCacheHeader; private boolean shouldProcessSymbols; private boolean shouldCreateDylibSections; @@ -81,8 +71,7 @@ public class DyldCacheProgramBuilder extends MachoProgramBuilder { */ protected DyldCacheProgramBuilder(Program program, ByteProvider provider, FileBytes fileBytes, boolean shouldProcessSymbols, boolean shouldCreateDylibSections, - boolean shouldAddRelocationEntries, MessageLog log, - TaskMonitor monitor) { + boolean shouldAddRelocationEntries, MessageLog log, TaskMonitor monitor) { super(program, provider, fileBytes, log, monitor); this.shouldProcessSymbols = shouldProcessSymbols; this.shouldCreateDylibSections = shouldCreateDylibSections; @@ -104,11 +93,11 @@ public class DyldCacheProgramBuilder extends MachoProgramBuilder { * @throws Exception if a problem occurs */ public static void buildProgram(Program program, ByteProvider provider, FileBytes fileBytes, - boolean shouldProcessSymbols, boolean shouldCreateDylibSections, boolean addRelocationEntries, - MessageLog log, TaskMonitor monitor) throws Exception { - DyldCacheProgramBuilder dyldCacheProgramBuilder = new DyldCacheProgramBuilder(program, - provider, fileBytes, shouldProcessSymbols, shouldCreateDylibSections, - addRelocationEntries, log, monitor); + boolean shouldProcessSymbols, boolean shouldCreateDylibSections, + boolean addRelocationEntries, MessageLog log, TaskMonitor monitor) throws Exception { + DyldCacheProgramBuilder dyldCacheProgramBuilder = + new DyldCacheProgramBuilder(program, provider, fileBytes, shouldProcessSymbols, + shouldCreateDylibSections, addRelocationEntries, log, monitor); dyldCacheProgramBuilder.build(); } @@ -244,69 +233,73 @@ public class DyldCacheProgramBuilder extends MachoProgramBuilder { * Fixes any chained pointers within each of the data pages. * * @throws MemoryAccessException if there was a problem reading/writing memory. - * @throws CancelledException + * @throws CancelledException if user cancels */ private void fixPageChains() throws MemoryAccessException, CancelledException { long fixedAddressCount = 0; - + // locate slide Info DyldCacheSlideInfoCommon slideInfo = dyldCacheHeader.getSlideInfo(); if (slideInfo == null || !(slideInfo instanceof DyldCacheSlideInfo2)) { - log.appendMsg("Can't handle version " +slideInfo.getVersion() + " slide info, only version 2"); + log.appendMsg( + "Can't handle version " + slideInfo.getVersion() + " slide info, only version 2"); return; } DyldCacheSlideInfo2 slideInfo2 = (DyldCacheSlideInfo2) slideInfo; - + List mappingInfos = dyldCacheHeader.getMappingInfos(); DyldCacheMappingInfo dyldCacheMappingInfo = mappingInfos.get(DATA_PAGE_MAP_ENTRY); long dataPageStart = dyldCacheMappingInfo.getAddress(); long pageSize = slideInfo2.getPageSize(); long pageStartsCount = slideInfo2.getPageStartsCount(); - + long deltaMask = slideInfo2.getDeltaMask(); long deltaShift = Long.numberOfTrailingZeros(deltaMask); long valueAdd = slideInfo2.getValueAdd(); - + short[] pageEntries = slideInfo2.getPageStartsEntries(); short[] extraEntries = slideInfo2.getPageExtrasEntries(); monitor.setMessage("Fixing chained data page pointers..."); - + monitor.setMaximum(pageStartsCount); - for (int index=0; index < pageStartsCount; index++) { + for (int index = 0; index < pageStartsCount; index++) { monitor.checkCanceled(); - + long page = dataPageStart + (pageSize * index); - + monitor.setProgress(index); - - int pageEntry = ((int)pageEntries[index]) & 0xffff; + + int pageEntry = pageEntries[index] & 0xffff; if (pageEntry == DYLD_CACHE_SLIDE_PAGE_ATTR_NO_REBASE) { continue; } - - List

unchainedLocList = new ArrayList
(1024); - + + List
unchainedLocList = new ArrayList<>(1024); + if ((pageEntry & DYLD_CACHE_SLIDE_PAGE_ATTR_EXTRA) != 0) { // go into extras and process list of chain entries for the same page - int extraIndex = (pageEntry & CHAIN_OFFSET_MASK); + int extraIndex = (pageEntry & CHAIN_OFFSET_MASK); do { - pageEntry = ((int) extraEntries[extraIndex]) & 0xffff; + pageEntry = extraEntries[extraIndex] & 0xffff; long pageOffset = (pageEntry & CHAIN_OFFSET_MASK) * BYTES_PER_CHAIN_OFFSET; - - processPointerChain(unchainedLocList, page, pageOffset, deltaMask, deltaShift, valueAdd); - extraIndex++; - } while ((pageEntry & DYLD_CACHE_SLIDE_PAGE_ATTR_EXTRA) == 0); + + processPointerChain(unchainedLocList, page, pageOffset, deltaMask, deltaShift, + valueAdd); + extraIndex++; + } + while ((pageEntry & DYLD_CACHE_SLIDE_PAGE_ATTR_EXTRA) == 0); } - else { - long pageOffset = pageEntry * BYTES_PER_CHAIN_OFFSET; - - processPointerChain(unchainedLocList, page, pageOffset, deltaMask, deltaShift, valueAdd); - } - + else { + long pageOffset = pageEntry * BYTES_PER_CHAIN_OFFSET; + + processPointerChain(unchainedLocList, page, pageOffset, deltaMask, deltaShift, + valueAdd); + } + fixedAddressCount += unchainedLocList.size(); unchainedLocList.forEach(entry -> { - // create a pointer at the fixed up chain pointer location + // create a pointer at the fixed up chain pointer location try { // don't use data utilities. does too much extra checking work listing.createData(entry, Pointer64DataType.dataType); @@ -318,10 +311,10 @@ public class DyldCacheProgramBuilder extends MachoProgramBuilder { } log.appendMsg("Fixed " + fixedAddressCount + " chained pointers. Creating Pointers"); - - monitor.setMessage("Created " + fixedAddressCount + " chained pointers"); + + monitor.setMessage("Created " + fixedAddressCount + " chained pointers"); } - + /** * Fixes up any chained pointers, starting at the given address. * @@ -332,50 +325,49 @@ public class DyldCacheProgramBuilder extends MachoProgramBuilder { * @param deltaShift shift needed for the deltaMask to extract the next offset * @param valueAdd value to be added to each chain pointer * - * @return count of number of addresses fixed - * @throws MemoryAccessException - * @throws CancelledException + * @throws MemoryAccessException IO problem reading file + * @throws CancelledException user cancels */ - private void processPointerChain(List
unchainedLocList, long page, long nextOff, long deltaMask, long deltaShift, long valueAdd) + private void processPointerChain(List
unchainedLocList, long page, long nextOff, + long deltaMask, long deltaShift, long valueAdd) throws MemoryAccessException, CancelledException { - + // TODO: should the image base be used to perform the ASLR slide on the pointers. // currently image is kept at it's initial location with no ASLR. - Address chainStart = memory.getProgram().getLanguage().getDefaultSpace().getAddress(page); + Address chainStart = memory.getProgram().getLanguage().getDefaultSpace().getAddress(page); byte origBytes[] = new byte[8]; - - long valueMask = 0xffffffffffffffffL >>> (64 - deltaShift); - + + long valueMask = 0xffffffffffffffffL >>> (64 - deltaShift); + long delta = -1; while (delta != 0) { monitor.checkCanceled(); - + Address chainLoc = chainStart.add(nextOff); long chainValue = memory.getLong(chainLoc); delta = (chainValue & deltaMask) >> deltaShift; - chainValue = chainValue & valueMask; - if (chainValue != 0) { - chainValue += valueAdd; - } - - if (shouldAddRelocationEntries) { + chainValue = chainValue & valueMask; + if (chainValue != 0) { + chainValue += valueAdd; + } + + if (shouldAddRelocationEntries) { // Add entry to relocation table for the pointer fixup memory.getBytes(chainLoc, origBytes); - program.getRelocationTable().add(chainLoc, (int) 1, - new long[] { chainValue }, origBytes, null); - } - - memory.setLong(chainLoc, chainValue); - - // delay creating data until after memory has been changed - unchainedLocList.add(chainLoc); + program.getRelocationTable().add(chainLoc, 1, new long[] { chainValue }, origBytes, + null); + } + + memory.setLong(chainLoc, chainValue); + + // delay creating data until after memory has been changed + unchainedLocList.add(chainLoc); nextOff += (delta * 4); } } - /** * Processes the DYLD Cache's DYLIB files. This will mark up the DYLIB files, added them to the