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

Make auto_cl more selective based on HTTP method #11945

Merged
merged 1 commit into from Jun 4, 2019

Conversation

Projects
None yet
3 participants
@busterb
Copy link
Member

commented Jun 4, 2019

According to https://tools.ietf.org/html/rfc7230#section-3.3.2, a zero content-length is valid for some kinds of HTTP methods.

Instead of implicitly disabling auto_cl if there is no actual content, only disable auto_cl default for HTTP methods where semantics of the message do not anticipate any content. This can still be overridden by a caller if it still wants to add an empty content-length for HTTP methods where it does not normally make sense (e.g. if it exploits a bug.)

Based on comments from #11937. @sempervictus does this look more in line with what you're thinking? There is not a way in packet.rb to disable auto_cl based on method because that is only defined in the child class.

Update: Looks like the first required use of 0-byte Content-Length required is in Meterpreter itself. This patch also fixes a regression in reverse_http/s transports which use it.

Verification steps

  • Establish a Windows Meterpreter reverse_http/s session
  • Verify that the session stages correctly and is interactive
Make auto_cl more selective based on HTTP method
According to https://tools.ietf.org/html/rfc7230#section-3.3.2, a zero content-length is valid for some kinds of HTTP methods.

Instead of implicitly disabling auto_cl if there is no actual content, disable auto_cl default for HTTP methods where semantics of the message do not anticipate any content. This can still be overridden by a caller if it still wants to add an empty content-length for HTTP methods where it does not normally make sense (e.g. if it exploits a bug.)
@busterb

This comment has been minimized.

Copy link
Member Author

commented Jun 4, 2019

Note: if the sanity test above isn't using reverse_http/s already, it would be nice if it did!

@bwatters-r7

This comment has been minimized.

Copy link
Contributor

commented Jun 4, 2019

Sanity test runs:
windows/x64/meterpreter/bind_tcp
windows/meterpreter/reverse_tcp
windows/x64/meterpreter_bind_tcp
windows/meterpreter_reverse_tcp

@bwatters-r7

This comment has been minimized.

Copy link
Contributor

commented Jun 4, 2019

Hard to argue with this much green....

windows/meterpreter/reverse_http
image

windows/meterpreter/reverse_https
image

image

@bwatters-r7 bwatters-r7 self-assigned this Jun 4, 2019

@bwatters-r7 bwatters-r7 merged commit e5a4c2d into rapid7:master Jun 4, 2019

3 checks passed

Metasploit Automation - Sanity Test Execution Successfully completed all tests.
Details
Metasploit Automation - Test Execution Successfully completed all tests.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

bwatters-r7 added a commit that referenced this pull request Jun 4, 2019

Land #11945, Make auto_cl more selective based on HTTP method
Merge branch 'land-11945' into upstream-master
@bwatters-r7

This comment has been minimized.

Copy link
Contributor

commented Jun 4, 2019

Release Notes

This fix resolves an issue in HTTP payloads that overaggressively disabled auto_cl.

msjenkins-r7 added a commit that referenced this pull request Jun 4, 2019

Land #11945, Make auto_cl more selective based on HTTP method
Merge branch 'land-11945' into upstream-master
@busterb

This comment has been minimized.

Copy link
Member Author

commented Jun 4, 2019

Thanks @bwatters-r7

@busterb busterb added the a2k19 label Jun 4, 2019

@tdoan-r7 tdoan-r7 added the rn-fix label Jun 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.