From d61e8cb6a6f59b00f0c6a47dfd4f2b5317c7057e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Fri, 18 Oct 2019 22:00:27 +0200
Subject: [PATCH] Use text() instead of html()

Using html() makes it possible to insert <script> tags in the DOM, and
in this case we aren't supposed to be inserting any HTML.

I haven't found a way to focus on a field with Capybara, then add a
character, and focus on another field. So I've manually triggered the
change event in the test.
---
 app/assets/javascripts/banners.js         |  4 ++--
 app/assets/javascripts/markdown_editor.js |  6 +++---
 spec/features/xss_spec.rb                 | 12 ++++++++++++
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/app/assets/javascripts/banners.js b/app/assets/javascripts/banners.js
index d69066c92..9ae732ee4 100644
--- a/app/assets/javascripts/banners.js
+++ b/app/assets/javascripts/banners.js
@@ -4,12 +4,12 @@
     initialize: function() {
       $("[data-js-banner-title]").on({
         change: function() {
-          $("#js-banner-title").html($(this).val());
+          $("#js-banner-title").text($(this).val());
         }
       });
       $("[data-js-banner-description]").on({
         change: function() {
-          $("#js-banner-description").html($(this).val());
+          $("#js-banner-description").text($(this).val());
         }
       });
       $("[name='banner[background_color]']").on({
diff --git a/app/assets/javascripts/markdown_editor.js b/app/assets/javascripts/markdown_editor.js
index c0a587e31..7369c356f 100644
--- a/app/assets/javascripts/markdown_editor.js
+++ b/app/assets/javascripts/markdown_editor.js
@@ -33,10 +33,10 @@
           editor.toggleClass("fullscreen");
           $(".fullscreen-container").toggleClass("medium-8", "medium-12");
           span = $(this).find("span");
-          if (span.html() === span.data("open-text")) {
-            span.html(span.data("closed-text"));
+          if (span.text() === span.data("open-text")) {
+            span.text(span.data("closed-text"));
           } else {
-            span.html(span.data("open-text"));
+            span.text(span.data("open-text"));
           }
           if (editor.hasClass("fullscreen")) {
             App.MarkdownEditor.find_textarea(editor).height($(window).height() - 100);
diff --git a/spec/features/xss_spec.rb b/spec/features/xss_spec.rb
index 71447c976..4852e3396 100644
--- a/spec/features/xss_spec.rb
+++ b/spec/features/xss_spec.rb
@@ -13,6 +13,18 @@ describe "Cross-Site Scripting protection", :js do
     expect(page.text).not_to be_empty
   end
 
+  scenario "edit banner" do
+    banner = create(:banner, title: attack_code)
+
+    login_as(create(:administrator).user)
+    visit edit_admin_banner_path(banner)
+
+    title_id = find_field("Title")[:id]
+    execute_script "document.getElementById('#{title_id}').dispatchEvent(new Event('change'))"
+
+    expect(page.text).not_to be_empty
+  end
+
   scenario "document title" do
     process = create(:legislation_process)
     create(:document, documentable: process, title: attack_code)