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

Fix reverse_tcp listener's handling of disconnects before staging #6499

Merged
merged 3 commits into from Jan 23, 2016

Conversation

bcook-r7
Copy link
Contributor

If we get a tcp disconnect before we finish accepting the socket, the reverse_tcp handler thread breaks out of its internal loop, and we end up with a zombie handler that accepts at the TCP level, but does not actually work any more. Fix #6497

This PR changes the behavior to continue instead. While I was poking around, I also included a commit to tidy up some of the odd ruby style.

Verification

  • Reverse_tcp payloads stage and continue working as expected
  • start a reverse_tcp handler:
./msfconsole -qx 'use exploit/multi/handler; set payload windows/meterpreter/reverse_tcp; set lhost 192.168.56.1; run -j -z'
  • scan the heck out of it
while true; nmap 192.168.56.1 -p 4444; done
  • verify that you can stage a new payload after (or during) the above abuse

@OJ
Copy link
Contributor

OJ commented Jan 22, 2016

I'll take this one on today as I have a case where I can repro.

@OJ OJ self-assigned this Jan 22, 2016
@OJ OJ added the library label Jan 22, 2016
@OJ
Copy link
Contributor

OJ commented Jan 23, 2016

Confirmed breakage on master using @bcook-r7's testing technique as well as in $PRODUCTION. Testing fix now.

@OJ
Copy link
Contributor

OJ commented Jan 23, 2016

From my testing on my local machine, and in $PRODUCTION, this fix doesn't appear to resolve the issue. I'm still unable to connect (both after the intenstive scan or during). I have to restart the handler to get anything to function again.

@busterb
Copy link
Member

busterb commented Jan 23, 2016

Confirmed - it happened to survive on my Mac earlier (YMMV), but broke on Linux at home too.

@OJ
Copy link
Contributor

OJ commented Jan 23, 2016

noooooo

@busterb
Copy link
Member

busterb commented Jan 23, 2016

You're not my dad!

Looks like we shoot ourselves twice - updated with a more robust (and hopefully less noisy on the logs as well) version.

@OJ
Copy link
Contributor

OJ commented Jan 23, 2016

Testing!

@OJ
Copy link
Contributor

OJ commented Jan 23, 2016

MSF be like...
image

You be like...
who da man

Everyone be like...
yaaaaay

Much better. Awesome work mate.

@busterb
Copy link
Member

busterb commented Jan 23, 2016

@OJ OJ merged commit a587975 into rapid7:master Jan 23, 2016
OJ added a commit that referenced this pull request Jan 23, 2016
@OJ
Copy link
Contributor

OJ commented Jan 23, 2016

Wow, you're quick @zeroSteiner, beat me to the label removal!

Thanks for the awesome work Brent. Looking good now.

@busterb
Copy link
Member

busterb commented Jan 23, 2016

For the label change, I think we have a web hook to remove those tags automatically; not sure why or how zeroSteiner's avatar shows up there, but I don't think it's correct. I put in a support query to find out.

It's part of a trial integration we're testing out to help manage and sort PRs and issues better: https://waffle.io/rapid7/metasploit-framework

@OJ
Copy link
Contributor

OJ commented Jan 23, 2016

Ah! Waffle.. yup. Nice.

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

Successfully merging this pull request may close these issues.

None yet

4 participants