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

Make interactive() exit when recv side disconnects #2108

Draft
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

gsingh93
Copy link
Contributor

This needs more testing and discussion, but I wanted to create a PR to at least see if this approach would work or whether this needs to be done in a completely different way.

This PR addresses #2106. I want tubes.interactive() to break out of its loop when it receives an EOF from the remote side. Currently if this happens, the send side will not break because it's blocked waiting for user input, and only after pressing enter does it try to send to the remote and see that it's disconnected.

My solution is to:

  1. Set go.set() in the receiving thread so the sending side knows that the receiving side disconnected.
  2. Instead of waiting indefinitely for a key press, wait for a timeout of 1 second, and then only report a sender EOF if the bytes read is 0 and not None (which would have meant a timeout occured). If there's no EOF, the loop continues to iterate and poll for input every second.

This works locally, and while it seems like there are some test cases I need to fix, I wanted to check if this general approach is OK before I put more time into it, or if there are any obvious problems with it that can't be easily fixed or if there's an overall better approach to this.

@disconnect3d
Copy link
Contributor

I haven't really looked at this problem at all, but can't we do something like select(...) on both network and stdin and either handle the network or stdin event?

You can definetely detect disconnects with select(), see https://stackoverflow.com/a/17818000/1508881

@gsingh93
Copy link
Contributor Author

Thanks! I originally didn't do that because it seemed complex, but I got something working. There's more code changes and it's a bit confusing, but it avoids modifying any of the internals of readline.

@gsingh93
Copy link
Contributor Author

Although I will say, even though I'm catching the exception that's thrown when the process is stopped, this is still output from the logging:

[ERROR] A stopped process does not have a file number

@gogo2464

This comment has been minimized.

@Arusekk
Copy link
Member

Arusekk commented Jul 9, 2023

One thing I believe should be considered is an option to keep current behaviour: now if the recv side disconnects, you can still send 'blind' TCP packets to it; I remember this was useful in a couple CTF challenges.

@peace-maker
Copy link
Member

@gsingh93 Do you remember what was keeping this in a draft state? I think a parameter on interactive() to keep the sending end open is useful. But have the new behavior as a default to exit when the recv side goes away.

The ssh_channel.interactive() implementation would need this too I think. Maybe we're able to merge the two? Just thinking for the long run, not this PR.

@peace-maker
Copy link
Member

I've rebased this on latest dev and tested a bit. interactive did return immediately when exiting a local process, but did not for a remote socket:

from pwn import *
#io = process('/bin/sh')
l = listen()
l.spawn_process('/bin/sh')
io = remote('127.0.0.1', l.lport)
io.interactive()

select printed the [ERROR] A stopped process does not have a file number error when trying to access the fileno.

def fileno(self):
if not self.connected():
self.error("A stopped process does not have a file number")
return self.proc.stdout.fileno()

That message could be removed to avoid the spam.

Maybe the old solution of doing short reads and checking if the receiving side disconnected should be reconsidered. You can only `select´ on sockets on Windows, so this isn't generally portable. I guess we can use select on posix and the sleeps on windows, but that would complicate the code further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants