search

Gallopsled / pwntools

CTF framework and exploit development library
http://pwntools.com
Other
11.98k stars 1.7k forks source link

`Context.log_file` does not allow bytes type #2384

Closed marinelay closed 5 months ago

marinelay commented 5 months ago

https://github.com/Gallopsled/pwntools/blob/5981c7290dd5190733c1f2e9ccf08633869f8b31/pwnlib/context/__init__.py#L1007-L1039

In the code, it seems that Context.log_file allows bytes type (at line 1034), but it raises TypeError: a bytes-like object is required, not 'str' at line 1036 in Python 3.x.

I think it makes sense to either delete bytes type or cast it to string type. If bytes is for Python 2.x, then it would be nice to replace it to str (because str and bytes are same in Python 2.x).

It is simplified code raising TypeError:

import tempfile
from pwnlib.context import context

foo_txt = tempfile.mktemp().encode()
context.log_file = foo_txt

Thank you!

peace-maker commented 5 months ago

Thanks for the report. We usually deal with those by implicitly converting string arguments to bytes and only use bytes internally. There is misc.need_bytes for this. Would you be willing to propose a pull request to fix this?

marinelay commented 5 months ago

Sure!

As I understand your comment, you use bytes internally, so you're asking me to ensure that bytes can be used without errors as well. If I've misunderstood, please let me know.

Additionally, I looked up misc.need_bytes, but I found only util.packing._need_bytes function. I think it is a function that replaces str to bytes, whereas we need the opposite functionality for this issue. Hence, I will use util.packing._need_text or util.packing._decode to solve this issue.

If the above is okay, I'll write PR for you!