HexHive / retrowrite

RetroWrite -- Retrofitting compiler passes through binary rewriting
Other
655 stars 78 forks source link

issues with handling aliased symbols #17

Open junxzm1990 opened 3 years ago

junxzm1990 commented 3 years ago

Hi,

The current version of retrowrite may not properly handle aliased symbols.

An object, such as a function, can have multiple aliased symbols. However, the loading process at https://github.com/HexHive/retrowrite/blob/master/librw/loader.py#L124 just picks the last one. This can cause problems because other references to the same object use the symbol linked to the relocation (e.g., https://github.com/HexHive/retrowrite/blob/master/librw/rw.py#L185) --- the symbol picked by the loading process and the symbol linked to the relocation can be different, though they are aliases.

A safer strategy is to keep all aliased symbols in the assembly file by using the ".set" primitive.

I have committed a tentative "patch" to the repo I forked locally: https://github.com/junxzm1990/retrowrite.

FYI, a tentative "patch" to the naming issues of global symbols I mentioned at https://github.com/HexHive/retrowrite/issues/15 is also committed to my local repo.

cyanpencil commented 3 years ago

Thank you for the bug report. I don't think we ever tested on binaries with aliased symbols, so you are most likely right on the cause of the bug.

We will have a look at your patches, thanks for sending them. Can I ask you if you provide the problematic binary here too, so we can test it ourselves?

junxzm1990 commented 3 years ago

@cyanpencil I tested the code on the Quick PDF Library (x64, PIE, symbols): https://www.debenu.com/products/development/debenu-pdf-library/?__hstc=16885030.d1c7d6c178e7392c96be82a9ed0d552e.1605134306956.1605192347633.1605711983878.4&__hssc=16885030.5.1605711983878&__hsfp=4103256049

Just FYI, I noticed more issues from the current version of Retrowrite: -- The statement at https://github.com/HexHive/retrowrite/blob/master/librw/rw.py#L352 will symbolize data objects as "LCaddr". This won't work with many external symbols whose addresses are set up as 0. -- The statement at https://github.com/HexHive/retrowrite/blob/master/librw/rw.py#L315 does not consider the type of relocation. However, for types like "R_X86_64_64", you need to go through the symbol table and get the name from the symbol indexed for the relocation. Simply using the name carried by the relocation can cause problems. -- ".tbss" is ignored. This can break thread-related operations (at least in the Quick PDF Library).

Retrowrite is a cool tool and I like it: small code base + specialized goals. Hopefully, we can make it better together.

junxzm1990 commented 3 years ago

BTW, I just committed a new version to my local repo at https://github.com/junxzm1990/retrowrite

I just played some quick and "dirty" tricks to make it work with some preliminary tests on the Quick PDF Library (without any instrumentation).

Hope it helps.

junxzm1990 commented 3 years ago

Just fixed more issues. Now my local version can work Quick PDF Library, using both g++ and clang++ to reassemble the disassembled results.

cyanpencil commented 3 years ago

Amazing! Would you like to open a pull request so that you get proper credit for your fixes? If you don't have time we will integrate your patches ourselves. It's gonna take some days to process it, as currently we are busy on adding support for aarch64 binaries and have some tight deadlines to meet, so please be patient with us.

diagprov commented 3 years ago

Hello everyone, just to let you know I'm currently working on a separate issue that is also closely related to the issue here: https://github.com/HexHive/retrowrite/issues/16

In this case the following relocations appear in the relocation table:

  1687: 000000000030b8d8     8 OBJECT  LOCAL  DEFAULT   26 memset_v.3282
  1964: 000000000030b8e0     8 OBJECT  LOCAL  DEFAULT   26 memset_v.2768

and break the relocation logic you mention for handling R_X86_64_64. As you say, it isn't a case of st_value+addend any more and so this reports "LC0", which causes things to break. If you don't have time to handle this case I am already looking at it; if you do please let me know so I can integrate and test your fixes.

Currently Retrowrite only supports PIE executable so we have to be careful here (supporting non-PIE is a future project). I am currently unsure why this particular example has emitted these two relocations (it also has a fully functioning plt for finding actual memset), but I have some ideas and I will come back to you when I know.

junxzm1990 commented 3 years ago

@diagprov the outputs you posted look like symbols. Can you post the relocations?

I should have fixed the problems related to "LC0" in my local fork at https://github.com/junxzm1990/retrowrite. I hoped to PR my fixes and also customize a Retrowrite tool for LibFuzzer. But I will have to defer that to December as I am on some other papers at this moment.

In my previous posts, I said the following:

_"-- The statement at https://github.com/HexHive/retrowrite/blob/master/librw/rw.py#L315 does not consider the type of relocation. However, for types like "R_X86_6464", you need to go through the symbol table and get the name from the symbol indexed for the relocation. Simply using the name carried by the relocation can cause problems."

I found this is not correct after I zoomed into the code again. The 'name' filed carried by a relocation item is indeed the name carried by the symbol indexed by the relocation (which is done by loader.py). But the loop at https://github.com/HexHive/retrowrite/blob/master/librw/rw.py#L306 still needs some revision: certain RIP-relative memory accesses correspond to no relocations (no cases where "target" == relocation['offset']), but the target actually matches the location of certain symbols (there are cases where "target" == symbol['st_value']). Such cases should also be symbolized; there are also cases where both ""target" == relocation['offset']" and ""target" == symbol['st_value']" are true, but "symbol" is different from the symbol indexed by the "relocation"; in such cases, ""target" == symbol['st_value']" should be prioritized; Finally, R_X86_64_DTPMOD64 is not handled.

I have committed a tentative workaround at https://github.com/junxzm1990/retrowrite/blob/master/librw/rw.py#L377. But still, my fix may not be complete and it definitely needs some refinements.