nil packet triggers an exception in Rex::Post::Meterpreter (packet.rb:323)
#10177
Comments
|
Gathering potentially-related comments on potentially-related issues: #9356 (
|
|
Is there a workaround to stop the bleeding here? My screen is all backtrace. Downgrade ruby or rex-socket? Anything? Just to clarify what I'm experiencing - anything over a session route has a 30-40% chance of causing an error/backtrace. |
|
So the line of code that is bailing is in this function: def to_r
# Forcibly convert to ASCII-8BIT encoding
raw = value.to_s.unpack("C*").pack("C*")
if (self.type & TLV_META_TYPE_STRING == TLV_META_TYPE_STRING)
raw += "\x00"
elsif (self.type & TLV_META_TYPE_UINT == TLV_META_TYPE_UINT)
raw = [value].pack("N") <----------- this one here.
elsif (self.type & TLV_META_TYPE_QWORD == TLV_META_TYPE_QWORD)
raw = [ self.htonq( value.to_i ) ].pack("Q<")
elsif (self.type & TLV_META_TYPE_BOOL == TLV_META_TYPE_BOOL)
if (value == true)
raw = [1].pack("c")
else
raw = [0].pack("c")
end
end
. . . . . . < snip > ....
[raw.length + HEADER_SIZE, self.type].pack("NN") + raw
endTwo things that come to mind are:
This version of |
|
I actually tried returning nil (and fixing up caller) and it seemed to work at first, but not really. Returning nil maybe doesn't send the packet but framework still waits for a response as if it did? |
|
Looks like its expecting a uint... So I don't think nil/"" will work. Iirc I tried walking the callers at some point, but its cross thread so I lost the chain. |
|
@OJ: if the framework end is at that line, its partially through a tlv parse, right? Meaning the sending end encoded a structure which, on deserialization, appears to have a type of uint and yet a value of nil... Alternatively, preceding steps in the framework pipeline in the recv stack are somehow truncating the packet/tlv. Right? |
|
It's trying a pack a |
|
Not to sound like an ungrateful jerk, but I'm failing to understand why this isn't a bigger issue. Routes are essentially unusable at the moment unless you're willing to put up with fullscreen backtraces every 2 seconds. Is using routes just not that popular anymore? I can't figure out why a hundred other people aren't posting identical issue reports. If anyone needs a reminder of just how awful this is, run smb_login with a userlist over a session route. At this point I'd be thrilled to be shown I'm wrong or I'm using routes "the wrong way", but I don't see it. What the hell is going on here? |
|
Has anyone tried to git bisect it? I haven't tried reproducing it yet, but can take a look. |
|
Grabbing since I can now reproduce it. Took a little while due to various firewalls and other nannies on this machine causing things to fail too gracefully at the OS layer. |
|
On python meterpreter, we see this series of events running smb_login. The failure happens in close, which is at the end of the TCP connection, so it produces more noise than problem during the connection. [] core_channel_open dispatching to handler: channel_open_stdapi_net_tcp_client |
|
The first commit where this bug occurs is in the transition to Ruby 2.5. Likely reasons why the hordes have not descended: our packages are still based on earlier versions of Ruby, you can still use any version of Ruby you want to 2.3. I'm going to try 2.4.4 on master and see what happens next. |
|
Works fine with Ruby 2.4.4: What does it mean? We're either depending on undefined behavior, there's an exception being swallowed in 2.4 that's different in 2.5, or 2.5 has a regression. It'd be nice to revert for now, but Kali Linux and other security distributions moved on to 2.5 globally for all Ruby tools around the time of the commit above, which is what dragged us along for the ride. Since they do their own packaging, and always use system packages, it's hard to revert, but it could be possible. Still, we should probably figure out what causes this to happen in Ruby 2.5 only. |
|
For reference, the backtrace in master at the time of posting is: |
|
It appears that the channel ID in the close message is Nil or sometimes '1' when I run this test with a clean setup. In the '1' case, it is still an error, because meterpreter says there is no such channel ID: |
|
Gross, look what I found lurking in the channel code, a finalizer! This does a close not when the user asks, but whenever the garbage collector happens to run. That's quite Ruby-version dependent in behavior. This already caused a bunch of crashes and problems in the registry code, let's see if it affects this too. |
|
When the TCP channel gets closed here, it appears to happen at least 3 separate times. There's a lot of non-threadsafe code here has TOCTOU problems. This pattern: Pretty much ensures that eventually Ruby would be fast enough that 2 threads would be in that inner condition at the same time, and someone would send a packet with a nil cid. |
|
Another issue is at least python meterpreter, if the remote side closes a TCP connection, will delete the channel from its internal table via |
|
And, after tweaking python meterpreter to accept closing channels that already self-closed, and metasploit to only try closing once, got it to work with Ruby 2.5.1 as well! |
Don't send a close message for a nil channel ID, and if we do send a close message, only do it once. I could have added a mutex somewher in _close(), but because it's a class method, it's a little awkward and would require all of the callers to instead have voluntary lock. As an alternative, I just made the finalizer close the channel instead. Fixes rapid7#10177
|
@rwhitcroft any comments on the referenced patch? Thanks! |
|
No sir, just thanks! |
|
Sure thing. I have a handful of other patches that I'll get up that fix other finalizer-related bugs as well, since this antipattern got repeated on a number of Meterpreter objects. |
Steps to reproduce
There are two cases in which a closing thread causes Meterpreter to send an empty packet, presumably because the module fails during an exception and the thread is forcibly shutdown.
Case # 1:
auxiliary/server/socks5is unable to connect to the requested destination.Case # 2:
auxiliary/scanner/mysql/mysql_versionfails while scanning a MySQL server.Case # 3: #10160 - SMB fails to connect, likely due to a lack of SMB2 support.
In all cases, the traceback doesn't provide an explanation as to why the module failed.
Expected behavior
An empty packet should not be sent, the thread should close without a traceback, and an informative error message should be provided to the user.
Current behavior
System stuff
Metasploit version
This behavior was observed with
mysql_versionon4.16.48-dev, and withsocks5on the latest master branch.I installed Metasploit with:
OS
This behavior was observed under Kali 2017.3 and Mac OS X 10.13.4.
The text was updated successfully, but these errors were encountered: