TIMELINE
rootredrain submitted a report to Ruby.
show raw
Jun 22nd
Hi,
I would like to report a HTTP Header injection vulnerability in
'net/http' that allows attackers to inject arbitrary headers in
request even create a new evil request.
PoC
require 'net/http'
http = Net::HTTP.new('192.168.30.214','80')
res = http.get("/r.php HTTP/1.1\r\nx-injection: memeda")
Example
Server Code:
#!/usr/bin/env ruby
require 'sinatra'
require 'uri'
require 'net/http'
get '/' do
'hello world'
end
post '/' do
ip = params[:ip]
port = params[:port]
path = params[:path]
# do what you want
http = Net::HTTP.new ip, port.to_i
res = http.get path
res.body
end
post data:
ip=192.168.30.214&port=80&path=/r.php%20HTTP/1.1%0d%0ax-injection: memeda
print_r all HTTP Headers：
Create an evil request
post data:
server log:
Suggestion:
Should validate URI legality before send request
btw，
Cloud I have a CVEID with this vulnerability? reported by
@redrain(rootredrain@xxxxxxxxx) and@ztz(ztz5651483@xxxxxxxxx)
4 attachments:
F100918: 123123.png
F100919: 222333.png
F100920: 4444.png
F100921: 5555.png
rootredrain posted a comment.
Jun 22nd (2 days ago)
The problem is this line in lib/net/http/generic_request.rb:324
def write_header(sock, ver, path)
buf = "#{@method} #{path} HTTP/#{ver}\r\n"
each_capitalized do |k,v|
buf << "#{k}: #{v}\r\n"
end
buf << "\r\n"
sock.write buf
end
"#{@method} #{path} HTTP/#{ver}\r\n" should be checked here to avoid
malicious input
shugo posted a comment.
Jun 24th (8 hrs ago)
Thanks for your report.
We don't consider this a vulnerability because Net::HTTP#get is not
designed to accept malicious input.
Applications have responsibility to verify input syntactically and
semantically (accepting all RFC2616-compliant input would not be a
good idea).
So we would like to handle this as a normal issue.
rootredrain posted a comment.
Jun 24th (2 hrs ago)
Hi shugo,
Thanks for the reply. Please don't leave this problem to developers,
they have uneven level at developing.
For example, assume we have a demo website, the only thing do is
generate a new HTTP request:
#!/usr/bin/env ruby
require 'sinatra'
get '/' do
'hello world'
end
post '/' do
ip = params[:ip]
port = params[:port]
path = params[:path]
# send the request to another site
http = Net::HTTP.new ip, port.to_i
res = http.get path
res.body
end
It's a common demand, right ?
But web developer may not realized that sinatra will auto decode url.
Attacker can encode \r\n to %0a%0d, send to the sinatra, sinatra will
decode url to \r\n and pass to thepath, finally cause a HTTP Header
Injection or CRLF Injection.
Please assume all input is malicious. Here is a similar vulnerability
in python: CVE-2016-5699
Here is what another HTTP lib Faraday do may change your mind.
lib/faraday/connection.rb:308
def url_prefix=(url, encoder = nil)
uri = url_prefix = Utils.URI(url)
self.path_prefix = uri.path
# ... ... ...
uri
end
uri = url_prefix = Utils.URI(url) try to convert url to URI, It will
raise an error whenurl is invalid.
lib/faraday/connection.rb:399
def build_exclusive_url(url = nil, params = nil, params_encoder = nil)
url = nil if url.respond_to?(:empty?) and url.empty?
base = url_prefix
# ... ... ...
uri = url ? base + url : base
# ... ... ...
end
uri = url ? base + url : base will trigger another examination convert_to_uri:
def convert_to_uri(uri)
if uri.is_a?(URI::Generic)
uri
elsif uri = String.try_convert(uri)
parse(uri)
else
raise ArgumentError,
"bad argument (expected URI object or URI string)"
end
end
If url is invalid, it will raise an error.
Please let me know if you need more info.
tenderlove posted a comment.
Jun 24th (2 hrs ago)
It's a common demand, right ?
I'm not sure about that.
I think this is a bug we should probably address, but I don't think we
should consider this a vulnerability. Fetching arbitrary paths from
user input seems pretty dubious.
rootredrain posted a comment.
Jun 24th (about 1 hr ago)
Hi tenderlove,
Here is my point :
All input can not be trusted.
We should validate url in Net::HTTP
tenderlove posted a comment.
Jun 24th (about 1 hr ago)
All input can not be trusted.
Yes, people should be whitelisting paths passed in. An open proxy is
already a vulnerability, regardless of header injection.
As I said, we should treat this as a bug. But since an open proxy is
already a security problem (that we cannot fix), then I don't think
this bug should be treated as a security issue.
shugo posted a comment.
Jun 24th (34 mins ago)
But web developer may not realized that sinatra will auto decode url.
Attacker can encode \r\n to %0a%0d, send to the sinatra, sinatra will
decode url to \r\n and pass to the path, finally cause a HTTP Header
Injection or CRLF Injection.
In that case, it seems to be a bug of that application, not Net::HTTP#get.
I'm not against adding argument verification to Net::HTTP#get, though.
rootredrain posted a comment.
Jun 24th (29 mins ago)
But since an open proxy is already a security problem
Yes, an open proxy is already a vulnerability and you can't fix that,
but attack scenarios is not only include an open proxy, but also
include many other parts.
A site like google image, user can paste image url on it, then site
will request the resource. It's possible to suffer this attack.
Some video sites allow user reference outside resource. It's possible
to suffer this attack.
So you can not treat it occur in an unusual scenarios. I still
consider it was a security issue.
rootredrain posted a comment.
Jun 24th (27 mins ago)
If you believe this is not a issue, please allow the public disclosure.
tenderlove closed the report and changed the status to Informative.
Jun 24th (23 mins ago)
I've closed as informative, and I'll allow public disclosure.
tenderlove requested to disclose this report publicly.
Jun 24th (20 mins ago)
rootredrain has requested mediation from HackerOne Support.
Jun 24th (15 mins ago)
The HTTP scheme handler accepts percent-encoded values as part of the URL.
The generic_request.rb allows unsafe characters, it dosen't have any
safe filtration, attackers can cause actual security threat. so we
consider it is a vulnerability
