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

ROPGadget multibr #1678

Merged
merged 3 commits into from
Sep 19, 2020
Merged

ROPGadget multibr #1678

merged 3 commits into from
Sep 19, 2020

Conversation

152334H
Copy link
Contributor

@152334H 152334H commented Sep 19, 2020

cf. JonathanSalwan/ROPgadget#145

Some ROP chains need a syscall; ret gadget. Adding the --multibr argument allows for this.

Testing

from pwn import *
context.log_level = 'warn'
rop = ROP('../libc-database/db/libc6_2.27-3ubuntu1_amd64.so')
print(rop.find_gadget(['syscall', 'ret']))

output:

Gadget(0xd2975, ['syscall', 'ret'], [], 0x4)

Edit

The first two commits didn't pass the build because the int 0x80 instructions in the docstrings are displayed with 'ret' appended:

$ diff *.log -y | grep \|
Got:                                                          | Expected:
    0x0008:       0x1000000b int 0x80; ret                    |     0x0008:       0x1000000b int 0x80
    0x0044:       0x1000000b int 0x80; ret                    |     0x0044:       0x1000000b int 0x80
Got:                                                          | Expected:
    0x002c:       0x10000000 int 0x80; ret                    |     0x002c:       0x10000000 int 0x80
    0x0068:       0x10000000 int 0x80; ret                    |     0x0068:       0x10000000 int 0x80
Got:                                                          | Expected:
    0x804802c:       0x10000000 int 0x80; ret                 |     0x804802c:       0x10000000 int 0x80
    0x8048068:       0x10000000 int 0x80; ret                 |     0x8048068:       0x10000000 int 0x80

This is a consequence of ROP().gadgets[] being indexed by address; the change appears purely cosmetic

$ ROPgadget --binary ../temp.elf --only 'sysenter|syscall|int|add|pop|leave|ret' --nojop --multibr
0x10000000 : int 0x80
0x10000000 : int 0x80 ; ret

Consequently I've patched the docstrings to match the new behavior.

@Arusekk Arusekk merged commit 72ca730 into Gallopsled:dev Sep 19, 2020
@152334H
Copy link
Contributor Author

152334H commented Sep 22, 2020

Regrettably, the small change was not cosmetic:

>>> from pwn import *
>>> pwnlib.__version__
'4.4.0dev0'
>>> context.arch = 'amd64'
>>> context.binary = ELF.from_assembly('syscall; ret')
[*] '/tmp/pwn-asm-5z32nly4/step3'
    Arch:     amd64-64-little
    RELRO:    No RELRO
    Stack:    No canary found
    NX:       NX disabled
    PIE:      No PIE (0x10000000)
    RWX:      Has RWX segments
>>> r = ROP(context.binary)
[*] Loaded 2 cached gadgets for '/tmp/pwn-asm-5z32nly4/step3'
>>> r.gadgets
{268435458: Gadget(0x10000002, ['ret'], [], 0x8), 268435456: Gadget(0x10000000, ['syscall', 'ret'], [], 0x8)}
>>> r.syscall is not None
False # !!!! Should not be false !!!!

Code responsible is here

I'll be opening another PR to hotpatch this (self.gadgets[each]['insns'] == [mapping[attr]] --> self.gadgets[each]['insns'][0] == mapping[attr]; this appears to match prior behavior), but this does undermine my confidence in there being no other side effects.

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