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

TLV traffic obfuscation with 4-non-null-byte XOR #6480

Merged
merged 9 commits into from Feb 11, 2016

Conversation

OJ
Copy link
Contributor

@OJ OJ commented Jan 17, 2016

This is the MSF side of the Meterpreter PR that can be found here: rapid7/metasploit-payloads#64

Details of the PR and its intent can be found in the Payloads repo. May I request that we discuss the bulk of it on the payloads side, and only discuss the MSF-side of the PR here.

Thanks!

raw = super
xor_key = rand(254) + 1
xor_key |= (rand(254) + 1) << 8
xor_key |= (rand(255) + 1) << 16
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be 254, like the above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

derp! :)

expect(parser.send(:raw)).to eq ""
expect(parser.send(:hdr_length_left)).to eq 8
expect(parser.send(:payload_length_left)).to eq 0
parser.send(:raw).to eq ""
Copy link
Contributor

Choose a reason for hiding this comment

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

We will need to keep the expects here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like a bad merge. Thanks for picking this up!

@OJ
Copy link
Contributor Author

OJ commented Feb 5, 2016

OK, this should be good to go now! We've got all the payloads wired up. Thanks to all who helped! @sempervictus @bcook-r7 @timwr @zeroSteiner

@OJ
Copy link
Contributor Author

OJ commented Feb 11, 2016

Pretty please can I get a bump on this? Working off the branch on gigs is annoying :D Thank you and much ❤️

@bcook-r7 bcook-r7 self-assigned this Feb 11, 2016
@bcook-r7
Copy link
Contributor

ok, let's do it

@bcook-r7 bcook-r7 merged commit 4ac7c5e into rapid7:master Feb 11, 2016
bcook-r7 pushed a commit that referenced this pull request Feb 11, 2016
@bcook-r7
Copy link
Contributor

done and done - thanks ever so much @OJ and crew!

@OJ
Copy link
Contributor Author

OJ commented Feb 11, 2016 via email

@OJ OJ deleted the default-xor branch February 11, 2016 08:44
# the serialized TLV content, and then returns the key plus the
# scrambled data as the payload.
#
def to_r
Copy link
Contributor

Choose a reason for hiding this comment

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

Mainly a style nitpick, but this could be cleaner:

xor_key = rand(0x100000000)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That doesn't guarantee non-null bytes in all 4 positions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would something like xor_key = rand(0xffffffff) +1 be better?

Copy link
Contributor Author

@OJ OJ Feb 15, 2016 via email

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Is avoiding any single byte good or bad for heuristic detection? Would the chance of the first byte never being an 'R' be more of a tell than it being an 'R' in rare cases?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that as long as not all of the bytes are 0, nothing else really makes a difference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I may just be overly paranoid, but I felt like it was a good thing to guarantee that every byte changed. That's really the only reasoning.

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

Successfully merging this pull request may close these issues.

None yet

5 participants