handle PE delay import tables with VA pointers instead of RVA pointers #439
Conversation
This comment has been minimized.
This comment has been minimized.
- detect RVA vs VA in parseDelayImports - make flavor="import table"/"delay import table" more clear
|
ready for review @rakuy0. tests pass and no further pending changes. |
There was a problem hiding this comment.
Just a few small questions. The only thing I think this is really missing is some kind of test.
Do you have a binary you can throw into https://github.com/vivisect/vivtestfiles so you or I could write a test for this?
| entry_name = x.rvaDLLName | ||
| else: | ||
| raise ValueError("unexpected flavor: " + flavor) |
There was a problem hiding this comment.
Since you have the check for the type above, are these additional checks necessary?
There was a problem hiding this comment.
when dealing with enum-style things in python i like to explicitly name each case and raise an exception otherwise. this helps catch bugs in the future if another enum member is added but the author forgets to update a switch/case. using a single if/else hides the bug and goes to the wrong logic block.
| # more votes for RVA than for VA | ||
| return sum([1 for vote in votes if vote]) > sum([1 for vote in votes if not vote]) | ||
|
|
||
| def parseImportTable(self, x, irva, flavor="import table", uses_rva=True): |
There was a problem hiding this comment.
Is flavor ever going to expand beyond normal or delayed imports (I'm just curious about the change from a boolean to a string)?
There was a problem hiding this comment.
i dont know. gcc delay import table???
i found is_import to be confusing: if its not an import table, what is the alternative? using the flavor string makes it very clear to the reader what variety of thing is being parsed.
#438 references a binary from VT. i'll add a test case with this sample. |
|
binary for testcase: vivisect/vivtestfiles#19 |
|
test added. no further pending work. |
closes #438
I'm sorry there's so much new code, but I've commented thoroughly so its clear what I'm doing and why. In theory, this code is only relevant for VS6 binaries (uncommon).
unrelated tweak to the parameter of
parseImportTableto make it explicit which flavor of table is being parsed. i foundis_importto be confusing: if its not an import table, what is the alternative? (ok, i know its delay import table but its not written out anywhere).