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.

Sign up for GitHub

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

Jump to bottom

Enhance documentation and error logging for preexec_fn usage #787

Merged
merged 1 commit into from
Nov 12, 2016
Merged

Enhance documentation and error logging for preexec_fn usage #787

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 28 additions & 6 deletions pwnlib/tubes/ssh.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ def getenv(self, variable, **kwargs):
"""Retrieve the address of an environment variable in the remote process.
"""
if not hasattr(self, 'argv'):
log.error("Can only call getenv() on ssh_channel objects created with ssh.process")
self.error("Can only call getenv() on ssh_channel objects created with ssh.process")

argv0 = self.argv[0]

Expand Down Expand Up @@ -667,8 +667,12 @@ def process(self, argv=None, executable=None, tty=True, cwd=None, env=None, time
See ``stdin``.
preexec_fn(callable):
Function which is executed on the remote side before execve().
This **MUST** be a self-contained function -- it must perform
all of its own imports, and cannot refer to variables outside
its scope.
Copy link
Contributor

Choose a reason for hiding this comment

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

We could just check whether this is the case by inspecting the callable object.

Copy link
Member Author

Choose a reason for hiding this comment

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

You can't, actually. And if you can, it's more complex than I care to worry about. If things break, the exception is thrown on the other side and all we have is the Python stack trace -- which appears as part of the tube data. There's not a way around this that I'm aware of.

Copy link
Contributor

Choose a reason for hiding this comment

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

I thought you chould just check if func_closure is None/() and if func_code.co_names only contains names from __builtins__.

However def f(): import x also has x in its co_names. So I guess you are right.

Copy link
Member Author

Choose a reason for hiding this comment

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

😞

preexec_args(object):
Argument passed to ``preexec_fn``.
This **MUST** only consist of native Python objects.
raw(bool):
If ``True``, disable TTY control code interpretation.
aslr(bool):
Expand Down Expand Up @@ -715,6 +719,23 @@ def process(self, argv=None, executable=None, tty=True, cwd=None, env=None, time
''
>>> s.process('/usr/bin/env', env={'A':'B'}).recvall()
'A=B\n'

>>> s.process('false', preexec_fn=1234)
Traceback (most recent call last):
...
PwnlibException: preexec_fn must be a function

>>> s.process('false', preexec_fn=lambda: 1234)
Traceback (most recent call last):
...
PwnlibException: preexec_fn cannot be a lambda

>>> def uses_globals():
... foo = bar
>>> print s.process('false', preexec_fn=uses_globals).recvall().strip() # doctest: +ELLIPSIS
Traceback (most recent call last):
...
NameError: global name 'bar' is not defined
"""
if not argv and not executable:
self.error("Must specify argv or executable")
Expand Down Expand Up @@ -766,15 +787,16 @@ def process(self, argv=None, executable=None, tty=True, cwd=None, env=None, time
# Allow the user to provide a self-contained function to run
def func(): pass
func = preexec_fn or func
func_src = inspect.getsource(func).strip()
func_name = func.__name__
func_args = preexec_args

if not isinstance(func, types.FunctionType):
log.error("preexec_fn must be a function")
self.error("preexec_fn must be a function")

func_name = func.__name__
if func_name == (lambda: 0).__name__:
log.error("preexec_fn cannot be a lambda")
self.error("preexec_fn cannot be a lambda")

func_src = inspect.getsource(func).strip()
setuid = setuid if setuid is None else bool(setuid)

script = r"""
Expand Down Expand Up @@ -1454,7 +1476,7 @@ def upload(self, file_or_directory, remote=None):
if os.path.isdir(file_or_directory):
return self.upload_dir(file_or_directory, remote)

log.error('%r does not exist' % file_or_directory)
self.error('%r does not exist' % file_or_directory)


def download(self, file_or_directory, remote=None):
Expand Down