Skip to content

Allow true zero-second timeout in send_request_*#12007

Merged
wvu merged 3 commits intorapid7:masterfrom
wvu:feature/http
Jun 28, 2019
Merged

Allow true zero-second timeout in send_request_*#12007
wvu merged 3 commits intorapid7:masterfrom
wvu:feature/http

Conversation

@wvu
Copy link
Copy Markdown
Contributor

@wvu wvu commented Jun 24, 2019 •
edited
Loading

⚠️ 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

@wvu
Allow true zero-second timeout in send_request_*
b49fa29 
Also fixes a bogus response when timeout is nil.
jrobles-r7
jrobles-r7 approved these changes Jun 24, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@jrobles-r7 jrobles-r7 left a comment

Choose a reason for hiding this comment

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

LGTM

@wvu wvu added the blocked Blocked by one or more additional tasks label Jun 24, 2019
@wvu wvu mentioned this pull request Jun 25, 2019
Merged
@wvu wvu removed the blocked Blocked by one or more additional tasks label Jun 25, 2019
@wvu
Copy link
Copy Markdown
Contributor Author

wvu 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 wvu merged commit 7739574 into rapid7:master Jun 28, 2019
wvu added a commit that referenced this pull request Jun 28, 2019
@wvu
Land #12007, true 0s timeout in send_request_*
0a00f38
msjenkins-r7 pushed a commit that referenced this pull request Jun 28, 2019
@wvu @msjenkins-r7
Land #12007, true 0s timeout in send_request_*
cbaa872
@wvu
Copy link
Copy Markdown
Contributor Author

wvu commented Jun 28, 2019 •
edited by tdoan-r7
Loading

Release Notes

This fix 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 the 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 wvu deleted the feature/http branch June 28, 2019 17:43
@tdoan-r7 tdoan-r7 added the rn-fix release notes fix label Jul 23, 2019
@wvu wvu mentioned this pull request Oct 7, 2019
Merged
3 tasks
@wvu wvu mentioned this pull request Jul 28, 2020
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@jrobles-r7 jrobles-r7 jrobles-r7 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

bug enhancement library rn-fix release notes fix

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

send_request_cgi timeout delays the session creation when used to deliver the payload

3 participants

@wvu @jrobles-r7 @tdoan-r7