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

[BUG] C-d from remote prompt after connection closed causes uncaught exception #133

Closed
calebstewart opened this issue Jun 16, 2021 · 6 comments · Fixed by #138
Closed

[BUG] C-d from remote prompt after connection closed causes uncaught exception #133

calebstewart opened this issue Jun 16, 2021 · 6 comments · Fixed by #138
Labels
bug Something isn't working

Comments

@calebstewart
Copy link
Owner

Describe the bug

If you are connected to a victim, and the remote shell is killed, if you press C-d to exit the remote shell, an uncaught ChannelError is raised. The routine in Manager.interactive needs to be modified to catch these exceptions.

In most places, exceptions are caught, but there was some wiggle-room left during transition between states, which causes this problem.

Describe the target system

Any target will trigger this error if the remote shell/process is killed and the next input from the attacker is C-d.

Steps To Reproduce

Steps to reproduce the behavior:

  1. On victim: socat TCP-LISTEN:4444,reuseaddr,fork EXEC:/bin/bash
  2. On Attacker: pwncat W.X.Y.Z 4444
  3. On Attacker: Press C-d to go to remote prompt
  4. On victim: Press C-c to kill socat
  5. On Attacker: Press C-d to return to pwncat prompt

This will cause the uncaught ChannelError, and exit pwncat completely. It's worth noting that if any other keys are pressed besides C-d, pwncat will recognize the channel has closed and continue correctly.

Expected behavior

Exception is caught just as if you pressed any other key.

Screenshots

image

Expected Solution

The problem is at lines 593-612 of manager.py:

            try:
                self.target.platform.interactive_loop(interactive_complete)
            except RawModeExit:
                pass
            except ChannelClosed:
                channel_closed = True
                self.log(
                    f"[yellow]warning[/yellow]: {self.target.platform}: connection reset"
                )
            except Exception:
                pwncat.util.console.print_exception()

            # Trigger thread to exit
            interactive_complete.set()
            output_thread.join()

            # Exit interactive mode
            if channel_closed:
                self.target.died()
            else:
                self.target.platform.interactive = False

The Exit interactive mode block should be within the try-block. Something like this:

            try:
                try:
                    self.target.platform.interactive_loop(interactive_complete)
                except RawModeExit:
                    pass
                self.target.platform.interactive = False
            except ChannelClosed:
                self.target.died()
                self.log(
                    f"[yellow]warning[/yellow]: {self.target.platform}: connection reset"
                )
            except Exception:
                pwncat.util.console.print_exception()

            # Trigger thread to exit
            interactive_complete.set()
            output_thread.join()
@calebstewart calebstewart added the bug Something isn't working label Jun 16, 2021
@calebstewart
Copy link
Owner Author

Thanks to @Mitul16 for pointing this out over in PR #132.

@Mitul16
Copy link
Contributor

Mitul16 commented Jun 17, 2021

Firstly, I got into some other work causing delay in your tests.
Secondly, I have tested this branch and it seems to have fixed the discussed issues.
But, with a little bit of digging I found somewhat similar results (getting kicked out from my pwncat instance)

If you want to use it, here is the error.log for the output shown in the images below.

I should have created another Issue, instead I am presenting here since it is a fairly similar issue if not the same

Steps to reproduce

  1. On victim machine: Start a bind shell listener - socat tcp-l:PORT,fork exec:/bin/sh
  2. On attacker machine: Connect to the bind shell - pc connect://IP:PORT
  3. On victim machine: Kill the connection - Ctrl-C to kill the process
  4. On attacker machine: Run an enumeration - run enumerate.file.suid
  5. On attacker machine: Wait for ChannelTimeout to occur (it occurs after the set 30 seconds timeout) - ...
  6. On attacker machine: Observe that the invalidated session still exists - sessions
  7. On attacker machine: Try to interact with this invalidated session - sessions 0
  8. On attacker machine: Use Ctrl-D to return to the remote terminal (it looks like this edit was not accepted by GitHub)
  9. On attacker machine: See the error - pwncat instance exits out

Screenshots

Get a session and kill it from the other side

Screenshot from 2021-06-17 16-27-09

Screenshot from 2021-06-17 16-27-18


Interact with the invalidated session and return to the remote terminal

Screenshot from 2021-06-17 16-27-39

Screenshot from 2021-06-17 16-27-46

@Mitul16
Copy link
Contributor

Mitul16 commented Jun 17, 2021

After a connection error, we should close the respective session if it is not closed and prevent the user from interacting with it.

@calebstewart
Copy link
Owner Author

I pushed an update. There was some incorrect handling of the results of socket.recv in pwncat.channel.socket.Socket.recv. It now raises the appropriate ChannelClosed exception, and in my tests closed the channel properly.

image

@calebstewart
Copy link
Owner Author

This should also prevent the obnoxiously long 30-second timeout when a connection drops, which is something that I couldn't figure out how to fix before, so I'm happy it's gone. 🤣

@calebstewart
Copy link
Owner Author

calebstewart commented Jun 17, 2021

Just did some more testing. There is still the other error you mentioned when trying to go back to the remote prompt after the shell dies. I'll work on that now.

Edit

Just pushed another commit that should take care of the context switch into the remote shell when a channel has disconnected prematurely.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants