-
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
New module to replicate xspy tool (and x11 library) #18877
base: master
Are you sure you want to change the base?
Conversation
I'll give this another check tomorrow, I know some of the reviews have some notes to touch up, but I think its ready for review. I'm going to do an x11 screenshot taker as well, so some of the lib functions aren't used yet, but wanted to get this in framework first before it gets too out of hand. |
@smcintyre-r7 you can go ahead and start reviewing. I know I need to update the open_x11 scanner docs, and a few other minor things. figured this may take a little bit since its so large |
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.
I left quite a few comments, but overall things are looking pretty good. Thanks for this new protocol!
got through some, just an update that I'm working on it, its not abandoned :) |
prob worth a squash when this is ready to merge, no one needs to see 31+ commits where I learn to code gooderer. |
I believe this is ready to look at again. It's undergone a lot of changes, so may want to treat it as a fresh PR. |
I'm inconsistently getting the following stack trace. My target is an Ubuntu 22.04 x64 workstation where I installed socat. It seems like it's easier to reproduce the stack trace after rebooting. If I run the module a couple of times without rebooting the target, it'll work after 1 or 2 failures. EOF Stack Trace
PCap of the transaction when the stack trace occurred. It may be simple enough to handle the EOF and carry on, I'm not entirely sure. I've definitely seen the module work, it's just this crash sometimes. |
was about to ping you @smcintyre-r7 asking you to review, and realized I missed the notice that you already did! I 100% believe this (even w/o evidence pcap), I've seen it every once in a while. My test box is a 22.04. I just rebooted, ran 10 times, vacuumed the office, ran 20 more times, ran it with ctr+c at random points (during setup, while listening), and ran another 20 times. Can't replicate, so I'll try another box, hopefully I can get it more consistently |
@smcintyre-r7 I loaded up your pcap, extracted the last packet, put it into the spec and (after adjusting one checked value), it loaded w/o an EOF. So I've added some error handling so it wont crash, but I'm not sure why the EOF happened in the first place. I'll uploaded some updated code later. I've added your pcap as an additional check as well. |
Tested against 20.04, working.
|
Pushed in the most recent copy I have. I've theorized the problem on why your crashed. It's a network thing, the packet reading isn't getting all the data read in to a variable off the wire. It then tries to bin parse the data, hits EOF because it didn't read it all in. I remember this happening before when I first started this module off, but its been so long I dont remember how I fixed it. At a minimum, now it won't crash, but exit gracefully. |
I looked into this some more and finally found some information about how the This would probably be pretty useful in a mixin, since you can give it the class and read the reply instead of hoping that def read_x11_reply(klass, timeout: 10)
unless klass.fields.field_name?(:response_length)
raise ::ArgumentError, 'X11 class must have the response_length field to be read'
end
remaining = timeout
reply_instance = klass.new
metalength = reply_instance.response_length.num_bytes
buffer, elapsed_time = Rex::Stopwatch.elapsed_time do
sock.read(reply_instance.response_length.abs_offset + metalength, remaining)
end
remaining -= elapsed_time
# see: https://www.x.org/releases/X11R7.7/doc/xproto/x11protocol.html#reply_format
response_length = reply_instance.response_length.read(buffer[-metalength..-1]).value
response_length *= 4 # field is in 4-byte units
response_length += 32 # 32 byte header is not included
while buffer.length < response_length && remaining > 0
chunk, elapsed_time = Rex::Stopwatch.elapsed_time do
sock.read(response_length - buffer.length, remaining)
end
remaining -= elapsed_time
break if chunk.nil?
buffer << chunk
end
unless buffer.length == response_length
if remaining <= 0
raise Rex::TimeoutError, 'X11: failed to read response due to timeout'
end
raise ::EOFError, 'X11: failed to read response'
end
reply_instance.read(buffer)
end With that in place, you can then update the code you originally had with error catching to this and it should work much more reliably. Any time you're trying to read an X11 reply message, this method should really be used to ensure that chunked data is handled correctly. map_data = read_x11_reply(X11GetMapReply) Let me know if you'd like me to PR the changes to your branch. |
Instead of the manual loop in the metasploit module - I believe there's a timed_read function in the rex core API too if that's at all useful, although it uses Or if that's already been considered, it'd be great to understand why it's not a viable method to call in this context 👀 |
It's viable sure, but I think this is better for the following reasons:
While looking through point 3, I noticed it does look like |
Sure! |
Consistently refer to replys as responses
Refactor some X11 code around
I ran this 50 times (5 sec each) vs my test box, didn't have any crashes (although didn't have any/many before). This is a VM running on the same network 1 hop away). I'm happy with these changes, and ready to have this landed! |
This PR implements a rudimentary pure ruby X11 library, and a module to bind to open x11 sessions to perform keylogging. This is a replica of the xspy tool from forever ago.
When this lands I'll circle back around on adding the screenshot capability.
Verification
msfconsole