From ea519f8d7ee8508b9f4bca98d7000e975b473941 Mon Sep 17 00:00:00 2001 From: Ryan Kurtz Date: Fri, 12 Jun 2026 04:16:41 -0400 Subject: [PATCH] GP-6960: Better handling of malformed SizeOfRawData in PE files --- .../ghidra/app/util/opinion/PeLoader.java | 112 +++++++++--------- 1 file changed, 57 insertions(+), 55 deletions(-) diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/PeLoader.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/PeLoader.java index 1d3c866289..ef417dd5b9 100644 --- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/PeLoader.java +++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/PeLoader.java @@ -673,67 +673,69 @@ public class PeLoader extends AbstractPeDebugLoader { x = ((sections[i].getCharacteristics() & SectionFlags.IMAGE_SCN_MEM_EXECUTE.getMask()) != 0x0); - int rawDataSize = sections[i].getSizeOfRawData(); - int rawDataPtr = sections[i].getPointerToRawData(); + int rawSize = sections[i].getSizeOfRawData(); + int rawPtr = sections[i].getPointerToRawData(); + long fileBytesSize = fileBytes.getSize(); virtualSize = sections[i].getVirtualSize(); MemoryBlock block = null; - if (rawDataSize != 0 && rawDataPtr != 0) { - int dataSize = - ((rawDataSize > virtualSize && virtualSize > 0) || rawDataSize < 0) - ? virtualSize - : rawDataSize; - if (ntHeader.checkRVA(dataSize) || - (0 < dataSize && dataSize < pe.getFileLength())) { - if (!ntHeader.checkRVA(dataSize)) { - Msg.warn(this, "OptionalHeader.SizeOfImage < size of " + - sections[i].getName() + " section"); - } - block = MemoryBlockUtils.createInitializedBlock(prog, false, sectionName, - address, fileBytes, rawDataPtr, dataSize, "", "", r, w, x, log); - sectionToAddress.put(sections[i], address); - } - if (rawDataSize == virtualSize) { - continue; - } - else if (rawDataSize > virtualSize) { - // virtual size fully initialized - continue; - } - // remainder of virtual size is uninitialized - if (rawDataSize < 0) { - Msg.error(this, - "Section[" + i + "] has invalid size " + - Integer.toHexString(rawDataSize) + " (" + - Integer.toHexString(virtualSize) + ")"); - break; - } - virtualSize -= rawDataSize; - address = address.add(rawDataSize); - } if (virtualSize == 0) { - Msg.error(this, "Section[" + i + "] has size zero"); + Msg.error(this, "Section '%s' has size zero".formatted(sectionName)); } - else { - int dataSize = (virtualSize > 0 || rawDataSize < 0) ? virtualSize : 0; - if (dataSize > 0) { - if (block != null) { - MemoryBlock paddingBlock = MemoryBlockUtils.createInitializedBlock(prog, - false, sectionName, address, dataSize, "", "", r, w, x, log); - if (paddingBlock != null) { - try { - prog.getMemory().join(block, paddingBlock); - } - catch (Exception e) { - log.appendMsg(e.getMessage()); - } + + // Malware can deliberately set invalid or abnormally large values of SizeOfRawData + // to break static analysis. If we suspect that is happening, default to the + // VirtualSize. This may result in padding bytes being replaced by garbage bytes, + // but it will allow the binary to be imported and analyzed. + if (rawSize < 0) { + log.appendMsg("WARNING: Section '%s' has negative SizeOfRawData (0x%x)" + .formatted(sectionName, rawSize)); + log.appendMsg("Using '%s' VirtualSize (0x%x) in its place" + .formatted(sectionName, virtualSize)); + rawSize = virtualSize; + } + + // If PointerToRawData and SizeOfRawData are set, make an initialized block from + // the FileBytes + if (rawSize != 0 && rawPtr != 0) { + int n = rawSize > virtualSize && virtualSize > 0 ? virtualSize : rawSize; + if (rawPtr + n > fileBytesSize) { + int orig = n; + n = (int) (fileBytesSize - rawPtr); + log.appendMsg( + "WARNING: Section '%s' (0x%x) extends beyond end of file...truncating to 0x%x" + .formatted(sectionName, orig, n)); + } + if (n > 0) { + block = MemoryBlockUtils.createInitializedBlock(prog, false, sectionName, + address, fileBytes, rawPtr, n, "", "", r, w, x, log); + sectionToAddress.put(sections[i], address); + virtualSize -= n; + address = address.add(n); + } + } + + // If there is any virtual size left, map in the padding bytes + if (virtualSize > 0) { + if (block != null) { + // Put the padding bytes in the same block we created above + MemoryBlock paddingBlock = MemoryBlockUtils.createInitializedBlock(prog, + false, sectionName, address, virtualSize, "", "", r, w, x, log); + if (paddingBlock != null) { + try { + prog.getMemory().join(block, paddingBlock); + } + catch (Exception e) { + log.appendMsg(e.getMessage()); } } - else { - MemoryBlockUtils.createUninitializedBlock(prog, false, sectionName, - address, dataSize, "", "", r, w, x, log); - sectionToAddress.putIfAbsent(sections[i], address); - } + } + else { + // We didn't create an initialized block above. Put all the padding in a + // new uninitialized block. + MemoryBlockUtils.createUninitializedBlock(prog, false, sectionName, + address, virtualSize, "", "", r, w, x, log); + sectionToAddress.putIfAbsent(sections[i], address); } } @@ -743,7 +745,7 @@ public class PeLoader extends AbstractPeDebugLoader { if (optionalHeader.getFileAlignment() != optionalHeader.getSectionAlignment()) { throw new IllegalStateException(ise); } - Msg.warn(this, "Section header processing aborted"); + log.appendMsg("Section header processing aborted"); } return sectionToAddress;