From c35c74b617e6be48834c2dad00d304518d21e283 Mon Sep 17 00:00:00 2001
From: dev747368 <48332326+dev747368@users.noreply.github.com>
Date: Wed, 4 Aug 2021 13:29:21 -0400
Subject: [PATCH] GP-1177 Add support for long section names in PE binaries.
 Add support for long PE section names that are formatted like: '/1234'.

---
 .../app/util/bin/format/pe/FileHeader.java    | 24 +++++-
 .../app/util/bin/format/pe/SectionHeader.java | 82 ++++++++++---------
 .../bin/format/pe/SeparateDebugHeader.java    | 14 ++--
 .../bin/format/pe/debug/DebugCOFFSymbol.java  | 26 +++---
 4 files changed, 86 insertions(+), 60 deletions(-)

diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pe/FileHeader.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pe/FileHeader.java
index 37998726ce..b42db049fa 100644
--- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pe/FileHeader.java
+++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pe/FileHeader.java
@@ -356,9 +356,11 @@ public class FileHeader implements StructConverter {
 			Msg.error(this, "File alignment == 0: section processing skipped");
 		}
 		else {
+			long stringTableOffset = getStringTableOffset();
 			sectionHeaders = new SectionHeader[numberOfSections];
 			for (int i = 0; i < numberOfSections; ++i) {
-				sectionHeaders[i] = SectionHeader.createSectionHeader(reader, tmpIndex);
+				sectionHeaders[i] =
+					SectionHeader.readSectionHeader(reader, tmpIndex, stringTableOffset);
 
 				// Ensure PointerToRawData + SizeOfRawData doesn't exceed the length of the file
 				int pointerToRawData = sectionHeaders[i].getPointerToRawData();
@@ -439,7 +441,7 @@ public class FileHeader implements StructConverter {
 			return;
 		}
 
-		int stringTableIndex = tmpIndex + DebugCOFFSymbol.IMAGE_SIZEOF_SYMBOL * numberOfSymbols;
+		long stringTableOffset = getStringTableOffset();
 
 		for (int i = 0; i < numberOfSymbols; ++i) {
 			if (!ntHeader.checkRVA(tmpIndex)) {
@@ -448,7 +450,7 @@ public class FileHeader implements StructConverter {
 			}
 
 			DebugCOFFSymbol symbol =
-				DebugCOFFSymbol.createDebugCOFFSymbol(reader, tmpIndex, stringTableIndex);
+				DebugCOFFSymbol.createDebugCOFFSymbol(reader, tmpIndex, stringTableOffset);
 
 			tmpIndex += DebugCOFFSymbol.IMAGE_SIZEOF_SYMBOL;
 
@@ -464,6 +466,22 @@ public class FileHeader implements StructConverter {
 		reader.setPointerIndex(oldIndex);
 	}
 
+	/**
+	 * Return the offset of the string table, or -1 if invalid or not present.
+	 * 
+	 * @return long offset of string table, or -1 if invalid or not present
+	 * @throws IOException if io error
+	 */
+	long getStringTableOffset() throws IOException {
+		if (pointerToSymbolTable <= 0 /* 0 is excluded because other stuff is there */ ||
+			!ntHeader.checkRVA(pointerToSymbolTable) || (numberOfSymbols < 0) ||
+			(pointerToSymbolTable + (numberOfSymbols * DebugCOFFSymbol.IMAGE_SIZEOF_SYMBOL) > reader
+					.length())) {
+			return -1;
+		}
+		return pointerToSymbolTable + (DebugCOFFSymbol.IMAGE_SIZEOF_SYMBOL * numberOfSymbols);
+	}
+
 	public boolean isLordPE() {
 		if (getPointerToSymbolTable() == LORDPE_SYMBOL_TABLE &&
 			getNumberOfSymbols() == LORDPE_NUMBER_OF_SYMBOLS) {
diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pe/SectionHeader.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pe/SectionHeader.java
index 6eba2f307a..53e5570371 100644
--- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pe/SectionHeader.java
+++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pe/SectionHeader.java
@@ -17,9 +17,7 @@ package ghidra.app.util.bin.format.pe;
 
 import java.io.*;
 
-import ghidra.app.util.bin.ByteArrayConverter;
-import ghidra.app.util.bin.StructConverter;
-import ghidra.app.util.bin.format.FactoryBundledWithBinaryReader;
+import ghidra.app.util.bin.*;
 import ghidra.program.model.data.*;
 import ghidra.program.model.mem.*;
 import ghidra.util.Conv;
@@ -237,29 +235,53 @@ public class SectionHeader implements StructConverter, ByteArrayConverter {
 	private short numberOfLinenumbers;
 	private int characteristics;
 
-	private FactoryBundledWithBinaryReader reader;
-	private long index;
-
-	static SectionHeader createSectionHeader(FactoryBundledWithBinaryReader reader, long index)
-			throws IOException {
-		SectionHeader sectionHeader =
-			(SectionHeader) reader.getFactory().create(SectionHeader.class);
-		sectionHeader.initSectionHeader(reader, index);
-		return sectionHeader;
-	}
+	private BinaryReader reader;
 
 	/**
-	 * DO NOT USE THIS CONSTRUCTOR, USE create*(GenericFactory ...) FACTORY METHODS INSTEAD.
+	 * Read a {@link SectionHeader} from the specified stream starting at {@code index}.
+	 *  
+	 * @param reader {@link BinaryReader} to read from
+	 * @param index long offset in the reader where the section header starts
+	 * @param stringTableOffset offset of the string table, or -1 if not available
+	 * @return new {@link SectionHeader}
+	 * @throws IOException if error reading data
 	 */
-	public SectionHeader() {
+	public static SectionHeader readSectionHeader(BinaryReader reader, long index,
+			long stringTableOffset) throws IOException {
+		SectionHeader result = new SectionHeader();
+
+		result.reader = reader;
+
+		result.name = reader.readAsciiString(index, IMAGE_SIZEOF_SHORT_NAME).trim();
+		if (result.name.startsWith("/") && stringTableOffset != -1) {
+			try {
+				int nameOffset = Integer.parseInt(result.name.substring(1));
+				result.name = reader.readAsciiString(stringTableOffset + nameOffset);
+			}
+			catch (NumberFormatException nfe) {
+				// ignore error, section name will remain as it was
+			}
+		}
+
+		// we need to skip IMAGE_SIZEOF_SHORT_NAME chars no matter what,
+		// since those bytes are always allocated
+		reader.setPointerIndex(index + IMAGE_SIZEOF_SHORT_NAME);
+
+		result.physicalAddress = result.virtualSize = reader.readNextInt();
+		result.virtualAddress = reader.readNextInt();
+		result.sizeOfRawData = reader.readNextInt();
+		result.pointerToRawData = reader.readNextInt();
+		result.pointerToRelocations = reader.readNextInt();
+		result.pointerToLinenumbers = reader.readNextInt();
+		result.numberOfRelocations = reader.readNextShort();
+		result.numberOfLinenumbers = reader.readNextShort();
+		result.characteristics = reader.readNextInt();
+
+		return result;
 	}
 
-	private void initSectionHeader(FactoryBundledWithBinaryReader reader, long index)
-			throws IOException {
-		this.reader = reader;
-		this.index = index;
-
-		parse();
+	private SectionHeader() {
+		// empty
 	}
 
 	SectionHeader(MemoryBlock block, OptionalHeader optHeader, int ptr) {
@@ -493,24 +515,6 @@ public class SectionHeader implements StructConverter, ByteArrayConverter {
 		return buff.toString();
 	}
 
-	private void parse() throws IOException {
-		name = reader.readAsciiString(index, IMAGE_SIZEOF_SHORT_NAME).trim();
-
-		// we need to skip IMAGE_SIZEOF_SHORT_NAME chars no matter what,
-		// since those bytes are always allocated
-		reader.setPointerIndex(index + IMAGE_SIZEOF_SHORT_NAME);
-
-		physicalAddress = virtualSize = reader.readNextInt();
-		virtualAddress = reader.readNextInt();
-		sizeOfRawData = reader.readNextInt();
-		pointerToRawData = reader.readNextInt();
-		pointerToRelocations = reader.readNextInt();
-		pointerToLinenumbers = reader.readNextInt();
-		numberOfRelocations = reader.readNextShort();
-		numberOfLinenumbers = reader.readNextShort();
-		characteristics = reader.readNextInt();
-	}
-
 	@Override
 	public DataType toDataType() throws DuplicateNameException {
 		UnionDataType union = new UnionDataType("Misc");
diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pe/SeparateDebugHeader.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pe/SeparateDebugHeader.java
index b546f7ad1b..f8f80d0702 100644
--- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pe/SeparateDebugHeader.java
+++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pe/SeparateDebugHeader.java
@@ -15,6 +15,10 @@
  */
 package ghidra.app.util.bin.format.pe;
 
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.List;
+
 import generic.continues.GenericFactory;
 import ghidra.app.util.bin.ByteProvider;
 import ghidra.app.util.bin.format.FactoryBundledWithBinaryReader;
@@ -22,10 +26,6 @@ import ghidra.app.util.bin.format.pe.debug.DebugDirectoryParser;
 import ghidra.util.Conv;
 import ghidra.util.Msg;
 
-import java.io.IOException;
-import java.util.ArrayList;
-import java.util.List;
-
 /**
  * <pre>
  * typedef struct _IMAGE_SEPARATE_DEBUG_HEADER {
@@ -114,12 +114,12 @@ public class SeparateDebugHeader implements OffsetValidator {
 
 		sections = new SectionHeader[numberOfSections];
 		for (int i = 0; i < numberOfSections; ++i) {
-			sections[i] = SectionHeader.createSectionHeader(reader, ptr);
+			sections[i] = SectionHeader.readSectionHeader(reader, ptr, -1);
 			ptr += SectionHeader.IMAGE_SIZEOF_SECTION_HEADER;
 		}
 
 		long tmp = ptr;
-		List<String> exportedNameslist = new ArrayList<String>();
+		List<String> exportedNameslist = new ArrayList<>();
 		while (true) {
 			String str = reader.readAsciiString(tmp);
 			if (str == null || str.length() == 0) {
@@ -257,6 +257,7 @@ public class SeparateDebugHeader implements OffsetValidator {
 		return parser;
 	}
 
+	@Override
 	public boolean checkPointer(long ptr) {
 		for (int i = 0; i < sections.length; ++i) {
 			long rawSize = sections[i].getSizeOfRawData() & Conv.INT_MASK;
@@ -269,6 +270,7 @@ public class SeparateDebugHeader implements OffsetValidator {
 		return false;
 	}
 
+	@Override
 	public boolean checkRVA(long rva) {
 		return (0 <= rva) && (rva <= sizeOfImage);
 	}
diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pe/debug/DebugCOFFSymbol.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pe/debug/DebugCOFFSymbol.java
index 24949f884c..8af28f604a 100644
--- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pe/debug/DebugCOFFSymbol.java
+++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pe/debug/DebugCOFFSymbol.java
@@ -15,13 +15,13 @@
  */
 package ghidra.app.util.bin.format.pe.debug;
 
-import ghidra.app.util.bin.*;
-import ghidra.app.util.bin.format.*;
-import ghidra.program.model.data.*;
-import ghidra.util.*;
-import ghidra.util.exception.*;
+import java.io.IOException;
 
-import java.io.*;
+import ghidra.app.util.bin.*;
+import ghidra.app.util.bin.format.FactoryBundledWithBinaryReader;
+import ghidra.program.model.data.*;
+import ghidra.util.Conv;
+import ghidra.util.exception.DuplicateNameException;
 
 /**
  * A class to represent the COFF symbol data structure.
@@ -135,10 +135,10 @@ public class DebugCOFFSymbol implements StructConverter {
         return createDebugCOFFSymbol(reader, index, symbolTable.getStringTableIndex());
     }
 
-    public static DebugCOFFSymbol createDebugCOFFSymbol(
-            FactoryBundledWithBinaryReader reader, int index,
-            int stringTableIndex) throws IOException {
-        DebugCOFFSymbol debugCOFFSymbol = (DebugCOFFSymbol) reader.getFactory().create(DebugCOFFSymbol.class);
+	public static DebugCOFFSymbol createDebugCOFFSymbol(FactoryBundledWithBinaryReader reader,
+			int index, long stringTableIndex) throws IOException {
+		DebugCOFFSymbol debugCOFFSymbol =
+			(DebugCOFFSymbol) reader.getFactory().create(DebugCOFFSymbol.class);
         debugCOFFSymbol.initDebugCOFFSymbol(reader, index, stringTableIndex);
         return debugCOFFSymbol;
     }
@@ -148,7 +148,8 @@ public class DebugCOFFSymbol implements StructConverter {
      */
     public DebugCOFFSymbol() {}
 
-    private void initDebugCOFFSymbol(FactoryBundledWithBinaryReader reader, int index, int stringTableIndex) throws IOException {
+	private void initDebugCOFFSymbol(FactoryBundledWithBinaryReader reader, int index,
+			long stringTableIndex) throws IOException {
         // read the union first...
         //
         int shortVal = reader.readInt(index);
@@ -295,7 +296,8 @@ public class DebugCOFFSymbol implements StructConverter {
         return numberOfAuxSymbols;
     }
 
-    public DataType toDataType() throws DuplicateNameException, IOException {
+    @Override
+	public DataType toDataType() throws DuplicateNameException, IOException {
     	String structureName = StructConverterUtil.parseName(DebugCOFFSymbol.class);
     	
     	Structure structure = new StructureDataType(structureName + "_" +numberOfAuxSymbols, 0);