Merge pull request #588 from onc-healthit/fi-1341-2-4

Security Fixes

Gemfile

@@ -2,6 +2,7 @@
 
 source 'https://rubygems.org'
 
+gem 'actionview'
 gem 'activerecord', '~> 5.2'
 gem 'addressable'
 gem 'ast'
@@ -19,7 +20,7 @@ gem 'rb-readline'
 gem 'rest-client'
 gem 'rubyXL'
 gem 'selenium-webdriver'
-gem 'sinatra'
+gem 'sinatra', '>= 2.1.0'
 gem 'sinatra-contrib'
 gem 'sqlite3'
 gem 'standalone_migrations', '~> 6.0'

Gemfile.lock

@@ -29,7 +29,6 @@ GEM
       public_suffix (>= 2.0.2, < 5.0)
     arel (9.0.0)
     ast (2.4.0)
-    backports (3.15.0)
     base62-rb (0.3.1)
     bcp47 (0.3.3)
       i18n
@@ -124,10 +123,11 @@ GEM
     mini_portile2 (2.5.3)
     minitest (5.14.4)
     msgpack (1.3.1)
-    multi_json (1.13.1)
+    multi_json (1.15.0)
     multi_xml (0.6.0)
     multipart-post (2.1.1)
-    mustermann (1.0.3)
+    mustermann (1.1.1)
+      ruby2_keywords (~> 0.0.1)
     netrc (0.11.0)
     nokogiri (1.11.7)
       mini_portile2 (~> 2.5.0)
@@ -150,7 +150,7 @@ GEM
     public_suffix (4.0.1)
     racc (1.5.2)
     rack (2.2.3)
-    rack-protection (2.0.7)
+    rack-protection (2.1.0)
       rack
     rack-test (1.1.0)
       rack (>= 1.0, < 3)
@@ -197,17 +197,16 @@ GEM
       json (>= 1.8, < 3)
       simplecov-html (~> 0.10.0)
     simplecov-html (0.10.2)
-    sinatra (2.0.7)
+    sinatra (2.1.0)
       mustermann (~> 1.0)
-      rack (~> 2.0)
-      rack-protection (= 2.0.7)
+      rack (~> 2.2)
+      rack-protection (= 2.1.0)
       tilt (~> 2.0)
-    sinatra-contrib (2.0.7)
-      backports (>= 2.8.2)
+    sinatra-contrib (2.1.0)
       multi_json
       mustermann (~> 1.0)
-      rack-protection (= 2.0.7)
-      sinatra (= 2.0.7)
+      rack-protection (= 2.1.0)
+      sinatra (= 2.1.0)
       tilt (~> 2.0)
     sqlite3 (1.4.2)
     standalone_migrations (6.0.0)
@@ -238,6 +237,7 @@ PLATFORMS
   ruby
 
 DEPENDENCIES
+  actionview
   activerecord (~> 5.2)
   addressable
   ast
@@ -260,7 +260,7 @@ DEPENDENCIES
   rubyXL
   selenium-webdriver
   simplecov
-  sinatra
+  sinatra (>= 2.1.0)
   sinatra-contrib
   sqlite3
   standalone_migrations (~> 6.0)

Gemfile.postgres

@@ -2,6 +2,7 @@
 
 source 'https://rubygems.org'
 
+gem 'actionview'
 gem 'activerecord', '~> 5.2'
 gem 'addressable'
 gem 'ast'
@@ -20,7 +21,7 @@ gem 'rb-readline'
 gem 'rest-client'
 gem 'rubyXL'
 gem 'selenium-webdriver'
-gem 'sinatra'
+gem 'sinatra', '>= 2.1.0'
 gem 'sinatra-contrib'
 gem 'sqlite3'
 gem 'standalone_migrations', '~> 6.0'

Gemfile.postgres.lock

@@ -29,7 +29,6 @@ GEM
       public_suffix (>= 2.0.2, < 5.0)
     arel (9.0.0)
     ast (2.4.0)
-    backports (3.15.0)
     base62-rb (0.3.1)
     bcp47 (0.3.3)
       i18n
@@ -107,10 +106,11 @@ GEM
     mini_portile2 (2.5.3)
     minitest (5.14.4)
     msgpack (1.3.1)
-    multi_json (1.13.1)
+    multi_json (1.15.0)
     multi_xml (0.6.0)
     multipart-post (2.1.1)
-    mustermann (1.0.3)
+    mustermann (1.1.1)
+      ruby2_keywords (~> 0.0.1)
     netrc (0.11.0)
     nokogiri (1.11.7)
       mini_portile2 (~> 2.5.0)
@@ -134,7 +134,7 @@ GEM
     public_suffix (4.0.1)
     racc (1.5.2)
     rack (2.2.3)
-    rack-protection (2.0.7)
+    rack-protection (2.1.0)
       rack
     rack-test (1.1.0)
       rack (>= 1.0, < 3)
@@ -167,6 +167,7 @@ GEM
       ruby-progressbar (~> 1.7)
       unicode-display_width (>= 1.4.0, < 1.7)
     ruby-progressbar (1.10.1)
+    ruby2_keywords (0.0.5)
     rubyXL (3.4.9)
       nokogiri (>= 1.4.4)
       rubyzip (>= 1.3.0)
@@ -180,17 +181,16 @@ GEM
       json (>= 1.8, < 3)
       simplecov-html (~> 0.10.0)
     simplecov-html (0.10.2)
-    sinatra (2.0.7)
+    sinatra (2.1.0)
       mustermann (~> 1.0)
-      rack (~> 2.0)
-      rack-protection (= 2.0.7)
+      rack (~> 2.2)
+      rack-protection (= 2.1.0)
       tilt (~> 2.0)
-    sinatra-contrib (2.0.7)
-      backports (>= 2.8.2)
+    sinatra-contrib (2.1.0)
       multi_json
       mustermann (~> 1.0)
-      rack-protection (= 2.0.7)
-      sinatra (= 2.0.7)
+      rack-protection (= 2.1.0)
+      sinatra (= 2.1.0)
       tilt (~> 2.0)
     sqlite3 (1.4.2)
     standalone_migrations (6.0.0)
@@ -221,6 +221,7 @@ PLATFORMS
   ruby
 
 DEPENDENCIES
+  actionview
   activerecord (~> 5.2)
   addressable
   ast
@@ -244,7 +245,7 @@ DEPENDENCIES
   rubyXL
   selenium-webdriver
   simplecov
-  sinatra
+  sinatra (>= 2.1.0)
   sinatra-contrib
   sqlite3
   standalone_migrations (~> 6.0)

lib/app.rb

@@ -15,6 +15,7 @@
 require 'kramdown'
 
 require 'rack'
+require_relative 'app/ext/sinatra_base'
 require_relative 'app/utils/logging'
 require_relative 'app/models'
 require_relative 'app/endpoint'

lib/app/endpoint.rb

@@ -11,6 +11,8 @@
 module Inferno
   class App
     class Endpoint < Sinatra::Base
+      include ERB::Util
+
       register Sinatra::ConfigFile
 
       config_file File.join('..', '..', ENV['INFERNO_CONFIG_FILE'] || 'config.yml')

lib/app/ext/sinatra_base.rb

@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+module Sinatra
+  class Base
+    # Brought in from `main` branch of Sinatra because it fixes a tree traveral
+    # but is not yet available in a published gem.  Hopefully this will be
+    # available in the version after v2.1.0.  This should be removed once it is
+    # fixed in the latest published gem. Disabled rubocop so exact syntax from
+    # original repo maintained.
+
+    # See https://github.com/sinatra/sinatra/blob/2e980f3534b680fbd79d7ec39552b4afb7675d6c/lib/sinatra/base.rb#L1092-L1106
+
+    # rubocop:disable all
+
+    # Attempt to serve static files from public directory. Throws :halt when
+    # a matching file is found, returns nil otherwise.
+    def static!(options = {})
+      return if (public_dir = settings.public_folder).nil?
+      path = "#{public_dir}#{URI_INSTANCE.unescape(request.path_info)}"
+      return unless valid_path?(path)
+
+      path = File.expand_path(path)
+      return unless path.start_with?(File.expand_path(public_dir) + '/')
+      return unless File.file?(path)
+
+      env['sinatra.static_file'] = path
+      cache_control(*settings.static_cache_control) if settings.static_cache_control?
+      send_file path, options.merge(:disposition => nil)
+    end
+
+    # rubocop:enable all
+  end
+end

lib/app/helpers/browser_logic.rb

@@ -1,9 +1,15 @@
 # frozen_string_literal: true
 
+require 'action_view/helpers/output_safety_helper'
+require 'action_view/helpers/capture_helper'
+require 'action_view/helpers/javascript_helper'
+
 module Inferno
   class App
     module Helpers
       module BrowserLogic
+        include ActionView::Helpers::JavaScriptHelper
+
         def js_hide_wait_modal
           "<script>console.log('hide_wait_modal');$('#WaitModal').modal('hide');</script>"
         end
@@ -41,10 +47,11 @@ def js_redirect(location)
         end
 
         def js_redirect_modal(location, _sequence, instance)
-          ok_button = "<a href=\"#{location}\" class=\"btn btn-primary\">Continue</a>"
+          safe_location = ERB::Util.html_escape(location)
+          ok_button = "<a href=\"#{safe_location}\" class=\"btn btn-primary\">Continue</a>"
           warning_text = "Inferno will now redirect you to an external website for user authorization.  For this test sequence to complete successfully, you will need to select a patient and authorize the Inferno client to access their data.  Once you authorize the Inferno client to access patient data, you should be redirected back to Inferno.  If something goes wrong, you can always return to Inferno at <a href=\"#{instance.base_url}#{base_path}/#{instance.id}\">#{instance.base_url}#{base_path}/#{instance.id}</a>.<br/><br/>"
 
-          "<script>console.log('js_redirect_modal');$('#testsRunningModal').find('.modal-body').html('#{warning_text} <textarea readonly class=\"form-control\" rows=\"3\">#{location}</textarea>'); $('#testsRunningModal').find('.modal-footer').append('#{ok_button}');</script>"
+          "<script>console.log('js_redirect_modal');$('#testsRunningModal').find('.modal-body').html('#{warning_text} <textarea readonly class=\"form-control\" rows=\"3\">#{escape_javascript(location)}</textarea>'); $('#testsRunningModal').find('.modal-footer').append('#{escape_javascript(ok_button)}');</script>"
         end
 
         def js_next_sequence(sequences)

lib/app/views/default.erb

@@ -15,7 +15,7 @@
         <div class="col-8">
           <a href="<%=instance.base_url%><%base_path%>"><img src='/images/icon_logo.png' alt='Inferno Logo Small'></a>
           <span class="server-name">
-            <%=instance.url%>
+            <%=html_escape(instance.url)%>
           </span>
         </div>
         <div class="col-4 server-actions" style='text-align:right;'>
@@ -69,10 +69,10 @@
           The redirect URI for this instance was visited, but no sequence was actively running that was waiting for this event.  Please run the a launch sequence test
           before visiting this redirect url.
         <% elsif error_code == 'no_ehr_launch' %>
-          No actively running launch sequences found for iss <%=params[:iss]%>.
+          No actively running launch sequences found for iss <%=html_escape(params[:iss])%>.
           Please ensure that the EHR launch test is actively running before attempting to launch Inferno from the EHR.
         <% elsif error_code == 'no_state' %>
-          Inferno has detected an issue with the SMART launch. No actively running launch sequences found with a state of <%=params[:state]%>.
+          Inferno has detected an issue with the SMART launch. No actively running launch sequences found with a state of <%=html_escape(params[:state])%>.
           The authorization server is not returning the correct state variable and therefore Inferno cannot identify which server is currently under test.
         <% end %>
       </div>
@@ -344,7 +344,7 @@
                         </small>
 
               </label>
-              <textarea type="text" class="form-control" name="bulk_public_key" id="bulk_public_key" placeholder="" rows="5"><%=instance.bulk_public_key %></textarea>
+              <textarea type="text" class="form-control" name="bulk_public_key" id="bulk_public_key" placeholder="" rows="5"><%=html_escape(instance.bulk_public_key) %></textarea>
           </div>
 
         <div class="form-group" data-requiredby="<%= test_set.variable_required_by(:bulk_private_key).map {|seq| seq.sequence_name}.join(',')%>" data-definedby="" data-prerequisite="bulk_private_key">
@@ -356,7 +356,7 @@
                         </small>
 
               </label>
-              <textarea type="text" class="form-control" name="bulk_private_key" id="bulk_private_key" placeholder="" rows="5"><%=instance.bulk_private_key %></textarea>
+              <textarea type="text" class="form-control" name="bulk_private_key" id="bulk_private_key" placeholder="" rows="5"><%=html_escape(instance.bulk_private_key) %></textarea>
           </div>     
       </div>
       <form method="POST" action="sequence_result">

lib/app/views/guided.erb

@@ -15,7 +15,7 @@
         <div class="col-8">
           <a href="<%=instance.base_url%><%base_path%>"><img src='/images/icon_logo.png' alt='Inferno Logo Small'></a>
           <span class="server-name">
-            <%=instance.url%>
+            <%=html_escape(instance.url)%>
           </span>
         </div>
         <div class="col-4 server-actions">
@@ -70,10 +70,10 @@
           The redirect URI for this instance was visited, but no sequence was actively running that was waiting for this event.  Please run the a launch sequence test
           before visiting this redirect url.
         <% elsif error_code == 'no_ehr_launch' %>
-          No actively running launch sequences found for iss <%=params[:iss]%>.
+          No actively running launch sequences found for iss <%=html_escape(params[:iss])%>.
           Please ensure that the EHR launch test is actively running before attempting to launch Inferno from the EHR.
         <% elsif error_code == 'no_state' %>
-          Inferno has detected an issue with the SMART launch. No actively running launch sequences found with a state of <%=params[:state]%>.
+          Inferno has detected an issue with the SMART launch. No actively running launch sequences found with a state of <%=html_escape(params[:state])%>.
           The authorization server is not returning the correct state variable and therefore Inferno cannot identify which server is currently under test.
         <% end %>
       </div>

lib/app/views/prerequisite_field.erb

@@ -23,7 +23,7 @@
           name="<%=prerequisite.to_s%>"
           id="<%=prerequisite.to_s%>"
           placeholder="<%= defined?(placeholder) && placeholder ? placeholder : ""%>"
-          value="<%=value%>"
+          value="<%=html_escape(value)%>"
           <%if defined?(readonly) && readonly %>readonly<%end%>
           <%if defined?(required) && required %>required<%end%>
           />

lib/app/views/request_details.erb

@@ -17,31 +17,31 @@
      
     <%= rr.request_method.upcase %> &nbsp;
     <%= rr.response_code %> &nbsp;
-    <%= rr.request_url %>
+    <%= html_escape(rr.request_url) %>
     <div class="test-result-details-subheader">Headers</div>
     <ul>
       <% valid_json?(rr.request_headers) && JSON.parse(rr.request_headers).each do |header_name, header_value| %>
-        <li><strong><%=header_name%></strong>: <code><%=header_value%></code><br/></li>
+        <li><strong><%=html_escape(header_name)%></strong>: <code><%=html_escape(header_value)%></code><br/></li>
       <% end %>
     </ul>
 
     <% unless rr.request_payload.nil? %>
       <div class="test-result-details-subheader">Payload</div>
-      <div><pre><code><%=CGI.escapeHTML rr.request_payload%></code></pre></div>
+      <div><pre><code><%=html_escape(rr.request_payload)%></code></pre></div>
     <% end %>
 
     <% if rr.direction == 'outbound' %>
       <div class="test-result-details-header">Response</div>
       <div class="test-result-details-subheader">Headers</div>
       <ul>
         <% valid_json?(rr.response_headers) && JSON.parse(rr.response_headers).each do |header_name, header_value| %>
-          <li><strong><%=header_name%></strong>: <code><%=header_value%></code></li>
+          <li><strong><%=html_escape(header_name)%></strong>: <code><%=html_escape(header_value)%></code></li>
         <% end %>
       </ul>
       <% unless rr.response_body.nil? %>
         <div class="test-result-details-subheader">Body</div>
-        <div><pre><code><%=CGI.escapeHTML rr.response_body%></code></pre></div>
+        <div><pre><code><%=html_escape(rr.response_body)%></code></pre></div>
       <% end %>
     <% end %>
   </div>
-</div>
+</div>