From 9eee79f218f1f0a0503792b1949d1eaeba1ad0b7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Sun, 6 Oct 2019 15:10:26 +0200
Subject: [PATCH] Sanitize markdown output

We were using the markdown renderer with the `filter_html` option set to
false, so we weren't removing hypothetical `<script>` tags.
---
 app/helpers/application_helper.rb                  | 3 ++-
 app/views/legislation/draft_versions/show.html.erb | 4 ++--
 spec/features/xss_spec.rb                          | 8 ++++++++
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 40dd1073b..1033d0420 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -32,7 +32,8 @@ module ApplicationHelper
       strikethrough:      true,
       superscript:        true
     }
-    Redcarpet::Markdown.new(renderer, extensions).render(text).html_safe
+
+    sanitize(Redcarpet::Markdown.new(renderer, extensions).render(text))
   end
 
   def author_of?(authorable, user)
diff --git a/app/views/legislation/draft_versions/show.html.erb b/app/views/legislation/draft_versions/show.html.erb
index 80ad1f886..8259c74fa 100644
--- a/app/views/legislation/draft_versions/show.html.erb
+++ b/app/views/legislation/draft_versions/show.html.erb
@@ -49,7 +49,7 @@
 
         <div data-sticky-container>
           <div data-sticky data-anchor="sticky-panel" class="draft-index sticky" data-tree-navigator>
-            <%= @draft_version.toc_html.html_safe %>
+            <%= sanitize(@draft_version.toc_html) %>
           </div>
         </div>
       </div>
@@ -66,7 +66,7 @@
                    data-legislation-annotatable-base-url="<%= legislation_process_draft_version_path(@process, @draft_version) %>"
                    data-legislation-open-phase="<%= @process.allegations_phase.open? %>">
           <% end %>
-            <%= @draft_version.body_html.html_safe %>
+            <%= sanitize(@draft_version.body_html) %>
           </section>
         </div>
       </div>
diff --git a/spec/features/xss_spec.rb b/spec/features/xss_spec.rb
index 482b5bd5d..adbd5b3b3 100644
--- a/spec/features/xss_spec.rb
+++ b/spec/features/xss_spec.rb
@@ -78,4 +78,12 @@ describe "Cross-Site Scripting protection", :js do
 
     expect(page.text).not_to be_empty
   end
+
+  scenario "markdown conversion" do
+    process = create(:legislation_process, description: attack_code)
+
+    visit legislation_process_path(process)
+
+    expect(page.text).not_to be_empty
+  end
 end