Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove print. Fix format. Improve error handling. #263

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Remove print. Fix format. Improve error handling. #263

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 17 additions & 11 deletions pefile.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1093,7 +1093,7 @@ def dump(self, indentation=0):
if key.startswith('Signature_'):
val_str = '{:8X}'.format(val)
else:
val_str = '{#:8X}'.format(val)
val_str = '0x{:8X}'.format(val)

if key == 'TimeDateStamp' or key == 'dwTimeStamp':
try:
Expand Down Expand Up @@ -2322,7 +2322,7 @@ def __parse__(self, fname, data, fast_load, is_obj):
'AddressOfEntryPoint lies outside the sections\' boundaries. '
'AddressOfEntryPoint: 0x%x' %
self.OPTIONAL_HEADER.AddressOfEntryPoint )

if not fast_load:
self.full_load()

Expand Down Expand Up @@ -2517,6 +2517,7 @@ def parse_section_relocations(self, section):

reloc_size = Structure(self.__COFF_RELOCATION_format__).sizeof()

has_sym = hasattr(self, 'symbols')
for i in range(section.NumberOfRelocations):
start = section.PointerToRelocations + reloc_size * i
end = start + reloc_size
Expand All @@ -2529,7 +2530,7 @@ def parse_section_relocations(self, section):
self.__data__[start : end],
file_offset = start)

if reloc.SymbolTableIndex >= len(self.symbols):
if has_sym and (reloc.SymbolTableIndex >= len(self.symbols)):
self.__warnings.append('Invalid symbol table index ({}) for relocation {} in section {}'.format(
reloc.SymbolTableIndex, i, section.get_name()))
continue
Expand Down Expand Up @@ -2660,21 +2661,25 @@ def parse_symbol_table(self):
symbol_size = Structure(self.__COFF_SYMBOL_format__).sizeof()
self.string_table_offset = self.FILE_HEADER.PointerToSymbolTable + self.FILE_HEADER.NumberOfSymbols * symbol_size

if self.string_table_offset + 4 >= len(self.__data__):
self.symbols = []
self.strings = {}

if (self.string_table_offset + 4) >= len(self.__data__):
self.__warnings.append('Symbol table is corrupt')
return

string_table_size = struct.unpack('<I', self.__data__[self.string_table_offset : self.string_table_offset + 4])[0]
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the DWORD value that can apparently be invalid in some PE files. Loaded one of the samples in IDA pro and it complained of the same value for the string value size.


if string_table_size < 4:
# This can be an invalid offset triggering an index error later
offset = self.string_table_offset + string_table_size - 1
if (string_table_size < 4) or (offset >= len(self.__data__)):
self.__warnings.append('Invalid string table size')
return

if self.__data__[self.string_table_offset + string_table_size - 1] != 0:
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line was crashing.

if self.__data__[offset] != 0:
self.__warnings.append('String table corrupted, last string not terminated')
return

self.strings = {}
offset = 4
string_table_end = self.string_table_offset + string_table_size
string_table = self.__data__[self.string_table_offset : string_table_end]
Expand All @@ -2697,7 +2702,6 @@ def get_symbol_name(symbol, string_table, string_table_size):
name = s
else:
end = string_table[offset:].find(0)
print(offset, end)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My script was producing a lot of output in the most recent version of pefile and I identified this as the line producing it.

if end >= 0:
name = string_table[offset:offset + end].decode('utf-8', 'replace')

Expand All @@ -2713,7 +2717,6 @@ def get_symbol_name(symbol, string_table, string_table_size):

return name

self.symbols = []
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This value is reference elsewhere and can cause AttributeError if it's not defined.

symbol_offset = self.FILE_HEADER.PointerToSymbolTable

while symbol_offset < self.string_table_offset:
Expand Down Expand Up @@ -5503,13 +5506,16 @@ def dump_section_relocations(self, section):
lines.append('-' * 79)
reloc_table = MACHINE_TYPE_TO_RELOCATION.get(self.FILE_HEADER.Machine)

if not hasattr(self, 'symbols'):
lines.append('<PARSE ERROR>')
return lines
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is to avoid dump_info from crashing if the COFF parsing was aborted.


for i, reloc in enumerate(section.relocations):
symbol_name = self.symbols[reloc.SymbolTableIndex].__name__

if reloc_table:
reloc_type_desc = reloc_table.get(reloc.Type)
if reloc_type_desc:
reloc_type = reloc_type_desc
reloc_type = reloc_type_desc if reloc_type_desc else '<UNKNOWN>'

lines.append('{index:8}{vaddr:#10x}{sym_table_index:8} {symbol_name:35} {type}'.format(
index=i, vaddr=reloc.VirtualAddress, sym_table_index=reloc.SymbolTableIndex, symbol_name=symbol_name, type=reloc_type))
Expand Down