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

XSS from params parser exception (status code : 400) #1428

Closed
JokerCatz opened this issue May 22, 2018 · 16 comments
Closed

XSS from params parser exception (status code : 400) #1428

JokerCatz opened this issue May 22, 2018 · 16 comments
Assignees
Milestone

Comments

@JokerCatz
Copy link

JokerCatz commented May 22, 2018

source at :

def params

def params
  super
rescue Rack::Utils::ParameterTypeError, Rack::Utils::InvalidParameterError => e
  raise BadRequest, "Invalid query parameters: #{e.message}"
end

demo code

# server.rb
require 'sinatra'
# yes ... it empty just require sinatra gem

call curl like

curl -i 'http://127.0.0.1:4567/' --data $'" %x\\"> <script>alert(1)</script>"'

return

HTTP/1.1 400 Bad Request
Content-Type: text/html;charset=utf-8
X-XSS-Protection: 1; mode=block
X-Content-Type-Options: nosniff
X-Frame-Options: SAMEORIGIN
Content-Length: 81

Invalid query parameters: invalid %-encoding (" %x\"> <script>alert(1)</script>")

I know it 400 , but the error message can be HTML ... & no way to disable / filter it ...

and you can use code like this to overwrite it

module Sinatra
  class Request < Rack::Request
    def params
      super
    rescue Rack::Utils::ParameterTypeError, Rack::Utils::InvalidParameterError => e
      raise BadRequest, "404"
    end
  end
end
@namusyaka
Copy link
Member

Really critical issue, thank you for the report.

This vulnerability has been added since this commit.
I'm going to fix this soon, also we should open the CVE about this.
/cc @jkowens

@namusyaka namusyaka added this to the v2.0.2 milestone May 30, 2018
@jkowens
Copy link
Member

jkowens commented May 31, 2018

The attached fix looks good 👍

namusyaka added a commit that referenced this issue May 31, 2018
escape invalid query params, fixes #1428
@namusyaka
Copy link
Member

CVE is here: https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-11627
I'm planning to cut next release v2.0.2 in a few days, thanks.

@rschultheis
Copy link

👋 I am Rob and I work on the GitHub team that sends vulnerability alerts. Since the CVE is now public, we'd like to send out alerts to users of sinatra <= 2.0.1.

We see here though that you're working on releasing 2.0.2 into RubyGems however. We plan to publish alerts an alert in the next 48 hours - can you advise on when you plan to release 2.0.2?

Thank you! :octocat:

@namusyaka
Copy link
Member

@rschultheis Sorry, Rob. We will release within a few hours from now.

@namusyaka
Copy link
Member

Done.
See: https://twitter.com/namusyaka/status/1004045559307558912

@rschultheis
Copy link

rschultheis commented Jun 5, 2018

Thank you for the quick turn around @namusyaka 🙇 . We've now started publishing alerts for the CVE.

@dentarg
Copy link
Member

dentarg commented Jun 7, 2018

@namusyaka @rschultheis FYI Sinatra applications using 1.4.8 (and lower I guess) are also affected, when run as "modular" applications

see my repro at https://github.com/dentarg/gists/tree/master/gists/sinatra-CVE-2018-11627#cve-2018-11627

@dentarg
Copy link
Member

dentarg commented Jun 7, 2018

My bad, the issue lies with rack 1.6.x. I will open the issue there.

@dentarg
Copy link
Member

dentarg commented Jun 7, 2018

Hmm, rack 1.6.10 isn't vulnerable out-of-the-box. Might still be an issue with Sinatra?

@dentarg
Copy link
Member

dentarg commented Jun 7, 2018

Ah, I see that it was discussed in #1070, to not release to the 1.x series.

@dentarg
Copy link
Member

dentarg commented Jun 7, 2018

Ah, with RACK_ENV=production Sinatra 1.4.8 does not exhibit the problem.

$ RACK_ENV=production BUNDLE_GEMFILE=Gemfile-sinatra-1.4.8 bundle exec ruby modular_app.rb
[2018-06-07 13:51:27] INFO  WEBrick 1.3.1
[2018-06-07 13:51:27] INFO  ruby 2.4.2 (2017-09-14) [x86_64-darwin16]
== Sinatra (v1.4.8) has taken the stage on 4567 for production with backup from WEBrick
[2018-06-07 13:51:27] INFO  WEBrick::HTTPServer#start: pid=37252 port=4567
[2018-06-07 13:51:30] ERROR Rack::Utils::InvalidParameterError: invalid %-encoding (" %x\\"> <script>alert(1)</script>")
	/Users/dentarg/.gem/ruby/2.4.2/gems/rack-1.6.10/lib/rack/utils.rb:127:in `rescue in parse_nested_query'
	/Users/dentarg/.gem/ruby/2.4.2/gems/rack-1.6.10/lib/rack/utils.rb:117:in `parse_nested_query'
	/Users/dentarg/.gem/ruby/2.4.2/gems/rack-1.6.10/lib/rack/request.rb:371:in `parse_query'
	/Users/dentarg/.gem/ruby/2.4.2/gems/rack-1.6.10/lib/rack/request.rb:215:in `POST'
	/Users/dentarg/.gem/ruby/2.4.2/gems/rack-1.6.10/lib/rack/request.rb:230:in `params'
	/Users/dentarg/.gem/ruby/2.4.2/gems/sinatra-1.4.8/lib/sinatra/base.rb:902:in `call!'
	/Users/dentarg/.gem/ruby/2.4.2/gems/sinatra-1.4.8/lib/sinatra/base.rb:895:in `call'
	/Users/dentarg/.gem/ruby/2.4.2/gems/rack-protection-1.5.5/lib/rack/protection/xss_header.rb:18:in `call'
	/Users/dentarg/.gem/ruby/2.4.2/gems/rack-protection-1.5.5/lib/rack/protection/path_traversal.rb:16:in `call'
	/Users/dentarg/.gem/ruby/2.4.2/gems/rack-protection-1.5.5/lib/rack/protection/json_csrf.rb:18:in `call'
	/Users/dentarg/.gem/ruby/2.4.2/gems/rack-protection-1.5.5/lib/rack/protection/base.rb:49:in `call'
	/Users/dentarg/.gem/ruby/2.4.2/gems/rack-protection-1.5.5/lib/rack/protection/base.rb:49:in `call'
	/Users/dentarg/.gem/ruby/2.4.2/gems/rack-protection-1.5.5/lib/rack/protection/frame_options.rb:31:in `call'
	/Users/dentarg/.gem/ruby/2.4.2/gems/rack-1.6.10/lib/rack/nulllogger.rb:9:in `call'
	/Users/dentarg/.gem/ruby/2.4.2/gems/rack-1.6.10/lib/rack/head.rb:13:in `call'
	/Users/dentarg/.gem/ruby/2.4.2/gems/sinatra-1.4.8/lib/sinatra/base.rb:182:in `call'
	/Users/dentarg/.gem/ruby/2.4.2/gems/sinatra-1.4.8/lib/sinatra/base.rb:2013:in `call'
	/Users/dentarg/.gem/ruby/2.4.2/gems/sinatra-1.4.8/lib/sinatra/base.rb:1487:in `block in call'
	/Users/dentarg/.gem/ruby/2.4.2/gems/sinatra-1.4.8/lib/sinatra/base.rb:1787:in `synchronize'
	/Users/dentarg/.gem/ruby/2.4.2/gems/sinatra-1.4.8/lib/sinatra/base.rb:1487:in `call'
	/Users/dentarg/.gem/ruby/2.4.2/gems/rack-1.6.10/lib/rack/handler/webrick.rb:88:in `service'
	/Users/dentarg/.rubies/ruby-2.4.2/lib/ruby/2.4.0/webrick/httpserver.rb:140:in `service'
	/Users/dentarg/.rubies/ruby-2.4.2/lib/ruby/2.4.0/webrick/httpserver.rb:96:in `run'
	/Users/dentarg/.rubies/ruby-2.4.2/lib/ruby/2.4.0/webrick/server.rb:290:in `block in start_thread'
127.0.0.1 - - [07/Jun/2018:13:51:30 CEST] "POST / HTTP/1.1" 500 365
- -> /
^C== Sinatra has ended his set (crowd applauds)
[2018-06-07 13:54:04] INFO  going to shutdown ...
[2018-06-07 13:54:04] INFO  WEBrick::HTTPServer#start done.
$ ./test.sh
+ curl --include http://127.0.0.1:4567/ --data '" %x\"> <script>alert(1)</script>"'
HTTP/1.1 500 Internal Server Error
Content-Type: text/html; charset=ISO-8859-1
Server: WEBrick/1.3.1 (Ruby/2.4.2/2017-09-14)
Date: Thu, 07 Jun 2018 11:51:30 GMT
Content-Length: 365
Connection: close

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0//EN">
<HTML>
  <HEAD><TITLE>Internal Server Error</TITLE></HEAD>
  <BODY>
    <H1>Internal Server Error</H1>
    invalid %-encoding (&quot; %x\&quot;&gt; &lt;script&gt;alert(1)&lt;/script&gt;&quot;)
    <HR>
    <ADDRESS>
     WEBrick/1.3.1 (Ruby/2.4.2/2017-09-14) at
     127.0.0.1:4567
    </ADDRESS>
  </BODY>
</HTML>

@dentarg
Copy link
Member

dentarg commented Jun 7, 2018

Ah, with RACK_ENV=production Sinatra 1.4.8 does not exhibit the problem.

...but Sinatra 2.0.1 does have the problem even with RACK_ENV=production (or APP_ENV).

Sorry for the noise, but maybe this saves someone else from playing 🕵️ themselves :)

dentarg added a commit to twingly/feedbag.herokuapp.com that referenced this issue Jun 7, 2018
dentarg added a commit to twingly/feedjira.herokuapp.com that referenced this issue Jun 7, 2018
@rschultheis
Copy link

Hi @dentarg,

The GitHub security alerts for CVE-2018-11627 are currently only going to repos that have sinatra 2.0.0 or 2.0.1 in their Gemfile and/or Gemfile.lock files. Since the CVE did not specify a minimum version, we did perform a very basic analysis to determine that the minimum non-beta version appears to be 2.0.0. If you have more insight onto what the correct minimum version for this vulnerability is we can update our data and alert the appropriate repos.

Thank you for the ping!

@dentarg
Copy link
Member

dentarg commented Jun 7, 2018

@rschultheis I think you are correct in that minimum version for this vulnerability is 2.0.0. (However, I have only checked 1.4.8, not anything below that.)

aratak added a commit to cimon-io/billet that referenced this issue Sep 3, 2018
Name: sinatra
Version: 2.0.1
Advisory: CVE-2018-11627
Criticality: Unknown
URL: sinatra/sinatra#1428
Title: XSS via the 400 Bad Request page
Solution: upgrade to >= 2.0.2
Cruikshanks added a commit to DEFRA/quke-demo-app that referenced this issue Sep 9, 2019
Because in the previous version of the gemspec we had an open reference to Sinatra it meant we were essentially saying any version would do.

Hakiri was flagging this with [CVE-2018-7212](sinatra/sinatra#1379), the resolution of which was to specify a version equal to or greater than 2.0.1

It was then flagging this project with [CVE-2018-11627](sinatra/sinatra#1428), and again the resolution was to specify a version, this time equal to or greater than 2.0.2
Cruikshanks added a commit to DEFRA/quke-demo-app that referenced this issue Sep 9, 2019
Because in the previous version of the gemspec we had an open reference to Sinatra it meant we were essentially saying any version would do.

Hakiri was flagging this with [CVE-2018-7212](sinatra/sinatra#1379), the resolution of which was to specify a version equal to or greater than 2.0.1

It was then flagging this project with [CVE-2018-11627](sinatra/sinatra#1428), and again the resolution was to specify a version, this time equal to or greater than 2.0.2
@nutronex
Copy link

By the way, Is it possible to attack against other , I think so because form content-type "x-www-form-urlencoded" will encode < to %3c and "text/palin" will not work in sinatra in this case .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants