Skip to content

Commit 3c1e51d

Browse files
committed
Merge branch 'GP-1104_ghizard_Fix_PDB_CLI_processing_bug' into patch
2 parents 7fde6c7 + fd2b5da commit 3c1e51d

File tree

2 files changed

+90
-19
lines changed

2 files changed

+90
-19
lines changed

Ghidra/Features/PDB/src/main/java/ghidra/app/util/pdb/pdbapplicator/PdbApplicator.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,23 @@ void pdbLogAndInfoMessage(Object originator, String message) {
439439
Msg.info(originator, message);
440440
}
441441

442+
/**
443+
* Puts error message to {@link PdbLog} and to Msg.error() which will
444+
* also log a stack trace if exception is specified.
445+
* @param originator a Logger instance, "this", or YourClass.class
446+
* @param message the error message to display/log
447+
* @param exc exception whose stack trace should be reported or null
448+
*/
449+
void pdbLogAndErrorMessage(Object originator, String message, Exception exc) {
450+
PdbLog.message(message);
451+
if (exc != null) {
452+
Msg.error(originator, message);
453+
}
454+
else {
455+
Msg.error(originator, message, exc);
456+
}
457+
}
458+
442459
/**
443460
* Returns the {@link TaskMonitor} to available for this analyzer.
444461
* @return the monitor.
@@ -928,6 +945,7 @@ void addMemorySectionRefinement(PeCoffSectionMsSymbol symbol) {
928945
//==============================================================================================
929946
// CLI-Managed infor methods.
930947
//==============================================================================================
948+
931949
// Currently in CLI, but could move.
932950
boolean isDll() {
933951
return pdbCliManagedInfoManager.isDll();
@@ -938,6 +956,15 @@ boolean isAslr() {
938956
return pdbCliManagedInfoManager.isAslr();
939957
}
940958

959+
/**
960+
* Get CLI metadata for specified tableNum and rowNum within the CLI
961+
* metadata stream.
962+
* @param tableNum CLI metadata stream table index
963+
* @param rowNum table row number
964+
* @return CLI metadata or null if specified tableNum not found
965+
* @throws PdbException if CLI metadata stream is not found in program file bytes
966+
* @throws IndexOutOfBoundsException if specified rowNum is invalid
967+
*/
941968
CliAbstractTableRow getCliTableRow(int tableNum, int rowNum) throws PdbException {
942969
return pdbCliManagedInfoManager.getCliTableRow(tableNum, rowNum);
943970
}

Ghidra/Features/PDB/src/main/java/ghidra/app/util/pdb/pdbapplicator/PdbCliInfoManager.java

Lines changed: 63 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@
3737
*/
3838
public class PdbCliInfoManager {
3939

40+
private PdbApplicator applicator;
41+
42+
private boolean initComplete = false;
4043
private CliStreamMetadata metadataStream;
4144

4245
// TODO: May move these out from this class to a higher level. Would mean passing in
@@ -46,22 +49,43 @@ public class PdbCliInfoManager {
4649

4750
/**
4851
* Manager of CLI-related tables that we might need access to for PDB processing.
49-
* @param applicator {@link PdbApplicator} for which this class is working.
52+
* @param applicator {@link PdbApplicator} for which this class is working (used for logging purposes only).
5053
*/
5154
PdbCliInfoManager(PdbApplicator applicator) {
5255
Objects.requireNonNull(applicator, "applicator may not be null");
53-
metadataStream = getCliStreamMetadata(applicator);
56+
this.applicator = applicator;
57+
}
58+
59+
private synchronized void initialize() {
60+
if (initComplete) {
61+
return;
62+
}
63+
initComplete = true;
64+
metadataStream = getCliStreamMetadata();
5465
}
5566

5667
boolean isDll() {
68+
initialize();
5769
return isDll;
5870
}
5971

6072
boolean isAslr() {
73+
initialize();
6174
return isAslr;
6275
}
6376

64-
CliAbstractTableRow getCliTableRow(int tableNum, int rowNum) throws PdbException {
77+
/**
78+
* Get CLI metadata for specified tableNum and rowNum within the CLI
79+
* metadata stream.
80+
* @param tableNum CLI metadata stream table index
81+
* @param rowNum table row number
82+
* @return CLI metadata or null if specified tableNum not found
83+
* @throws PdbException if CLI metadata stream is not found in program file bytes
84+
* @throws IndexOutOfBoundsException if specified rowNum is invalid
85+
*/
86+
CliAbstractTableRow getCliTableRow(int tableNum, int rowNum)
87+
throws PdbException, IndexOutOfBoundsException {
88+
initialize();
6589
if (metadataStream == null) {
6690
throw new PdbException("CliStreamMetadata is null");
6791
}
@@ -72,47 +96,67 @@ CliAbstractTableRow getCliTableRow(int tableNum, int rowNum) throws PdbException
7296
return table.getRow(rowNum);
7397
}
7498

75-
private CliStreamMetadata getCliStreamMetadata(PdbApplicator applicator) {
99+
/**
100+
* Get CLI stream metadata
101+
* @return CLI stream metadata or null if not found or error occured
102+
*/
103+
private CliStreamMetadata getCliStreamMetadata() {
76104
Program program = applicator.getProgram();
77105
if (program == null) {
78106
return null;
79107
}
80108

81109
List<FileBytes> allFileBytes = program.getMemory().getAllFileBytes();
110+
if (allFileBytes.isEmpty()) {
111+
applicator.pdbLogAndErrorMessage(this,
112+
"Unable to retrieve CliStreamMetadata: no FileBytes", null);
113+
return null;
114+
}
82115
FileBytes fileBytes = allFileBytes.get(0); // Should be that of main imported file
83-
ByteProvider provider = new FileBytesProvider(fileBytes);
84-
PortableExecutable pe = null;
116+
ByteProvider provider = new FileBytesProvider(fileBytes); // close not required
85117
try {
86118
GenericFactory factory = MessageLogContinuesFactory.create(applicator.getMessageLog());
87-
pe = PortableExecutable.createPortableExecutable(factory, provider, SectionLayout.FILE,
88-
true, true);
89-
NTHeader ntHeader = pe.getNTHeader();
119+
PortableExecutable pe = PortableExecutable.createPortableExecutable(factory, provider,
120+
SectionLayout.FILE, true, true);
121+
NTHeader ntHeader = pe.getNTHeader(); // will be null if header parse fails
122+
if (ntHeader == null) {
123+
applicator.pdbLogAndErrorMessage(this,
124+
"Unable to retrieve CliStreamMetadata: NTHeader file bytes not found", null);
125+
return null;
126+
}
90127
OptionalHeader optionalHeader = ntHeader.getOptionalHeader();
91128
int characteristics = ntHeader.getFileHeader().getCharacteristics();
92129
isDll = (characteristics & FileHeader.IMAGE_FILE_DLL) == FileHeader.IMAGE_FILE_DLL;
93130
DataDirectory[] dataDirectory = optionalHeader.getDataDirectories();
94131
int optionalHeaderCharaceristics = optionalHeader.getDllCharacteristics();
95132
isAslr = (optionalHeaderCharaceristics &
96133
OptionalHeader.IMAGE_DLLCHARACTERISTICS_HIGH_ENTROPY_VA) == OptionalHeader.IMAGE_DLLCHARACTERISTICS_HIGH_ENTROPY_VA;
134+
if (OptionalHeader.IMAGE_DIRECTORY_ENTRY_COM_DESCRIPTOR >= dataDirectory.length) {
135+
applicator.pdbLogAndErrorMessage(this,
136+
"Unable to retrieve CliStreamMetadata: Bad index (" +
137+
OptionalHeader.IMAGE_DIRECTORY_ENTRY_COM_DESCRIPTOR +
138+
") for COMDescriptorDataDirectory in DataDirectory array of size " +
139+
dataDirectory.length,
140+
null);
141+
return null;
142+
}
97143
COMDescriptorDataDirectory comDir =
98144
(COMDescriptorDataDirectory) dataDirectory[OptionalHeader.IMAGE_DIRECTORY_ENTRY_COM_DESCRIPTOR];
99145
ImageCor20Header header = comDir.getHeader();
100146
if (header == null) {
147+
applicator.pdbLogAndErrorMessage(this,
148+
"Unable to retrieve CliStreamMetadata: no COMDir header", null);
101149
return null;
102150
}
103151
return header.getMetadata().getMetadataRoot().getMetadataStream();
104152
}
105-
catch (Exception e) {
106-
applicator.pdbLogAndInfoMessage(this, "Unable to retrieve CliStreamMetadata");
153+
catch (RuntimeException | IOException e) {
154+
// We do not know what can go wrong. Some of the header parsing might have issues,
155+
// and we'd rather log the error and limp on by with whatever other processing we can
156+
// do than to fail here.
157+
applicator.pdbLogAndErrorMessage(this,
158+
"Unable to retrieve CliStreamMetadata: " + e.getMessage(), e);
107159
return null;
108160
}
109-
finally {
110-
try {
111-
provider.close();
112-
}
113-
catch (IOException ioe) {
114-
applicator.pdbLogAndInfoMessage(this, "Problem closing ByteProvider");
115-
}
116-
}
117161
}
118162
}

0 commit comments

Comments
 (0)