PR#2: Fix XSS in the 400 Bad Request page. - rpms/rubygem-sinatra

rpms / rubygem-sinatra

#2 Fix XSS in the 400 Bad Request page.

Merged 6 years ago by jaruga. Opened 6 years ago by jaruga.

rpms/ jaruga/rubygem-sinatra feature/f27-fix-xss-for-http-400-request-page into f27

Fix XSS in the 400 Bad Request page (CVE-2018-11627, rhbz#1585218).

Jun Aruga • 6 years ago

cca46fc

rubygem-sinatra-2.0.0-escape-invalid-query-params.patch

file added

+25

		`@@ -0,0 +1,25 @@`
		`+ From a7a7019090a121a0d9b91075dfd4c0148e913ef3 Mon Sep 17 00:00:00 2001`
		`+ From: Kunpei Sakai <namusyaka@gmail.com>`
		`+ Date: Thu, 31 May 2018 01:05:27 +0900`
		`+ Subject: [PATCH] escape invalid query params, fixes #1428`
		`+`
		`+ ---`
		`+ lib/sinatra/base.rb \| 2 +-`
		`+ 1 file changed, 1 insertion(+), 1 deletion(-)`
		`+`
		`+ diff --git a/lib/sinatra/base.rb b/lib/sinatra/base.rb`
		`+ index 3b76793f..cd01e994 100644`
		`+ --- a/lib/sinatra/base.rb`
		`+ +++ b/lib/sinatra/base.rb`
		`+ @@ -78,7 +78,7 @@ module Sinatra`
		`+ def params`
		`+ super`
		`+ rescue Rack::Utils::ParameterTypeError, Rack::Utils::InvalidParameterError => e`
		`+ - raise BadRequest, "Invalid query parameters: #{e.message}"`
		`+ + raise BadRequest, "Invalid query parameters: #{Rack::Utils.escape_html(e.message)}"`
		`+ end`
		`+`
		`+ private`
		`+ --`
		`+ 2.17.0`
		`+`

rubygem-sinatra.spec

file modified

+9 -1

		`@@ -5,13 +5,16 @@`
		`Summary: Ruby-based web application framework`
		`Name: rubygem-%{gem_name}`
		`Version: 2.0.0`
		`- Release: 2%{?dist}`
		`+ Release: 3%{?dist}`
		`License: MIT`
		`URL: http://www.sinatrarb.com/`
		`Source0: https://rubygems.org/gems/%{gem_name}-%{version}.gem`
		`# git clone https://github.com/sinatra/sinatra.git && cd sinatra`
		`# git checkout v2.0.0 && tar czvf sinatra-2.0.0-tests.tgz test/`
		`Source1: %{gem_name}-%{version}-tests.tgz`
		`+ # Fix XSS in the 400 Bad Request page.`
		`+ # https://github.com/sinatra/sinatra/commit/1278686`
		`+ Patch0: rubygem-sinatra-2.0.0-escape-invalid-query-params.patch`
		`BuildRequires: rubygems-devel`
		`%if ! 0%{?bootstrap}`
		`BuildRequires: rubygem(rack) >= 2.0`
		`@@ -58,6 +61,8 @@`

		`gem spec %{SOURCE0} -l --ruby > %{gem_name}.gemspec`

		`+ %patch0 -p1`
		`+`
		`%build`
		`gem build %{gem_name}.gemspec`
		`%gem_install`
		`@@ -108,6 +113,9 @@`
		`%{gem_instdir}/examples`

		`%changelog`
		`+ * Wed Jun 13 2018 Jun Aruga <jaruga@redhat.com> - 1:2.0.0-3`
		`+ - Fix XSS in the 400 Bad Request page (CVE-2018-11627, rhbz#1585218).`
		`+`
		`* Thu Jul 27 2017 Fedora Release Engineering <releng@fedoraproject.org> - 1:2.0.0-2`
		`- Rebuilt for https://fedoraproject.org/wiki/Fedora_27_Mass_Rebuild`

jaruga commented 6 years ago

This PR is for f27 branch.
This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1585218

I did below tests.
Can you review this PR such as changelog and commit's comment.

rpmlint check: ok
Install check: ok
IRB (Run at mock environment) check: ok
Scratch build: ok
https://koji.fedoraproject.org/koji/taskinfo?taskID=27596113

I am going to check if security issue is actually fixed by this patch.

vondruch commented 6 years ago

I am going to check if security issue is actually fixed by this patch.

This is the reproducer, right?

https://github.com/dentarg/gists/tree/master/gists/sinatra-CVE-2018-11627

I have not tested the functionality, but the diff LGTM.

jaruga commented 6 years ago

@vondruch thanks for the information. Let me check. and thanks for the review.

I was seeing the target issue page. But the page you mentioned might be better.
https://github.com/sinatra/sinatra/issues/1428

jaruga commented 6 years ago

I tested this patch with the script files: https://github.com/dentarg/gists/tree/master/gists/sinatra-CVE-2018-11627
I confirmed it worked.

$ ruby modular_app.rb &

$ ./test.sh

Then below is okay result with patched file. The 400 error message is http-escaped.

Invalid query parameters: invalid %-encoding (&quot; %x\&quot;&gt; &lt;script&gt;alert(1)&lt;&#x2F;script&gt;&quot;)
````

Below is ng result with unpatched file. The message is not http-escaped.

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

For instance, I tested like this in the %check section.
https://src.fedoraproject.org/fork/jaruga/rpms/rubygem-sinatra/c/2746958a915e03e7c32b593674e9aaabd1a4020f?branch=feature%2Ff27-fix-xss-for-http-400-request-page-test

And the scratch build is ok.
Scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=27612051

@vondruch If you are okay for the test, I would merge this PR.

Pull-Request has been merged by jaruga

6 years ago