diff --git a/CHANGELOG.md b/CHANGELOG.md index 220ce2d..9031ca5 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index a46e3c7..dc3337f 100644 --- a/app/controllers/search_controller.rb +++ b/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 diff --git a/app/models/search_aspect.rb b/app/models/search_aspect.rb index 2999052..7bded93 100644 --- a/app/models/search_aspect.rb +++ b/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 diff --git a/app/models/search_term.rb b/app/models/search_term.rb index fe3dddf..91f9465 100644 --- a/app/models/search_term.rb +++ b/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 diff --git a/app/models/user_preferences.rb b/app/models/user_preferences.rb index 882ac60..bde662b 100644 --- a/app/models/user_preferences.rb +++ b/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 diff --git a/app/views/ppd/index.html.haml b/app/views/ppd/index.html.haml index f422980..3135288 100644 --- a/app/views/ppd/index.html.haml +++ b/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 diff --git a/app/views/search/_search_results.html.haml b/app/views/search/_search_results.html.haml index 7093d62..a123298 100644 --- a/app/views/search/_search_results.html.haml +++ b/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