From 7f1bfc6bd7aba0e9804c0b65d10b5ec46df4e592 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javi=20Mart=C3=ADn?= <javim@elretirao.net>
Date: Sat, 19 Oct 2019 02:20:59 +0200
Subject: [PATCH] Avoid using html() to set languages description

The jQuery html() function does not filter <script> tags, so if somehow
an attacker introduced a <script> in the translation, we would be
vulnerable to a XSS attack.

Note using $.parseHTML wouldn't solve the problem, since it doesn't
filter attributes in image tags.

Since changing the text of the part which doesn't have the count wasn't
very clean, I've added another <span> tag for the part with the
description, and so we can use jQuery's text() function to replace it.
---
 app/assets/javascripts/globalize.js |  7 ++++---
 config/locales/en/general.yml       |  6 +++---
 config/locales/es/general.yml       |  6 +++---
 spec/features/xss_spec.rb           | 10 ++++++++++
 4 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/app/assets/javascripts/globalize.js b/app/assets/javascripts/globalize.js
index a7757f520..c653bf627 100644
--- a/app/assets/javascripts/globalize.js
+++ b/app/assets/javascripts/globalize.js
@@ -77,9 +77,10 @@
     update_description: function() {
       var count, description;
       count = App.Globalize.enabled_locales().length;
-      description = App.Globalize.language_description(count);
-      $(".js-languages-description").html(description);
-      $(".js-languages-count").text(count);
+      description = $(App.Globalize.language_description(count)).filter(".description").text();
+
+      $(".js-languages-description .description").text(description);
+      $(".js-languages-description .count").text(count);
     },
     language_description: function(count) {
       switch (count) {
diff --git a/config/locales/en/general.yml b/config/locales/en/general.yml
index b08677ab4..44b7c9358 100644
--- a/config/locales/en/general.yml
+++ b/config/locales/en/general.yml
@@ -806,9 +806,9 @@ en:
       remove_language: Remove language
       add_language: Add language
       languages_in_use:
-        zero: "<span class='js-languages-count'>0</span> languages in use"
-        one: "<span class='js-languages-count'>1</span> language in use"
-        other: "<span class='js-languages-count'>%{count}</span> languages in use"
+        zero: "<span class='count'>0</span> <span class='description'>languages in use</span>"
+        one: "<span class='count'>1</span> <span class='description'>language in use</span>"
+        other: "<span class='count'>%{count}</span> <span class='description'>languages in use</span>"
   social:
     facebook: "%{org} Facebook"
     twitter: "%{org} Twitter"
diff --git a/config/locales/es/general.yml b/config/locales/es/general.yml
index 4b2391528..791d98cb1 100644
--- a/config/locales/es/general.yml
+++ b/config/locales/es/general.yml
@@ -803,9 +803,9 @@ es:
       remove_language: Eliminar idioma
       add_language: Añadir idioma
       languages_in_use:
-        zero: "<span class='js-languages-count'>0</span> idiomas en uso"
-        one: "<span class='js-languages-count'>1</span> idioma en uso"
-        other: "<span class='js-languages-count'>%{count}</span> idiomas en uso"
+        zero: "<span class='count'>0</span> <span class='description'>idiomas en uso</span>"
+        one: "<span class='count'>1</span> <span class='description'> idioma en uso</span>"
+        other: "<span class='count'>%{count}</span> <span class='description'>idiomas en uso</span>"
   social:
     facebook: "Facebook de %{org}"
     twitter: "Twitter de %{org}"
diff --git a/spec/features/xss_spec.rb b/spec/features/xss_spec.rb
index 4852e3396..32114b89c 100644
--- a/spec/features/xss_spec.rb
+++ b/spec/features/xss_spec.rb
@@ -61,6 +61,16 @@ describe "Cross-Site Scripting protection", :js do
     expect(page.text).not_to be_empty
   end
 
+  scenario "languages in use" do
+    I18nContent.create(key: "shared.translations.languages_in_use", value: attack_code)
+
+    login_as(create(:administrator).user)
+    visit edit_admin_budget_path(create(:budget))
+    click_link "Remove language"
+
+    expect(page.text).not_to be_empty
+  end
+
   scenario "proposal actions in dashboard" do
     proposal = create(:proposal)