-
Notifications
You must be signed in to change notification settings - Fork 14k
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 process migration on reverse_tcp meterpreter sessions w/ newer Ruby #12502
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Feel a bit gross about MSF-related changes to fix something that resulted from a Ruby change, but I think that this makes sense regardless of what Ruby is doing behind the scenes.
@bwatters-r7 done some testing at this end and from what I can see it looks good. I can land it unless you're doing more testing across the board? |
@OJ, I did some manual testing and it looks great. I just plugged it into the automated testing I used to track it down and was going to land as soon as that finished in a couple minutes. |
@bwatters-r7 ok mate, I'll leave it to you then and I'll head to bed :) |
@OJ, WIll-do. Sleep well. |
…s w/ newer Ruby Merge branch 'land-12502' into upstream-master
…s w/ newer Ruby Merge branch 'land-12502' into upstream-master
Release NotesPreviously, metasploit framework's code made incorrect assumptions on what elements in a thread hand-off were synchronous. When we updated to Ruby updated to 2.5.5, the new version included protections to threads that unmasked these assumptions and caused thread handoffs to fail because they no longer behaved in a synchronous fashion. Specifically, this was seen in the |
This fixes #12390, a bug where Meterpreter process migration with reverse_tcp would hang on newer versions of Ruby (since Ruby 2.5.5). This appears to be due to an assumption in the MSF code that killing a thread is a synchronous operation, when it needs a .join to ensure the thread has exited before continuing. Ruby 2.5.5 may have introduced stronger guarantees that the system would continue with the context of the main thread for a bit longer than before after calling '.kill' on a child thread.
Earlier iterations of this patch sprinked ::Thread.pass around for a similar effect, which likely just bought enough time for the child threads to exit on their own. This makes it explicit.
There is opportunity to hunt for more bugs of this class with
git grep "thread = nil"
Verification
List the steps needed to make sure this thing works
./msfconsole -qx 'use multi/handler; set payload windows/meterpreter/reverse_tcp; set lhost 192.168.56.1; run'
or get a reverse_tcp session somehowmigrate -N OneDrive.exe
or whatever your preference is