From 015613a1408cd95aba79ac9bc80cf34a38da7fa3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Sat, 9 Apr 2022 02:07:05 +0200
Subject: [PATCH] Fix HTML injection in search results summary

In commit f374478dd, we enabled the possibility to use HTML in the
search results translations in order to add a <strong> tag to these
results. However, that meant we were also allowing HTML tags inside the
search term itself, and so it was possible to inject HTML on the page.

Stripping the HTML tags solves the issue.

Note the issue wasn't a high severity issue because tags such as
`<script>` weren't allowed since we were using the `sanitize` helper.
---
 .../search_results_summary_component.rb       |  6 +++-
 app/views/management/proposals/index.html.erb |  6 ++--
 spec/system/html_injection_spec.rb            | 36 +++++++++++++++++++
 3 files changed, 44 insertions(+), 4 deletions(-)
 create mode 100644 spec/system/html_injection_spec.rb

diff --git a/app/components/shared/search_results_summary_component.rb b/app/components/shared/search_results_summary_component.rb
index cc95c691a..dc5fc8814 100644
--- a/app/components/shared/search_results_summary_component.rb
+++ b/app/components/shared/search_results_summary_component.rb
@@ -10,6 +10,10 @@ class Shared::SearchResultsSummaryComponent < ApplicationComponent
   private
 
     def summary
-      sanitize(t("proposals.index.search_results", count: results.size, search_term: search_terms))
+      sanitize(t(
+        "proposals.index.search_results",
+        count: results.size,
+        search_term: strip_tags(search_terms)
+      ))
     end
 end
diff --git a/app/views/management/proposals/index.html.erb b/app/views/management/proposals/index.html.erb
index 549fc8f48..da89c1dbf 100644
--- a/app/views/management/proposals/index.html.erb
+++ b/app/views/management/proposals/index.html.erb
@@ -10,9 +10,9 @@
         <% if @search_terms %>
           <h3>
             <%= page_entries_info @proposals %>
-            <%= sanitize(
-              t("proposals.index.search_results", count: @proposals.size, search_term: @search_terms)
-            ) %>
+            <%= sanitize(t("proposals.index.search_results",
+                           count: @proposals.size,
+                           search_term: strip_tags(@search_terms))) %>
           </h3>
         <% end %>
 
diff --git a/spec/system/html_injection_spec.rb b/spec/system/html_injection_spec.rb
new file mode 100644
index 000000000..5399354c9
--- /dev/null
+++ b/spec/system/html_injection_spec.rb
@@ -0,0 +1,36 @@
+require "rails_helper"
+
+describe "HTML injection protection" do
+  let(:attack_code) { "<a href='/evil'>Click me</a>" }
+
+  scenario "debates search" do
+    visit debates_path(search: attack_code)
+
+    expect(page).to have_content "containing the term 'Click me'"
+    expect(page).not_to have_link "Click me"
+  end
+
+  scenario "investments search" do
+    visit budget_investments_path(budget_id: create(:budget), search: attack_code)
+
+    expect(page).to have_content "containing the term 'Click me'"
+    expect(page).not_to have_link "Click me"
+  end
+
+  scenario "proposals search" do
+    visit proposals_path(search: attack_code)
+
+    expect(page).to have_content "containing the term 'Click me'"
+    expect(page).not_to have_link "Click me"
+  end
+
+  scenario "proposals search in the management area" do
+    login_managed_user(create(:user, :level_two))
+    login_as_manager
+
+    visit management_proposals_path(search: attack_code)
+
+    expect(page).to have_content "containing the term 'Click me'"
+    expect(page).not_to have_link "Click me"
+  end
+end