CVE-2018-3769: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: ruby-grape ruby gem suffers from a cross-site scripting (XSS) vulnerability via "format" parameter.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 6.1
Published: 2018-07-05T16:29Z
Repository: https://github.com/ruby-grape/grape
Language: Ruby
Files changed: CHANGELOG.md, lib/grape/middleware/error.rb, spec/grape/api_spec.rb, spec/grape/middleware/exception_spec.rb
Commit date: 2018-05-25 21:07:45 -0400
Commit message: When returning an HTML error, make sure it's safe (#1763)

* When calling into an API specifying a crafted format that is HTML,
the returned error renders the HTML back to the user, causing a potential XSS
issue.  For example:

http://example.com/api/endpoint?format=%3Cscript%3Ealert(document.cookie)%3C/script%3E

Renders as html:

The requested format '<script>alert(document.cookie)</script>' is not supported.

When an error generates html back to the user, make sure it's properly escaped.

Fixes issue #1762

* Add changelog entry

* Use a method that also works in rails3

* Add spec formatting for older rails/activesupport version

Diff: diff --git a/CHANGELOG.md b/CHANGELOG.md
index 574c0c0..d2d99ec 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,10 +4,12 @@
 
 * [#1759](https://github.com/ruby-grape/grape/pull/1759): Instrument serialization as `'format_response.grape'` - [@zvkemp](https://github.com/zvkemp).
 
 #### Fixes
 
+
+* [#1762](https://github.com/ruby-grape/grape/pull/1763): Fix unsafe HTML rendering on errors - [@ctennis](https://github.com/ctennis).
 * [#1759](https://github.com/ruby-grape/grape/pull/1759): Update appraisal for rails_edge - [@zvkemp](https://github.com/zvkemp).
 * [#1758](https://github.com/ruby-grape/grape/pull/1758): Fix expanding load_path in gemspec - [@2maz](https://github.com/2maz).
 * Your contribution here.
 
 ### 1.0.3 (4/23/2018)
diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb
index f721649..0991b48 100644
--- a/lib/grape/middleware/error.rb
+++ b/lib/grape/middleware/error.rb
@@ -1,6 +1,7 @@
 require 'grape/middleware/base'
+require 'active_support/core_ext/string/output_safety'
 
 module Grape
   module Middleware
     class Error < Base
       def default_options
@@ -67,10 +68,13 @@ module Grape
         original_exception = error.is_a?(Exception) ? error : error[:original_exception] || nil
         rack_response(format_message(message, backtrace, original_exception), status, headers)
       end
 
       def rack_response(message, status = options[:default_status], headers = { Grape::Http::Headers::CONTENT_TYPE => content_type })
+        if headers[Grape::Http::Headers::CONTENT_TYPE] == TEXT_HTML
+          message = ERB::Util.html_escape(message)
+        end
         Rack::Response.new([message], status, headers).finish
       end
 
       def format_message(message, backtrace, original_exception = nil)
         format = env[Grape::Env::API_FORMAT] || options[:format]
diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb
index 82c5024..c38205f 100644
--- a/spec/grape/api_spec.rb
+++ b/spec/grape/api_spec.rb
@@ -2140,11 +2140,15 @@ XML
       subject.get :excel do
         'some binary content'
       end
       get '/excel.json'
       expect(last_response.status).to eq(406)
-      expect(last_response.body).to eq("The requested format 'txt' is not supported.")
+      if ActiveSupport::VERSION::MAJOR == 3
+        expect(last_response.body).to eq('The requested format &#x27;txt&#x27; is not supported.')
+      else
+        expect(last_response.body).to eq('The requested format &#39;txt&#39; is not supported.')
+      end
     end
   end
 
   describe '.formatter' do
     context 'multiple formatters' do
@@ -3522,11 +3526,31 @@ XML
       subject.get '/something' do
         'foo'
       end
       get '/something'
       expect(last_response.status).to eq(406)
-      expect(last_response.body).to eq("{\"error\":\"The requested format 'txt' is not supported.\"}")
+      if ActiveSupport::VERSION::MAJOR == 3
+        expect(last_response.body).to eq('{&quot;error&quot;:&quot;The requested format &#x27;txt&#x27; is not supported.&quot;}')
+      else
+        expect(last_response.body).to eq('{&quot;error&quot;:&quot;The requested format &#39;txt&#39; is not supported.&quot;}')
+      end
+    end
+  end
+
+  context 'with unsafe HTML format specified' do
+    it 'escapes the HTML' do
+      subject.content_type :json, 'application/json'
+      subject.get '/something' do
+        'foo'
+      end
+      get '/something?format=<script>blah</script>'
+      expect(last_response.status).to eq(406)
+      if ActiveSupport::VERSION::MAJOR == 3
+        expect(last_response.body).to eq('The requested format &#x27;&lt;script&gt;blah&lt;/script&gt;&#x27; is not supported.')
+      else
+        expect(last_response.body).to eq('The requested format &#39;&lt;script&gt;blah&lt;/script&gt;&#39; is not supported.')
+      end
     end
   end
 
   context 'body' do
     context 'false' do
diff --git a/spec/grape/middleware/exception_spec.rb b/spec/grape/middleware/exception_spec.rb
index 2f65cc5..c11b496 100644
--- a/spec/grape/middleware/exception_spec.rb
+++ b/spec/grape/middleware/exception_spec.rb
@@ -190,11 +190,11 @@ describe Grape::Middleware::Error do
         run ExceptionSpec::ExceptionApp
       end
     end
     it 'is possible to return errors in jsonapi format' do
       get '/'
-      expect(last_response.body).to eq('{"error":"rain!"}')
+      expect(last_response.body).to eq('{&quot;error&quot;:&quot;rain!&quot;}')
     end
   end
 
   context do
     subject do
@@ -205,12 +205,12 @@ describe Grape::Middleware::Error do
       end
     end
 
     it 'is possible to return hash errors in jsonapi format' do
       get '/'
-      expect(['{"error":"rain!","detail":"missing widget"}',
-              '{"detail":"missing widget","error":"rain!"}']).to include(last_response.body)
+      expect(['{&quot;error&quot;:&quot;rain!&quot;,&quot;detail&quot;:&quot;missing widget&quot;}',
+              '{&quot;detail&quot;:&quot;missing widget&quot;,&quot;error&quot;:&quot;rain!&quot;}']).to include(last_response.body)
     end
   end
 
   context do
     subject do
@@ -256,11 +256,11 @@ describe Grape::Middleware::Error do
         run ExceptionSpec::ExceptionApp
       end
     end
     it 'is possible to specify a custom formatter' do
       get '/'
-      expect(last_response.body).to eq('{:custom_formatter=>"rain!"}')
+      expect(last_response.body).to eq('{:custom_formatter=&gt;&quot;rain!&quot;}')
     end
   end
 
   context do
     subject do

Vulnerable code:       expect(last_response.body).to eq("The requested format 'txt' is not supported.")
      expect(last_response.body).to eq("{\"error\":\"The requested format 'txt' is not supported.\"}")
      expect(last_response.body).to eq('{"error":"rain!"}')
      expect(['{"error":"rain!","detail":"missing widget"}',
              '{"detail":"missing widget","error":"rain!"}']).to include(last_response.body)
      expect(last_response.body).to eq('{:custom_formatter=>"rain!"}')
Fixed code: 
* [#1762](https://github.com/ruby-grape/grape/pull/1763): Fix unsafe HTML rendering on errors - [@ctennis](https://github.com/ctennis).
require 'active_support/core_ext/string/output_safety'
        if headers[Grape::Http::Headers::CONTENT_TYPE] == TEXT_HTML
          message = ERB::Util.html_escape(message)
        end
      if ActiveSupport::VERSION::MAJOR == 3
        expect(last_response.body).to eq('The requested format &#x27;txt&#x27; is not supported.')
      else
        expect(last_response.body).to eq('The requested format &#39;txt&#39; is not supported.')
      end
      if ActiveSupport::VERSION::MAJOR == 3
        expect(last_response.body).to eq('{&quot;error&quot;:&quot;The requested format &#x27;txt&#x27; is not supported.&quot;}')
      else
        expect(last_response.body).to eq('{&quot;error&quot;:&quot;The requested format &#39;txt&#39; is not supported.&quot;}')
      end
    end
  end

  context 'with unsafe HTML format specified' do
    it 'escapes the HTML' do
      subject.content_type :json, 'application/json'
      subject.get '/something' do
        'foo'
      end
      get '/something?format=<script>blah</script>'
      expect(last_response.status).to eq(406)
      if ActiveSupport::VERSION::MAJOR == 3
        expect(last_response.body).to eq('The requested format &#x27;&lt;script&gt;blah&lt;/script&gt;&#x27; is not supported.')
      else
        expect(last_response.body).to eq('The requested format &#39;&lt;script&gt;blah&lt;/script&gt;&#39; is not supported.')
      end
      expect(last_response.body).to eq('{&quot;error&quot;:&quot;rain!&quot;}')
      expect(['{&quot;error&quot;:&quot;rain!&quot;,&quot;detail&quot;:&quot;missing widget&quot;}',
              '{&quot;detail&quot;:&quot;missing widget&quot;,&quot;error&quot;:&quot;rain!&quot;}']).to include(last_response.body)
      expect(last_response.body).to eq('{:custom_formatter=&gt;&quot;rain!&quot;}')