Merge pull request #239 from epimorphics/spike/2024-09-security-patch

Primarily Security Patch Integration

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 This app allows the user to explore HMLR price-paid open linked data.
 
+## Changelog
+
 ## 1.7.8 - 2024-08
 
 - (Jon) Implemented improved boilerplate metrics integration to offer analysis
@@ -18,8 +20,11 @@ This app allows the user to explore HMLR price-paid open linked data.
   inline
 - (Jon) Updated the `lr_common_styles` gem to the latest 1.9.6 patch release.
 
-## 1.7.7 - 2024-08
+## 1.7.7 - 2024-09
 
+- (Jon) Updated search query processing and rendering to santise supplied input
+  for HTML output and resolve discovered XSS vulnerability in displayed results
+  [GH-236](https://github.com/epimorphics/hmlr-linked-data/issues/236)
 - (Dan) Fixes the bug search results not displaying
   [232](https://github.com/epimorphics/ppd-explorer/issues/232)
 - (Dan) Adds page titles to donwload page and error page. Improves code dryness

app/controllers/search_controller.rb

@@ -54,7 +54,7 @@ def render_error_page(err, message, status, template = 'ppd/error')
     @message = message
 
     # log the error with as much detail as possible in development to aid in resolving the issue
-    message = "#{err.class.name} error #{uuid} ::: #{message} ::: #{err.class}" if Rails.env.development?
+    @message = "#{err.class.name} error: #{message}" if Rails.env.development?
 
     # Keep it simple silly in production!
     Rails.logger.error message

app/models/search_aspect.rb

@@ -29,13 +29,15 @@ def preference_value_as_regex(preferences)
   end
 
   def search_term(_value, preferences)
-    val = preference_value(preferences)
-    SearchTerm.new(key, "#{key_as_label} matches '#{val}'", val)
+    val = sanitised(preference_value(preferences))
+    label = "#{key_as_label} matches '#{val}'"
+
+    SearchTerm.new(key, label, val)
   end
 
   # Sanitise input and convert to Lucene expression
   def text_index_term(preferences)
-    terms = sanitise_punctuation(preference_value(preferences))
+    terms = sanitise_punctuation(sanitised(preference_value(preferences)))
             .split
             .reject { |token| LUCENE_KEYWORDS.include?(token.downcase) }
             .reject(&:empty?)
@@ -45,9 +47,12 @@ def text_index_term(preferences)
             "Sorry, '#{preference_value(preferences)}' is not a permissible search term"
     end
 
-    terms
-      .join(' AND ')
-      .gsub(/\A(.*)\Z/, '( \1 )')
+    terms.join(' AND ').gsub(/\A(.*)\Z/, '( \1 )')
+  end
+
+  # Sanitises a string for HTML output (using Rails' built-in sanitizer)
+  def sanitised(val)
+    Rails::Html::FullSanitizer.new.sanitize(val)
   end
 
   private

app/models/search_term.rb

@@ -23,7 +23,8 @@ def form_name
 
   def form_value
     # @values ? @values.join(",") : @value
-    @value
+    # remove any HTML tags from the value
+    Rails::Html::FullSanitizer.new.sanitize(@value)
   end
 
   def label
@@ -44,7 +45,9 @@ def truncated_label_term
   end
 
   def clean_label_term
+    # remove any HTML tags from the label term
+    term = Rails::Html::FullSanitizer.new.sanitize(@label_term)
     # previously we added a profanity filter here, but it was decided not to keep that feature
-    @label_term
+    term
   end
 end

app/models/user_preferences.rb

@@ -83,9 +83,14 @@ def unlimited?
   private
 
   # Remove any non-allowlisted parameters, or params with empty values
+  # (e.g. empty strings, empty arrays)
   def sanitise!
     @params.keep_if do |_k, v|
-      !v.to_s.strip.empty?
+      if v.is_a?(Array)
+        v.any? { |x| !x.to_s.strip.empty? }
+      else
+        !v.to_s.strip.empty?
+      end
     end
   end
 

app/views/ppd/index.html.haml

@@ -27,37 +27,37 @@
       .form-group
         %label.col-sm-4.control-label{ for: "paon" } Building name or number
         .col-sm-8
-          %input#paon.form-control{ type: "text", autocomplete: "on", name: "paon", value: @preferences.param( :paon ), placeholder: "For example: Rose Cottage or 17" }
+          %input#paon.form-control{ type: "text", autocomplete: "on", name: "paon", value: sanitize(@preferences.param( :paon )), placeholder: "For example: Rose Cottage or 17" }
 
       .form-group
         %label.col-sm-4.control-label{ for: "street" } Street
         .col-sm-8
-          %input#street.form-control{ type: "text", autocomplete: "street-address", name: "street", value: @preferences.param( :street ), placeholder: "Use the full name, or just part of it e.g: Harbour Road or Colston" }
+          %input#street.form-control{ type: "text", autocomplete: "street-address", name: "street", value: sanitize(@preferences.param( :street )), placeholder: "Use the full name, or just part of it e.g: Harbour Road or Colston" }
 
       .form-group
         %label.col-sm-4.control-label{ for: "town" } Town or city
         .col-sm-8
-          %input#town.form-control{ type: "text", autocomplete: "address-level2", name: "town", value: @preferences.param( :town ), placeholder: "For example: Plymouth" }
+          %input#town.form-control{ type: "text", autocomplete: "address-level2", name: "town", value: sanitize(@preferences.param( :town )), placeholder: "For example: Plymouth" }
 
       .form-group
         %label.col-sm-4.control-label{ for: "district" } District
         .col-sm-8
-          %input#district.form-control{ type: "text", autocomplete: "address-level3", name: "district", value: @preferences.param( :district ), placeholder: "For example: City of Westminster" }
+          %input#district.form-control{ type: "text", autocomplete: "address-level3", name: "district", value: sanitize(@preferences.param( :district )), placeholder: "For example: City of Westminster" }
 
       .form-group
         %label.col-sm-4.control-label{ for: "county" } County
         .col-sm-8
-          %input#county.form-control{ type: "text", autocomplete: "county", name: "county", value: @preferences.param( :county ), placeholder: "For example: Devon" }
+          %input#county.form-control{ type: "text", autocomplete: "county", name: "county", value: sanitize(@preferences.param( :county )), placeholder: "For example: Devon" }
 
       .form-group
         %label.col-sm-4.control-label{ for: "locality" } Locality
         .col-sm-8
-          %input#locality.form-control{ type: "text", autocomplete: "on", name: "locality", value: @preferences.param( :locality ), placeholder: "For example: Thurloxton" }
+          %input#locality.form-control{ type: "text", autocomplete: "on", name: "locality", value: sanitize(@preferences.param( :locality )), placeholder: "For example: Thurloxton" }
 
       .form-group
         %label.col-sm-4.control-label{ for: "postcode" } Postcode
         .col-sm-8
-          %input#postcode.form-control{ type: "text", autocomplete: "postal-code", name: "postcode", value: @preferences.param( :postcode ), placeholder: "Use a full postcode, or just the first group e.g. PL6 5WS or PL6." }
+          %input#postcode.form-control{ type: "text", autocomplete: "postal-code", name: "postcode", value: sanitize(@preferences.param( :postcode )), placeholder: "Use a full postcode, or just the first group e.g. PL6 5WS or PL6." }
 
       %fieldset.form-group
         %legend.control-legend.col-sm-4.control-label{ for: "ptype" } Property type
@@ -142,7 +142,7 @@
               .input-group
                 %span.input-group-addon
                   £
-                %input#min_price.form-control{ type: "text", name: :min_price, value: @preferences.param( :min_price ) }
+                %input#min_price.form-control{ type: "text", name: :min_price, value: sanitize(@preferences.param( :min_price )) }
             .col-sm-8.hidden.validation-warning
               %p.bg-warning
                 Please enter a valid numerical value, or leave blank
@@ -154,7 +154,7 @@
               .input-group
                 %span.input-group-addon
                   £
-                %input#max_price.form-control{ type: "text", name: :max_price, value: @preferences.param( :max_price ) }
+                %input#max_price.form-control{ type: "text", name: :max_price, value: sanitize(@preferences.param( :max_price )) }
             .col-sm-8.hidden.validation-warning
               %p.bg-warning
                 Please enter a valid numerical value, or leave blank

app/views/search/_search_results.html.haml

@@ -12,11 +12,13 @@
     #{sr.summarise} by searching for:
   %ul.search-terms.list.list-bullet
     - @preferences.each_search_term do |search_term|
+      - clean_label = sanitize(search_term.label)
+      - clean_value = search_term.value.is_a?(Date) ?  search_term.value : sanitize(search_term.value)
       %li
         %label
-          = raw(search_term.label)
-          - st_path = @preferences.as_path( :search, {}, {search_term.name => search_term.value } )
-          %a.search-term{ href: st_path, 'aria-label' => "Remove search term #{raw(search_term.label)}" }
+          = raw(clean_label)
+          - st_path = @preferences.as_path( :search, {}, {search_term.name => clean_value } )
+          %a.search-term{ href: st_path, 'aria-label' => "Remove search term #{raw(clean_label)}" }
             %i.fa.fa-times-circle.fa-lg
 
   %p.search-selection