search

NationalSecurityAgency / ghidra

Ghidra is a software reverse engineering (SRE) framework
https://www.nsa.gov/ghidra
Apache License 2.0
50.67k stars 5.78k forks source link

Loss of stack depth when calling a function #4295

Open Wall-AF opened 2 years ago

Wall-AF commented 2 years ago

Describe the bug The stack depth is lost (i.e. zeroed and ends up '-'ve) after a call to another function inside the one you are decompiling on applications using SP as the stack pointer.

To Reproduce Steps to reproduce the behavior:

  1. Unzip the attached zip file,
  2. Load the xml file (exported from the "Debug Function Decompilation" menu action for the function),
  3. Ensure your Listing: view has the "Stack Depth" column visible,
  4. See error

Expected behavior To use the correct register/value to determine the stak depth after a function call.

Attachments There are 2 examples in the attachmement dragmedi_FUN_1008_1434.zip

Environment (please complete the following information):

Additional context When looking at a 16-bit x86 application the stack pointer is SP. Tracking this through the SymbolcPropogator class I've discovered in method handleFunctionSideEffects(...) the stack varnode used to adjust the current position is ESP and dispite that being added to the context earlier in the process, I don't believe the SP value is propagated to ESP resulting in a big fat zero in subsequent calculations instead of the value stored in SP!

Wall-AF commented 2 years ago

It seems that class VarnodeContext the method getStackRegister(...) is using the "parent" register (if it has one) and if that is commented out, the stack depth is corrected. https://github.com/NationalSecurityAgency/ghidra/blob/7a30cefebb91ddb25a6c4030e67001202fd20936/Ghidra/Features/Base/src/main/java/ghidra/program/util/VarnodeContext.java#L358-L373

The questions are:

  1. Why use the parent register; and
  2. Are there some hidden/non-obvious consequences to removing it?