-
Notifications
You must be signed in to change notification settings - Fork 519
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.
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
Remove print. Fix format. Improve error handling. #263
base: master
Are you sure you want to change the base?
Conversation
Malformed COFF symbol tables can cause pefile to crash (690EB2BFE038A2632F50E8A7E2FB9F64, 3E14E435D4B01127B8526F4B985347E3, A6763BAF63AF7467B3AD3486A0785C9F, etc). Apparently the DWORD symbol table size is bogus and this causes pefile to index out of bound. This commit adds additional error handling to allow parsing to continue even if this occurs.
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] |
There was a problem hiding this comment.
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.
self.__warnings.append('Invalid string table size') | ||
return | ||
|
||
if self.__data__[self.string_table_offset + string_table_size - 1] != 0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line was crashing.
@@ -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) |
There was a problem hiding this comment.
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.
@@ -2713,7 +2717,6 @@ def get_symbol_name(symbol, string_table, string_table_size): | |||
|
|||
return name | |||
|
|||
self.symbols = [] |
There was a problem hiding this comment.
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.
@@ -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 |
There was a problem hiding this comment.
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.
Malformed COFF symbol tables can cause pefile to crash (see 690EB2BFE038A2632F50E8A7E2FB9F64, 3E14E435D4B01127B8526F4B985347E3, A6763BAF63AF7467B3AD3486A0785C9F, etc). Apparently the DWORD symbol table size is bogus and this causes pefile to index out of bound. This commit adds additional error handling to allow parsing to continue even if this occurs.