Issue 1: readImports()
The function assumes that the entries in the import directory (eg:name) will always reside in
the same section as imports rva, while in fact it is not, the name rva might be anywhere.
Therefore I added an extra check to get the section where the name rva is after extracting it from
directory.
Another problem was how thunk addresses were processed, it was always calculated as an offset from
the imports RVA, I have no idea why was it implemented that way, but it caused A LOT of errors due
to crafted imports table (At our project we are using the emulator to extract features from packed
binary, which almost always messes the header for only the windows loader to understand, so my target
was always to mimc the windows loader even if it is at the expense of performance.)
I reimplemented it so that the offset would have the direct value to the thunk address.
In windows loader, it doesn't check whether the address is in a section or not, its just used as an offset
to get data, while testing the import table,a colleague said it might be possible to have data stored in
the headers, we have a crafted sample to reproduce issue, the sample exploits code cave just to store
a simple name rva just to check and it, as expected, fails.
I thought of a solution so I don't have to store the whole file in memory plus Binee already includes sections,
I decided to store the headers in a separate section, giving it a virtual address of 0, and the raw data and
added a check to getSectionByRva() so if the file wants to access some data from the headers, it will handle it,
return the headers section, and it worked.
Issue 3: readExports()
Upon extracting each export directory, it was extracted correctly, but when it came to setting the export info,
it didn't set the Ordinal correctly, the ordinal base wasn't taken into consideration.
I fixed this by a simple addition with the already extracted ordinal base.
Issue 4: updateRelocation()
It was assumed that there will always be padding to check if it is the last block, which is not the case.
To reproduce try importing/loading ws2_32.dll.
I fixed by checking if there was more to read from the buffer, if it errors I just return.
A better check is to check the size from the DataDirectories.
Refactoring:
There is that function getSectionByRva(uint32), which takes an rva as a input and returns a section,
for some reason, despite having such function, there were some cases where a loop was implemented
to get the section, I removed all that parts and called the function.
Added a boolean variable to represent the operation checking the first bit of import thunk, just to present
better readability.
kgwinnup
commented
4 years ago
Issue 1: readImports() The function assumes that the entries in the import directory (eg:name) will always reside in the same section as imports rva, while in fact it is not, the name rva might be anywhere. Therefore I added an extra check to get the section where the name rva is after extracting it from directory.
Another problem was how thunk addresses were processed, it was always calculated as an offset from the imports RVA, I have no idea why was it implemented that way, but it caused A LOT of errors due to crafted imports table (At our project we are using the emulator to extract features from packed binary, which almost always messes the header for only the windows loader to understand, so my target was always to mimc the windows loader even if it is at the expense of performance.) I reimplemented it so that the offset would have the direct value to the thunk address.
Link to sample:https://www.virustotal.com/gui/file/b582e828fd2952cdf1914df8bd4f6fd558bc2ebd88b1918bb88f648cb7120ad7/detection
Issue 2: Accessing headers data
In windows loader, it doesn't check whether the address is in a section or not, its just used as an offset to get data, while testing the import table,a colleague said it might be possible to have data stored in the headers, we have a crafted sample to reproduce issue, the sample exploits code cave just to store a simple name rva just to check and it, as expected, fails.
Our crafter sample:3a3e2cfd0bfbd4d5c43ca8d3ed8a311888560d7ede959bc6e5598bd805348d9c Link:https://www.virustotal.com/gui/file/3a3e2cfd0bfbd4d5c43ca8d3ed8a311888560d7ede959bc6e5598bd805348d9c/detection
I thought of a solution so I don't have to store the whole file in memory plus Binee already includes sections, I decided to store the headers in a separate section, giving it a virtual address of 0, and the raw data and added a check to getSectionByRva() so if the file wants to access some data from the headers, it will handle it, return the headers section, and it worked.
Issue 3: readExports() Upon extracting each export directory, it was extracted correctly, but when it came to setting the export info, it didn't set the Ordinal correctly, the ordinal base wasn't taken into consideration. I fixed this by a simple addition with the already extracted ordinal base.
reference: https://docs.microsoft.com/en-us/windows/win32/debug/pe-format#export-ordinal-table
Issue 4: updateRelocation() It was assumed that there will always be padding to check if it is the last block, which is not the case. To reproduce try importing/loading ws2_32.dll. I fixed by checking if there was more to read from the buffer, if it errors I just return. A better check is to check the size from the DataDirectories.
Refactoring: There is that function getSectionByRva(uint32), which takes an rva as a input and returns a section, for some reason, despite having such function, there were some cases where a loop was implemented to get the section, I removed all that parts and called the function.
Added a boolean variable to represent the operation checking the first bit of import thunk, just to present better readability.