Make pwn template always set context.binary

[FlorianKothmeier]

pwn template only set context.binary when an argument was specified and only defined a placeholder path instead. It would be more reasonable to set context.binary even if no path was specified as pwntools can automatically determine the correct settings for the binary when you fill in the placeholder path.

Old template without specifying target binary as argument:

#!/usr/bin/env python3
# -*- coding: utf-8 -*-
# This exploit template was generated via:
# $ pwn template
from pwn import *

# Set up pwntools for the correct architecture
context.update(arch='i386')
exe = './path/to/binary'

# Many built-in settings can be controlled on the command-line and show up
# in "args".  For example, to dump all data sent/received, and disable ASLR
# for all created processes...
# ./exploit.py DEBUG NOASLR


def start(argv=[], *a, **kw):
    '''Start the exploit against the target.'''
    if args.GDB:
        return gdb.debug([exe] + argv, gdbscript=gdbscript, *a, **kw)
    else:
        return process([exe] + argv, *a, **kw)

# Specify your GDB script here for debugging
# GDB will be launched if the exploit is run via e.g.
# ./exploit.py GDB
gdbscript = '''
continue
'''.format(**locals())

#===========================================================
#                    EXPLOIT GOES HERE
#===========================================================

io = start()

# shellcode = asm(shellcraft.sh())
# payload = fit({
#     32: 0xdeadbeef,
#     'iaaa': [1, 2, 'Hello', 3]
# }, length=128)
# io.send(payload)
# flag = io.recv(...)
# log.success(flag)

io.interactive()

New template without specifying target binary as argument:

#!/usr/bin/env python3
# -*- coding: utf-8 -*-
# This exploit template was generated via:
# $ pwn template
from pwn import *

# Set up pwntools for the correct architecture
exe = context.binary = ELF('./path/to/binary')

# Many built-in settings can be controlled on the command-line and show up
# in "args".  For example, to dump all data sent/received, and disable ASLR
# for all created processes...
# ./exploit.py DEBUG NOASLR



def start(argv=[], *a, **kw):
    '''Start the exploit against the target.'''
    if args.GDB:
        return gdb.debug([exe.path] + argv, gdbscript=gdbscript, *a, **kw)
    else:
        return process([exe.path] + argv, *a, **kw)

# Specify your GDB script here for debugging
# GDB will be launched if the exploit is run via e.g.
# ./exploit.py GDB
gdbscript = '''
continue
'''.format(**locals())

#===========================================================
#                    EXPLOIT GOES HERE
#===========================================================

io = start()

# shellcode = asm(shellcraft.sh())
# payload = fit({
#     32: 0xdeadbeef,
#     'iaaa': [1, 2, 'Hello', 3]
# }, length=128)
# io.send(payload)
# flag = io.recv(...)
# log.success(flag)

io.interactive()

Partially resolves #2276

[peace-maker]

Thank you! This might help simple usecases of just invoking pwn template.

This feels like we need to improve the documentation around how to use the template generator tool. When using the plain invocation instead of providing the target binary during pwn template ./challenge invocation, the path has to be edited into the generated script manually, which seems redundant and like a misuse of the template generator :(

This also shifts the requirement to change the generated script right away to remote-only exploit templates generated using pwn template --host abc.com --port 1234 since it'd try to access the ./path/to/binary dummy file now, so you'd have to comment out that line and remember to add the context.update(arch='XXX') back manually.

Maybe the best compromise would be to keep the current output of not using context.binary if there is no binary given and host is set.

[FlorianKothmeier]

Thanks for the feedback!

What's the preferred development approach here? Should changes from the review stay as a separate commit or be squashed into the first one?

[peace-maker]

Separate commits are fine for such small changes like this. We squash the whole PR branch before merging.

[peace-maker]

Is #2309 an acceptable solution to this problem too?

[FlorianKothmeier]

@peace-maker Yes, that patch sounds good. However, this still leaves he old behavior when the challenge files could not be determined automatically. But this is probably good enough

[peace-maker]

We can still add the change to include context.binary when no exe was set nor found. I'd still like to avoid crashing when trying to open the './path/to/binary' file if you only specify a remote host and port. There often are remote-only challenges without a handout.

[FlorianKothmeier]

@peace-maker I just added a commit that implements the fallback you suggested for remote-only challenges. Sorry that it took me this long

[peace-maker]

Thank you, I like it!