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

Wrap commands cont. ';' in sh when $SHELL == zsh #635

Closed
wants to merge 2 commits into from
Closed

Wrap commands cont. ';' in sh when $SHELL == zsh #635

wants to merge 2 commits into from

Conversation

justinsteven
Copy link
Contributor

Preface: I might be going about this all the wrong way, but this is a personal workaround for something that annoys me a lot. Others' mileage may vary.

tmux and zsh seem to have issues with SIGINT and I don't know whose fault it is, or if it's working as designed on their behalf.

This behaviour manifests when run_in_new_terminal() is told to run foo;bar (e.g. when we call gdb with a -x'd script and try to clean the script up after gdb exits - see

pwntools/pwnlib/gdb.py

Line 310 in fc5b7b9

cmd += ' -x "%s" ; rm "%s"' % (tmp.name, tmp.name)
). Hitting Ctrl-C (e.g. to break in to the program running in the debugger) and trying to resize the pane causes gdb to die and the pane to disappear.

It only seems to occur when the user's shell is zsh.

Minimal reproducer for this behaviour:

% tmux splitw 'man ascii'

Hit Ctrl-C on man's pager and resize the pane. Everything is ok.

% tmux splitw 'man ascii; id'

Hit Ctrl-C on man's pager and resize the pane. The pane disappears.

In the second case above, tmux is doing zsh -c 'man ascii;id' and zsh within tmux is seemingly interfering with the SIGINT that Ctrl-C sends.

The workaround

In run_in_new_terminal(), when we're heading down the tmux split path AND the user's shell is zsh AND command contains ; (we're probably trying to do more than one thing at a time) then wrap everything in sh -c '' and go through escaping hell to handle ' and \ within commands properly.

It might be worth using this workaround more often or all the time to simplify code path flow and maximise code coverage, but this will result in a lot of "useless" invocations of sh.

Tests

pwn.gdb.attach() on a non-pwnlib managed process

% pidof sleep

% sleep 600 &
[1] 18451

% env -u DISPLAY python
Python 2.7.11+ (default, May  9 2016, 15:54:33)
[GCC 5.3.1 20160429] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import pwn
>>> pwn.context.log_level = 'debug'
>>> pwn.gdb.attach("sleep")
[*] attaching to youngest process "sleep" (PID = 18451)
[*] running in new terminal: gdb-multiarch "/bin/sleep" 18451
[DEBUG] Launching a new terminal: ['/usr/bin/tmux', 'splitw', 'gdb-multiarch "/bin/sleep" 18451']
[x] Waiting for debugger
[+] Waiting for debugger: Done
>>> pwn.gdb.attach("sleep", execute="continue")
[*] attaching to youngest process "sleep" (PID = 18451)
[*] running in new terminal: gdb-multiarch "/bin/sleep" 18451 -x "/tmp/pwnoAz___.gdb" ; rm "/tmp/pwnoAz___.gdb"
[DEBUG] Launching a new terminal: ['/usr/bin/tmux', 'splitw', 'sh -c \'gdb-multiarch "/bin/sleep" 18451 -x "/tmp/pwnoAz___.gdb" ; rm "/tmp/pwnoAz___.gdb"\'']
[x] Waiting for debugger
[+] Waiting for debugger: Done
>>> quit()

Both invocations of gdb worked, and the panes were resizable with and without doing Ctrl-C beforehand. /tmp/pwnoAz___.gdb got cleaned up.

pwn.gdb.attach() on a pwnlib-managed process via pwnlib.tubes.process

% env -u DISPLAY python
Python 2.7.11+ (default, May  9 2016, 15:54:33)
[GCC 5.3.1 20160429] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import pwn
>>> pwn.context.log_level = 'debug'
>>> p = pwn.process(["sleep", "600"])
[x] Starting program '/bin/sleep' argv=['sleep', '600']
[+] Starting program '/bin/sleep' argv=['sleep', '600'] : Done
>>> pwn.gdb.attach(p)
[*] running in new terminal: gdb-multiarch "/bin/sleep" 18967
[DEBUG] Launching a new terminal: ['/usr/bin/tmux', 'splitw', 'gdb-multiarch "/bin/sleep" 18967']
[x] Waiting for debugger
[+] Waiting for debugger: Done
>>> pwn.gdb.attach(p, execute="continue")
[*] running in new terminal: gdb-multiarch "/bin/sleep" 18967 -x "/tmp/pwnhXCVPI.gdb" ; rm "/tmp/pwnhXCVPI.gdb"
[x] Waiting for debugger
[+] Waiting for debugger: Done
>>> quit()
[*] Stopped program '/bin/sleep'

Both invocations of gdb worked, and the panes were resizable with and without doing Ctrl-C beforehand. /tmp/pwnhXCVPI.gdb got cleaned up.

pwn.gdb.attach() on a pwnlib-managed process via pwnlib.tubes.ssh using password authentication

% env -u DISPLAY python
Python 2.7.11+ (default, May  9 2016, 15:54:33)
[GCC 5.3.1 20160429] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import pwn
>>> pwn.log_level = 'debug'
>>> s = pwn.ssh(host="172.18.0.2", user="justin", password="<REDACTED>")
[x] Connecting to 172.18.0.2 on port 22
[+] Connecting to 172.18.0.2 on port 22: Done
>>> p = s.process(["sleep", "600"])
[x] Opening new channel: execve('sleep', ['sleep', '600'], os.environ)
[+] Opening new channel: execve('sleep', ['sleep', '600'], os.environ): Done
>>> pwn.gdb.attach(p)
[x] Opening new channel: "stty raw -ctlecho -echo; cd '.' >/dev/null 2>&1; mktemp"
[+] Opening new channel: "stty raw -ctlecho -echo; cd '.' >/dev/null 2>&1; mktemp": Done
[x] Receiving all data
[x] Receiving all data: 0B
[+] Receiving all data: Done (20B)
[*] Closed SSH channel with 172.18.0.2
>>> pwn.gdb.attach(p, execute="continue")
[x] Opening new channel: "stty raw -ctlecho -echo; cd '.' >/dev/null 2>&1; mktemp"
[+] Opening new channel: "stty raw -ctlecho -echo; cd '.' >/dev/null 2>&1; mktemp": Done
[x] Receiving all data
[x] Receiving all data: 0B
[+] Receiving all data: Done (20B)
[*] Closed SSH channel with 172.18.0.2
>>> [1]  + done       sleep 600
>>> quit()

Both invocations of gdb worked, and the panes were resizable with and without doing Ctrl-C beforehand. An empty tempfile and a tempfile containing continue got left on the remote host (see #629 which I need to revisit)

pwn.gdb.attach() on a pwnlib-managed process via pwnlib.tubes.ssh using key authentication

% env -u DISPLAY python
Python 2.7.11+ (default, May  9 2016, 15:54:33)
[GCC 5.3.1 20160429] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import pwn
>>> pwn.log_level = 'debug'
>>> s = pwn.ssh(host="172.18.0.2", user="justin")
[x] Connecting to 172.18.0.2 on port 22
[+] Connecting to 172.18.0.2 on port 22: Done
>>> p = s.process(["sleep", "600"])
[x] Opening new channel: execve('sleep', ['sleep', '600'], os.environ)
[+] Opening new channel: execve('sleep', ['sleep', '600'], os.environ): Done
>>> pwn.gdb.attach(p)
[x] Opening new channel: "stty raw -ctlecho -echo; cd '.' >/dev/null 2>&1; mktemp"
[+] Opening new channel: "stty raw -ctlecho -echo; cd '.' >/dev/null 2>&1; mktemp": Done
[x] Receiving all data
[x] Receiving all data: 0B
[+] Receiving all data: Done (0B)
[*] Closed SSH channel with 172.18.0.2
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "pwnlib/gdb.py", line 203, in attach
    shell.upload_data(execute or '', tmpfile)
  File "pwnlib/tubes/ssh.py", line 1117, in upload_data
    self.sftp.put(f.name, remote)
  File "build/bdist.linux-x86_64/egg/paramiko/sftp_client.py", line 676, in put
  File "build/bdist.linux-x86_64/egg/paramiko/sftp_client.py", line 634, in putfo
  File "build/bdist.linux-x86_64/egg/paramiko/sftp_client.py", line 327, in open
  File "build/bdist.linux-x86_64/egg/paramiko/sftp_client.py", line 730, in _request
  File "build/bdist.linux-x86_64/egg/paramiko/sftp_client.py", line 781, in _read_response
  File "build/bdist.linux-x86_64/egg/paramiko/sftp_client.py", line 807, in _convert_status
IOError: [Errno 2] No such file
>>> pwn.gdb.attach(p, "continue")
[x] Opening new channel: "stty raw -ctlecho -echo; cd '.' >/dev/null 2>&1; mktemp"
[+] Opening new channel: "stty raw -ctlecho -echo; cd '.' >/dev/null 2>&1; mktemp": Done
[x] Receiving all data
[x] Receiving all data: 0B
[+] Receiving all data: Done (20B)
[*] Closed SSH channel with 172.18.0.2

Not sure what happened to the first pwn.gdb.attach() but I tried it again for with fingers crossed and it worked:

>>> pwn.gdb.attach(p)
[x] Opening new channel: "stty raw -ctlecho -echo; cd '.' >/dev/null 2>&1; mktemp"
[+] Opening new channel: "stty raw -ctlecho -echo; cd '.' >/dev/null 2>&1; mktemp": Done
[x] Receiving all data
[x] Receiving all data: 0B
[+] Receiving all data: Done (20B)
[*] Closed SSH channel with 172.18.0.2
>>> quit()

The second and third invocations of gdb worked, and the panes were resizable with and without doing Ctrl-C beforehand. Two empty tempfiles and a tempfile containing continue got left on the remote host (see #629 which I need to revisit)

Creating some files:

% env -u DISPLAY python
Python 2.7.11+ (default, May  9 2016, 15:54:33)
[GCC 5.3.1 20160429] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import pwn
>>> pwn.context.log_level = 'debug'
>>> cmd = "touch /tmp/ab"
>>> pwn.run_in_new_terminal(cmd)
[DEBUG] Launching a new terminal: ['/usr/bin/tmux', 'splitw', 'touch /tmp/ab']

/tmp/ab gets touched

>>> cmd = "touch /tmp/a'b"
>>> print cmd
touch /tmp/a'b
>>> pwn.run_in_new_terminal(cmd)
[DEBUG] Launching a new terminal: ['/usr/bin/tmux', 'splitw', "touch /tmp/a'b"]

/tmp/a'b does not get touched. This is an existing escaping bug outside of what this PR modifies (we're not hitting the new code because command doesn't contain ;), and probably warrants attention.

>>> cmd = "touch /tmp/abc; id"
>>> print cmd
touch /tmp/abc; id
>>> pwn.run_in_new_terminal(cmd)
[DEBUG] Launching a new terminal: ['/usr/bin/tmux', 'splitw', "sh -c 'touch /tmp/abc; id'"]

/tmp/abc gets touched

>>> cmd = "touch /tmp/a'bc; id"
>>> print cmd
touch /tmp/a'bc; id
>>> pwn.run_in_new_terminal(cmd)
[DEBUG] Launching a new terminal: ['/usr/bin/tmux', 'splitw', "sh -c 'touch /tmp/a'\\\\\\''bc; id'"]

/tmp/a'bc gets touched

>>> cmd = "touch /tmp/a\\'bc; id"
>>> print cmd
touch /tmp/a\'bc; id
>>> pwn.run_in_new_terminal(cmd)
[DEBUG] Launching a new terminal: ['/usr/bin/tmux', 'splitw', "sh -c 'touch /tmp/a\\\\'\\\\\\''bc; id'"]

/tmp/a\'bc gets touched

@justinsteven
Wrap commands cont. ';' in sh when $SHELL == zsh
203c79a 
works around issues of zsh+tmux and strange handling of SIGINT
@justinsteven
Copy link
Contributor Author

Appreciate this is a gnarly PR owing to the foul shell escaping.

It might be worth going down the wrap-with-sh path even when $SHELL != zsh and/or when command doesn't contain ; to simplify the code paths. This would probably also fix the unrelated-to-the-PR failure above for pwn.run_in_new_terminal("touch /tmp/a'b")

I'm also open to cleaner ways of detecting if the user's shell is zsh

@zachriggle
Copy link
Member

zachriggle commented Jun 30, 2016 •
edited
Loading

Wouldn't it be cleaner to always use /bin/sh directly instead of $SHELL? Are there any down-sides?

Separately, it's probably a good plan to discontinue the use of run_in_new_terminal to permit multiple commands via semicolon-separation. This avoids the issue entirely.

zachriggle
zachriggle reviewed Jun 30, 2016
View reviewed changes
pwnlib/util/misc.py
if 'zsh' in os.environ['SHELL'] and ';' in command:
# zsh and tmux have issues with handling SIGINT
# wrapping command with sh -c (and escaping single quotes) works around this
command = "sh -c '" + command.replace("\\", "\\\\").replace("'", "'\\\\\\''") + "'"
Copy link
Member

Choose a reason for hiding this comment

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

We have sh_escape for exactly this reason

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I'll take a look

@justinsteven
Copy link
Contributor Author

justinsteven commented Jun 30, 2016 •
edited
Loading

@zachriggle said:

Wouldn't it be cleaner to always use /bin/sh directly instead of $SHELL? Are there any down-sides?

If you mean "avoid doing $SHELL in the first place, always use /bin/sh" then It's not up to us. tmux, in its implementation of splitw, is throwing $SHELL (only when the commands to be splitw'd have a ;). Hence I threw a sh in so that tmux's invocation of $SHELL invokes sh which invokes our ;-split commands.

If you mean "always go down the path of wrapping splitw'd commands with sh" then I agree, it's something to consider. The downside is that cases which don't need the wrapping (i.e. people who don't use zsh, and/or cases where we're not run_in_new_terminal()'ing multiple commands split with ;) then you have a useless invocation of sh.

Separately, it's probably a good plan to discontinue the use of run_in_new_terminal to permit multiple commands via semicolon-separation. This avoids the issue entirely.

Probably a good idea. This is the tack I took in #628 but it ended up feeling pretty hacky, and when I found out the issue only bites (as far as I've seen) zsh+tmux users, I thought the fix could be better targeted. I'd support having run_in_new_terminal(), and by extension tmux splitw, not be used to run ;-separated commands.

@justinsteven
Copy link
Contributor Author

All the tests I cooked up still pass except for:

>>> import pwn
>>> pwn.context.log_level = 'debug'
>>> cmd = "touch /tmp/a\\'bc; id"
>>> print cmd
touch /tmp/a\'bc; id
>>> pwn.run_in_new_terminal(cmd)
[DEBUG] Launching a new terminal: ['/usr/bin/tmux', 'splitw', 'sh -c "touch /tmp/a\\\\\'bc; id"']

/tmp/a'bc gets touched, not /tmp/a\'bc

I was probably too ambitious with my escaping of \. Before this patch (and after this patch, but in the non-; code path) run_in_new_terminal passes \ through to sh without escaping. My bad.

@TethysSvensson TethysSvensson added this to the 3.1.0 milestone Aug 18, 2016
@TethysSvensson
Copy link
Contributor

Could you reopen this PR against the dev branch?

@TethysSvensson
Copy link
Contributor

Or perhaps against the stable branch? Is it a bugfix or a feature?

@justinsteven justinsteven mentioned this pull request Aug 24, 2016
Closed
zachriggle added a commit that referenced this pull request Aug 24, 2016
@zachriggle
Disallow usage of semicolons in run_in_new_terminal.
ea94ee4 
Fixes #690
Fixes #635
Kyle-Kyle pushed a commit to Kyle-Kyle/pwntools that referenced this pull request Apr 25, 2021
@mzr @disconnect3d
Invoke heap commands only with libc debug symbols. (Gallopsled#635)
3408e99 
* * Invoke heap command only with libc debug symbols.

* changed message
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
3.1.0
Development

Successfully merging this pull request may close these issues.
3 participants
@justinsteven @zachriggle @TethysSvensson