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

Shellcode encoder flakky behaviour #1761

Closed
rhythmize opened this issue Jan 15, 2021 · 4 comments · Fixed by #1767
Closed

Shellcode encoder flakky behaviour #1761

rhythmize opened this issue Jan 15, 2021 · 4 comments · Fixed by #1767

Comments

@rhythmize
Copy link
Contributor

I'm trying to generate an encoded shellcode with pwntools using following code. I'm running it with python3

>>> from pwn import *
>>> context.arch = 'i386'
>>> shellcode = asm(shellcraft.execve('/bin/sh'))
>>> encode(shellcode, avoid=bytearray([0x31]))

This snippet is showing flakky behaviour for generation shellcode, sometimes it generates the shellcode and other times it crash.
During crash following is the traceback I get.

[!] Encoder <pwnlib.encoders.i386.delta.i386DeltaEncoder object at 0x7f0038a1e850> did not succeed
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/algorhythm/.local/lib/python3.9/site-packages/pwnlib/context/__init__.py", line 1449, in setter
    return function(*a, **kw)
  File "/home/algorhythm/.local/lib/python3.9/site-packages/pwnlib/encoders/encoder.py", line 82, in encode
    v = encoder(raw_bytes, avoid, pcreg)
  File "/home/algorhythm/.local/lib/python3.9/site-packages/pwnlib/encoders/i386/xor.py", line 70, in __call__
    a, b = xor_pair(raw_bytes, avoid)
  File "/home/algorhythm/.local/lib/python3.9/site-packages/pwnlib/util/fiddling.py", line 374, in xor_pair
    avoid = avoid.encode('utf-8')
AttributeError: 'set' object has no attribute 'encode'

I tried debugging it via traceback, but it appears that for the bytearray I'm passing as the avoid parameter to encode function, the type of that parameter is somehow being interpreted as a set in case of failure, which is causing the crash. In case of successful shellcode generation, I'm assuming the type of the avoid parameter is intact.

What I'm unable to understand is how can the same code behaves differently on subsequent executions.

Please suggest. All inputs are much appreciated.

Thanks in advance.

@Arusekk
Copy link
Member

Arusekk commented Jan 15, 2021

Yes, this is a bug. We should fix it in some time, but if you wish to help, please submit a PR before I do :)

But first what do you mean by "sometimes"? What do you change between invocations so that it behaves differently?

@rhythmize
Copy link
Contributor Author

There's nothing changed between the invocations, that's what's confusing me more. Like if I only re-execute the last line in the python shell

encode(shellcode, avoid=bytearray([0x31]))

I get crash on some of the subsequent calls and then at one of the executions, there's no error (without any change in the code) and shellcode is generated successfuly. Nothing is changed between these calls and yet I see different behaviour on subsequent executions of same function call.

@rhythmize
Copy link
Contributor Author

Hi,

I found the reason for different behaviours for encode function call.
Actually, the issue happens to be in xor encoder but not in delta encoder.

So if I use, delta encoder

encoders.i386.delta.encode(shellcode, avoid=bytearray([0x31]))

I am getting successful shellcode generation consistently.

Whereas, with xor encoder

encoders.i386.xor.encode(shellcode, avoid=bytearray([0x31]))

I'm getting the crash consistently.

The random behaviour is because of line 75 in encoder.py which shuffles the list of available encoders. Thus arbitrarily selecting delta or xor encoder if we don't specify which encoder to use.

rhythmize added a commit to rhythmize/pwntools that referenced this issue Jan 16, 2021
Don't encode `avoid` parameter in `fiddler.py` if type is `bytearray`
@rhythmize
Copy link
Contributor Author

rhythmize commented Jan 16, 2021

Hi @Arusekk, I have opened a PR to fix the issue, please review and let me know if any changes are required.
Thanks

@Arusekk Arusekk linked a pull request Jan 16, 2021 that will close this issue
rhythmize added a commit to rhythmize/pwntools that referenced this issue Jun 1, 2021
Don't encode `avoid` parameter in `fiddler.py` if type is `bytearray`
Arusekk pushed a commit that referenced this issue Jun 5, 2021
* Fix i386 encoder issue #1761

Don't encode `avoid` parameter in `fiddler.py` if type is `bytearray`

* Pass `avoid` as `bytes` instead of `set` when `encoder.encode()` is called

* Requested change: Increase readability

* Update doctest in XOR encoder

Add my failing testcase to ensure the code is fixed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants