From d407bcab367e75d86cfc731e72b8c6c1b3ef4c3f Mon Sep 17 00:00:00 2001
From: Jarek Radosz <jradosz@gmail.com>
Date: Tue, 1 Oct 2019 18:04:40 +0200
Subject: [PATCH] FIX: Correctly escape category description text (#8107)

* FIX: Correctly escape category description text

This bug has been introduced in db14e10943aeb87f3a2e06f02ca788986f039077.

* Remove unnecessary `html_safe`

`Theme.lookup_field` already returns html-safe strings: https://github.com/discourse/discourse/blob/7ad338e3e6dee158cd704e7e62395bf5df835f7f/app/models/theme.rb#L237-L242

* Rename `description` where it's acutally `descriptionText`
---
 .../javascripts/discourse/helpers/category-link.js.es6     | 4 ++--
 .../javascripts/select-kit/components/category-row.js.es6  | 6 +++---
 app/helpers/application_helper.rb                          | 3 ---
 app/models/category.rb                                     | 3 ++-
 app/views/users/omniauth_callbacks/complete.html.erb       | 2 +-
 spec/components/category_badge_spec.rb                     | 7 +++++++
 spec/models/category_spec.rb                               | 2 +-
 7 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/app/assets/javascripts/discourse/helpers/category-link.js.es6 b/app/assets/javascripts/discourse/helpers/category-link.js.es6
index 3b62c82170d..7f02dae2804 100644
--- a/app/assets/javascripts/discourse/helpers/category-link.js.es6
+++ b/app/assets/javascripts/discourse/helpers/category-link.js.es6
@@ -75,7 +75,7 @@ export function categoryLinkHTML(category, options) {
 registerUnbound("category-link", categoryLinkHTML);
 
 function defaultCategoryLinkRenderer(category, opts) {
-  let description = get(category, "description_text");
+  let descriptionText = get(category, "description_text");
   let restricted = get(category, "read_restricted");
   let url = opts.url
     ? opts.url
@@ -121,7 +121,7 @@ function defaultCategoryLinkRenderer(category, opts) {
     'data-drop-close="true" class="' +
     classNames +
     '"' +
-    (description ? 'title="' + escapeExpression(description) + '" ' : "") +
+    (descriptionText ? 'title="' + descriptionText + '" ' : "") +
     ">";
 
   let categoryName = escapeExpression(get(category, "name"));
diff --git a/app/assets/javascripts/select-kit/components/category-row.js.es6 b/app/assets/javascripts/select-kit/components/category-row.js.es6
index c0c3657b1c6..29153014229 100644
--- a/app/assets/javascripts/select-kit/components/category-row.js.es6
+++ b/app/assets/javascripts/select-kit/components/category-row.js.es6
@@ -84,9 +84,9 @@ export default SelectKitRowComponent.extend({
   },
 
   @computed("category.description_text")
-  descriptionText(description) {
-    if (description) {
-      return this._formatCategoryDescription(description);
+  descriptionText(descriptionText) {
+    if (descriptionText) {
+      return this._formatCategoryDescription(descriptionText);
     }
   },
 
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index 9d7b6e4e0a1..34f92d79857 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -437,17 +437,14 @@ module ApplicationHelper
 
   def theme_lookup(name)
     Theme.lookup_field(theme_ids, mobile_view? ? :mobile : :desktop, name)
-      &.html_safe
   end
 
   def theme_translations_lookup
     Theme.lookup_field(theme_ids, :translations, I18n.locale)
-      &.html_safe
   end
 
   def theme_js_lookup
     Theme.lookup_field(theme_ids, :extra_js, nil)
-      &.html_safe
   end
 
   def discourse_stylesheet_link_tag(name, opts = {})
diff --git a/app/models/category.rb b/app/models/category.rb
index 6009164df89..af7b0731669 100644
--- a/app/models/category.rb
+++ b/app/models/category.rb
@@ -261,7 +261,8 @@ class Category < ActiveRecord::Base
 
     @@cache ||= LruRedux::ThreadSafeCache.new(1000)
     @@cache.getset(self.description) do
-      Nokogiri::HTML.fragment(self.description).text.strip.html_safe
+      text = Nokogiri::HTML.fragment(self.description).text.strip
+      Rack::Utils.escape_html(text).html_safe
     end
   end
 
diff --git a/app/views/users/omniauth_callbacks/complete.html.erb b/app/views/users/omniauth_callbacks/complete.html.erb
index 5eb0ab9db92..dc6a0b0d4d0 100644
--- a/app/views/users/omniauth_callbacks/complete.html.erb
+++ b/app/views/users/omniauth_callbacks/complete.html.erb
@@ -26,7 +26,7 @@
   <div class="dialog">
     <p>
       <%=t "login.auth_complete" %>
-      <a href="<%= Discourse.base_url.html_safe %>?authComplete=true"><%= t("login.click_to_continue") %></a>
+      <a href="<%= Discourse.base_url %>?authComplete=true"><%= t("login.click_to_continue") %></a>
     </p>
   </div>
 </body>
diff --git a/spec/components/category_badge_spec.rb b/spec/components/category_badge_spec.rb
index a5636c9df59..76eb8c23d65 100644
--- a/spec/components/category_badge_spec.rb
+++ b/spec/components/category_badge_spec.rb
@@ -14,4 +14,11 @@ describe CategoryBadge do
     expect(html).to include(ERB::Util.html_escape("<b>name</b>"))
     expect(html).to include("title='title'")
   end
+
+  it "escapes code block contents" do
+    c = Fabricate(:category, description: '<code>\' &lt;b id="x"&gt;</code>')
+    html = CategoryBadge.html_for(c)
+
+    expect(html).to include("title='&#x27; &lt;b id=&quot;x&quot;&gt;'")
+  end
 end
diff --git a/spec/models/category_spec.rb b/spec/models/category_spec.rb
index c660a6bd6b3..66944834299 100644
--- a/spec/models/category_spec.rb
+++ b/spec/models/category_spec.rb
@@ -357,7 +357,7 @@ describe Category do
       c = Category.new
       expect(c.description_text).to be_nil
       c.description = "&lt;hello <a>test</a>."
-      expect(c.description_text).to eq("<hello test.")
+      expect(c.description_text).to eq("&lt;hello test.")
     end
   end