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

Refactor pwnlib.encoders.encode() to be more nice to bytes objects #1923

Open
wants to merge 8 commits into
base: stable
Choose a base branch
from

Conversation

heapcrash
Copy link
Collaborator

@heapcrash heapcrash commented Jun 25, 2021

  • Add byteset() to misc which does "the right thing" and returns a set of bytes objects
  • Make bytes_avoid be a bytes() object instead of a set
  • Update documentation to note that functions take bytes() and not str()
  • Move Encoder class to its own module so that it doesn't show up in docs
  • Remove documentation for individual encoders, ideally people should only use the public APIs

Example Script (examples/encoder.py)

This requires running from make -C extra/docker develop in order for the current username to be "pwntools".

#!/usr/bin/env python3
from pwn import *

context.randomize = False
context.log_level = 'error'

def go():
    info('========== %s ==========', context.arch)
    with context.silent:
        sc = asm(shellcraft.sh())
        avoid=b'\x00\n\t '
        enc = encode(sc, avoid=avoid, force=1)

    assert not (byteset(avoid) & byteset(enc))
    assert enc != sc

    with context.silent:
        io = ELF.from_bytes(enc).process()
        io.sendline('whoami')

    try:
        info('%r', io.recvline() == b'pwntools\n')
    except EOFError:
        info('EOFError')

    with context.silent:
        io.close()


context.clear(arch='i386')
go()

context.clear(arch='amd64')
go()

context.clear(arch='arm')
go()

context.clear(arch='mips')
go()

Python3

$ python3 examples/encoder.py
[*] ========== i386 ==========
[*] True
[*] ========== amd64 ==========
[*] True
[*] ========== arm ==========
[*] True
[*] ========== mips ==========
[*] True

Python2

$ python2 examples/encoder.py
[*] ========== i386 ==========
[*] True
[*] ========== amd64 ==========
[*] True
[*] ========== arm ==========
[*] True
[*] ========== mips ==========
[*] True

* Add byteset() to misc which does "the right thing" and returns a set of bytes
* Make bytes_avoid be a bytes() object instead of a set
* Update documentation to note that functions take bytes() and not str()
Also adds examples/encoder.py to make sure the encoders can work in
simple situations, avoiding basic characters, and being forced to
encode even if the avoid= are missing in the original bytes.
@heapcrash heapcrash requested a review from Arusekk June 25, 2021 12:45
@heapcrash heapcrash changed the base branch from dev to stable June 25, 2021 12:49
@Arusekk
Copy link
Member

Arusekk commented Jun 25, 2021

Personally I would prefer the set to be a set of numbers, but looks good.

@heapcrash
Copy link
Collaborator Author

heapcrash commented Jun 25, 2021

Personally I would prefer the set to be a set of numbers, but looks good.

The issue with this is the difference between Py2 and Py3. I think there's value in keeping the behavior of the two as close as possible, for making the code as simple as possible.

Py2

>>> set(b'asdf')
{'a', 'd', 'f', 's'}
>>> byteset(b'asdf')
{'a', 'd', 'f', 's'}

Py3

>>> set(b'asdf')
{97, 100, 102, 115}
>>> byteset(b'asdf')
{b'a', b'd', b'f', b's'}

@Arusekk
Copy link
Member

Arusekk commented Jun 25, 2021 via email

@heapcrash
Copy link
Collaborator Author

I hereby vote the next release is Pwntools v5.000 😛

@heapcrash
Copy link
Collaborator Author

heapcrash commented Jul 8, 2021 via email

Copy link
Member

@peace-maker peace-maker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Allowing to run the encoders again at least is great.

pwnlib/encoders/encoder.py Outdated Show resolved Hide resolved
pwnlib/encoders/encoder.py Outdated Show resolved Hide resolved
@peace-maker
Copy link
Member

I agree with having a byteset and maybe byteiter helper. It's not really 100% relevant to this PR, but a way to implement it. You don't have to use it, but there were occasions where I wanted the bytes and had to p8(x) again while looping over some bytes.

.. automodule:: pwnlib.encoders.encoder
:members:

.. automodule:: pwnlib.encoders.i386.ascii_shellcode
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ASCII encoder isn't added to the encoder mapping in the Encoder class. We should keep the docs for that one, since it has to be used explicitly.

It has extra settings worth documenting and decodes the shellcode onto the stack, so a jmp esp is required afterwards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants