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

Allow true zero-second timeout in send_request_* #12007

Merged
merged 3 commits into from Jun 28, 2019

Conversation

Projects
None yet
2 participants
@wvu-r7
Copy link
Contributor

commented Jun 24, 2019

⚠️ This PR changes core behavior to align with expected behavior

Also fixes a bogus response when timeout is nil.

  1. Timeout.timeout(0) implies that the block will not time out, which is usually not what we want when we set the timeout parameter to 0 to address a lack of response.
[1] pry(#<Msf::Modules::Exploit__Multi__Http__Issue::MetasploitModule>)> send_request_cgi({}, 0)
^CInterrupt:
from /Users/wvu/.rbenv/versions/2.6.2/lib/ruby/gems/2.6.0/gems/rex-core-0.1.13/lib/rex/io/stream.rb:95:in `select'
[2] pry(#<Msf::Modules::Exploit__Multi__Http__Issue::MetasploitModule>)>

See #2820 for the only known example. Note that Juan had to ^C his module when he wanted to return immediately. So, this actually fixes a bug.

  1. A timeout value of nil is returning an unfilled Rex::Proto::Http::Response, which is a bogus 200 OK that isn't meant to be parsed.
[2] pry(#<Msf::Modules::Exploit__Multi__Http__Issue::MetasploitModule>)> send_request_cgi({}, nil)
=> #<Rex::Proto::Http::Response:0x00007faf42e18e28
 @auto_cl=true,
 @body="",
 @bufq="",
 @chunk_max_size=10,
 @chunk_min_size=1,
 @code=200,
 @count_100=0,
 @headers={},
 @inside_chunk=false,
 @max_data=1048576,
 @message="OK",
 @peerinfo={"addr"=>"127.0.0.1", "port"=>8080},
 @proto="1.1",
 @request="GET / HTTP/1.1\r\nHost: 127.0.0.1:8080\r\nUser-Agent: Mozilla/4.0 (compatible; MSIE 6.0; Windows NT 5.1)\r\nContent-Type: application/x-www-form-urlencoded\r\n\r\n",
 @state=1,
 @transfer_chunked=false>
[3] pry(#<Msf::Modules::Exploit__Multi__Http__Issue::MetasploitModule>)> puts _
HTTP/1.1 200 OK
Content-Length: 0

=> nil
[4] pry(#<Msf::Modules::Exploit__Multi__Http__Issue::MetasploitModule>)>

I did not find any examples of a nil timeout being used. Most people set something low instead. Either way, the response should be nil, never something bogus.

Resolves #12004, in a manner of speaking. cc @cyrus-and

Allow true zero-second timeout in send_request_*
Also fixes a bogus response when timeout is nil.
@jrobles-r7
Copy link
Contributor

left a comment

LGTM

@wvu-r7 wvu-r7 added the delayed label Jun 24, 2019

@wvu-r7 wvu-r7 referenced this pull request Jun 25, 2019

Open

Feature/reverse ssh #12002

@wvu-r7 wvu-r7 removed the delayed label Jun 25, 2019

@wvu-r7

This comment has been minimized.

Copy link
Contributor Author

commented Jun 28, 2019

Since I got a LGTM from @jrobles-r7, this addresses the linked PR, and no else has raised concerns, I'm landing this. :)

@wvu-r7 wvu-r7 merged commit 7739574 into rapid7:master Jun 28, 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

wvu-r7 added a commit that referenced this pull request Jun 28, 2019

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

@wvu-r7

This comment has been minimized.

Copy link
Contributor Author

commented Jun 28, 2019

Release Notes

This changes the behavior of the send_request_cgi and send_request_raw methods in the HttpClient library (technically the underlying code in Rex::Proto::Http::Client) to perform expected behavior when supplied with a zero-second timeout.

Previously, a module using the library in this particular fashion would hang indefinitely. Now, calling either method with such a timeout will return immediately.

@wvu-r7 wvu-r7 deleted the wvu-r7:feature/http branch Jun 28, 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.