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

Linux reverse_tcp should read known # of bytes #12271

Merged

Conversation

sempervictus
Copy link
Contributor

The linux x64 reverse tcp stager is hardcoded to read 4K off the
socket. When a small intermediate stager is used, this can result
in reading part of the next stage as well, which means that the
intermediate stager will never recv the # of bytes it needs and
hang indefinitely.

Break out the mettle piece to use separate methods for assembly and
binary payload generation as well as actually putting the product
on the existing session socket.

Change the first part of the stage to check for the intermediate
stager generation method, and use the size of the produced stager
in the recvfrom call or fall back to the prior 4K read size.

Testing:
None yet

Ping @bcook-r7, @acammack-r7, @OJ, @zeroSteiner

@bcook-r7
Copy link
Contributor

bcook-r7 commented Sep 3, 2019

Note, the same problem is in other stagers, e.g. x86 also does a blind read.

The linux x64 reverse tcp stager is hardcoded to read 4K off the
socket. When a small intermediate stager is used, this can result
in reading part of the next stage as well, which means that the
intermediate stager will never recv the # of bytes it needs and
hang indefinitely.

Break out the mettle piece to use separate methods for assembly and
binary payload generation as well as actually putting the product
on the existing session socket.

Change the first part of the stage to check for the intermediate
stager generation method, and use the size of the produced stager
in the recvfrom call or fall back to the prior 4K read size.

Testing:
  None yet

Ping @bcook-r7, @acammack-r7, @OJ, @zeroSteiner
See notes for x64.

This part does not appear to be working properly yet - stages
generated with this commit recv 102b on the first call to read(),
but subsequently things seem to go off the rails after the
intermediate stage is loaded.

Needs testing and fixup at present for x86 (no worse than before
in terms of success rate however).
@timwr
Copy link
Contributor

timwr commented Sep 3, 2019

Surely the payload is now just reading a hardcoded 126 bytes instead of 4096. What if we update the stager in the future so that it's more than 126 bytes? Any previously generated payload will only read 126 bytes and fail. Perhaps it might be better to first read the length and then mmap/read that number of bytes. We do this on aarch64 (and java):

@timwr
Copy link
Contributor

timwr commented Sep 3, 2019

Is the current behaviour causing issues? There was a bug on OSX that occurred occasionally but it was fixed with a rex.sleep: #11165 potentially we could fix them both at once to use a length (and metasm).

@sempervictus
Copy link
Contributor Author

sempervictus commented Sep 3, 2019

Current behavior is broken. The dynamic read size should be a payload option IMO, and your midstager would have to differ for this to break, actual stage doesn't matter.
Also all for dynamic read sizing as an option...

Copy link
Contributor

@acammack-r7 acammack-r7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I prefer this to the timing-based approaches we've used in the past to separate the intermediate stager from the stage on the wire. There are some asm hygiene items to fix, though.

lib/msf/core/payload/linux/reverse_tcp.rb Outdated Show resolved Hide resolved
lib/msf/core/payload/linux/x64/reverse_tcp.rb Show resolved Hide resolved
Address Adam's comments on the PR - remove redundantly pushed
size from mmap section.
@sempervictus sempervictus changed the title Linux x64 reverse_tcp should read known # of bytes Linux reverse_tcp should read known # of bytes Sep 3, 2019
Push read_size to edx as suggested by Adam, optimize shellcode a
bit by selecting using dx instead of edx for sizes under 64K.

Testing:
  Internal only, creates session on every try instead of every 5th.
@acammack-r7
Copy link
Contributor

@bwatters-r7 Will also be interested, I think.

For real this time?
Copy link
Contributor Author

@sempervictus sempervictus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you. x86 likely as well since we are moving the size param to edx

@acammack-r7
Copy link
Contributor

Not in the default case. When it's not a meterpreter stager it acts just as it did before, it is just a bit more explicit about what it's doing. Also, when the intermediate stage is short (like it is now), the size fits into a single byte, so the stage still doesn't grow.

@acammack-r7
Copy link
Contributor

The x64 stager looks to be oscillating in size, I'll dig into it.

Since these stagers can shrink based on the expected size of the next
stage, do our best to anticipate a small size. This makes the cached
payload size consistent for now, though if the x64 mettle stager grows
past 128 bytes I think we'll see the stager start oscillating in size
again. If you run into that and are reading this, sorry :(
@acammack-r7 acammack-r7 merged commit 2ee5ec9 into rapid7:master Sep 5, 2019
@acammack-r7
Copy link
Contributor

acammack-r7 commented Sep 5, 2019

Release Notes

Previously on Linux, the x86 and x64 reverse TCP stagers would often read past the end of Meterpreter's intermediate stager and grab the first few bytes of the final Meterpreter payload. Both stagers now only read the expected number of bytes as a hot fix. This makes them more reliable pending reworking them to read the size of the next stage off the wire as done on other platforms.

@tdoan-r7 tdoan-r7 added the rn-fix release notes fix label Sep 9, 2019
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug payload rn-fix release notes fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants