Skip to content
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

Detect more endings #139

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

STKFLT
Copy link

@STKFLT STKFLT commented Feb 27, 2021

I found some cases where Ropper would output fewer instructions for a given binary than ROPGadget. I found that the regex for end instructions on both x86 and ARM are overly specific:

x86: missing the retf/retn distinction

ARM: only included one type of pop when imo any pop into pc (aside from conditionals) should qualify as an end instruction. also the JOP code for ARM was missing some forms of bl and blx

Most of the regex here is pulled from ROPGadget and I attempted to validate as best I could against the instruction set documentation (https://iitd-plos.github.io/col718/ref/arm-instructionset.pdf)

There is also a regex compilation caching feature I added that relates to another pull request I am going to send soon.

it culled valid but weird gadgets (e.g. bx pc) which is allowed but deprecated
@STKFLT
Copy link
Author

STKFLT commented Feb 27, 2021

after thinking more about this and experimenting some, it may make sense to modify this somewhat. on ARM, the pop (i.e. ldm) base register can be any register and as such, an instruction like ldmda r4!, {r0, r8, sb, sl, fp, sp, pc} is better categorized as a JOP gadget

@STKFLT STKFLT marked this pull request as draft February 27, 2021 20:29
@STKFLT
Copy link
Author

STKFLT commented Feb 27, 2021

Other case in question:
add sp, sp, #0xc; ldm sp!, {pc}

@STKFLT
Copy link
Author

STKFLT commented Feb 28, 2021

The refined split between ROP and JOP for ldm* instructions is:
its ROP if:

  • its unconditional
  • up/down bit set to 1, meaning it must go up the stack
  • PSR & force user bit set to 0 or 1. I don't know enough about arm to understand the implications of this bit but it doesn't seem to change that we are changing pc based on a stack value and moving sp
  • write back bit set to 1. If we aren't also moving sp then this doesn't behave like a ret style instruction
  • base register is sp
  • pc is in registers to load

its JOP if:

  • its unconditional
    -up/down bit set to whatever
  • PSR & force user bit set to whatever
  • write back bit set to whatever
  • base register is NOT sp
  • pc is in registers to load

@sashs
Copy link
Owner

sashs commented Mar 4, 2021

Hi.
Many thanks for you pull requests. I will check it.
One question:
In the file rop.py: Why did you add the imagebase in the for loop if you subtract it again afterwards?

@STKFLT
Copy link
Author

STKFLT commented Mar 4, 2021

I add it when disassembling the gadget so that relative branch targets are correct (I can show an example if you'd like), but then subtract it for the gadget address because there's logic elsewhere that adds the imageBase back on for printing e.g.

toReturn = '%s (%s): ' % (cstr(toHex(self._lines[0][0] + self.imageBase, self.__arch.addressLength), analyseColor),cstr(toHex(address + self.imageBase, self.__arch.addressLength), Color.GREEN))

I'm open to trying to combine all of that so it's consistent, I just wanted to keep the changes narrowed down until you weighed in.

Are there other areas where the distinction between address and address+imageBase are important beyond the string functions in gadget.py?

@STKFLT
Copy link
Author

STKFLT commented Mar 4, 2021

Also I've totally just mixed together these pull requests at this point, sorry :/

@sashs sashs marked this pull request as ready for review April 25, 2021 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants