Fix EUID handling in Popen and Shell Detection

calebstewart commented 3 years ago

Description of Changes

As discussed in #179, the handling of euid processes during Popen with shell=True and during shell detection causes uncaught exceptions or dropped permissions.

In the former case, I simply removed the handling of shell=True from Linux.Popen. The way that pwncat executes processes without /bin/sh -c is equivalent to running with /bin/sh -c, so I don't believe this will cause any issues, but needs some more testing before merging.

In the latter case, I simply caught the OSError raised by Path.readlink() during shell detection. In this case, we fall back to using the SHELL environment variable which is unreliable at best. This means that normally, if you are in a euid context, you will lose fancy shell highlighting, but this is mainly cosmetic and doing this ensures pwncat doesn't crash and drop your session.

I don't believe this fully fixes the aforementioned issue. To fully fix that problem, I think implementing a module for correcting euid situations after escalation is ideal. In reality, you don't ever want to be in a euid situation because permissions are... weird... and many utilities will drop your permissions unexpectedly. We can normally rectify this, so I don't want to close that issue until a module that resolves this is implemented.

Please note any noqa: comments needed to appease flake8.

Major Changes Implemented:

Removed handling of shell argument to Linux.Popen
Added OSError handler in shell detection routine.

Pre-Merge Tasks

[x] Formatted all modified files w/ python-black
[x] Sorted imports for modified files w/ isort
[x] Ran flake8 on repo, and fixed any new problems w/ modified files
[x] Ran pytest test cases
[x] Added brief summary of updates to CHANGELOG (under [Unreleased])

For issues with pre-merge tasks, see CONTRIBUTING.md

calebstewart commented 3 years ago

I added a call to refresh_uid() after exiting interactive mode. This was part of the discussion in the closed #180 pull request.

calebstewart commented 2 years ago

It's been a while since creating this. I'm not sure why I never merged it, but I read the the relevant content again, and re-ran the tests just to make sure everything was solid, and it looks fine to me. I'm going to merge now.

calebstewart / pwncat