diff --git a/app/assets/javascripts/discourse/app/components/tag-info.hbs b/app/assets/javascripts/discourse/app/components/tag-info.hbs index bda4cdadc52..bba77c53a35 100644 --- a/app/assets/javascripts/discourse/app/components/tag-info.hbs +++ b/app/assets/javascripts/discourse/app/components/tag-info.hbs @@ -49,7 +49,7 @@ {{/if}}
- {{this.tagInfo.description}} + {{html-safe this.tagInfo.description}}
{{/if}} diff --git a/app/assets/javascripts/discourse/app/lib/render-tag.js b/app/assets/javascripts/discourse/app/lib/render-tag.js index 87d97bebb68..609aa06c412 100644 --- a/app/assets/javascripts/discourse/app/lib/render-tag.js +++ b/app/assets/javascripts/discourse/app/lib/render-tag.js @@ -39,13 +39,17 @@ export function defaultRenderTag(tag, params) { classes.push(params.size); } + // remove all html tags from hover text + const hoverDescription = + params.description && params.description.replace(/<.+?>/g, ""); + let val = "<" + tagName + href + " data-tag-name=" + tag + - (params.description ? ' title="' + escape(params.description) + '" ' : "") + + (params.description ? ' title="' + escape(hoverDescription) + '" ' : "") + " class='" + classes.join(" ") + "'>" + diff --git a/app/assets/javascripts/discourse/tests/unit/lib/render-tag-test.js b/app/assets/javascripts/discourse/tests/unit/lib/render-tag-test.js new file mode 100644 index 00000000000..37ce04598e3 --- /dev/null +++ b/app/assets/javascripts/discourse/tests/unit/lib/render-tag-test.js @@ -0,0 +1,23 @@ +import renderTag from "discourse/lib/render-tag"; +import { module, test } from "qunit"; +import { setupTest } from "ember-qunit"; + +module("Unit | Utility | render-tag", function (hooks) { + setupTest(hooks); + + test("defaultRenderTag", function (assert) { + assert.strictEqual( + renderTag("foo", { description: "foo description" }), + "foo", + "formats tag as link with plain description in hover" + ); + + assert.strictEqual( + renderTag("foo", { + description: 'foo description link', + }), + "foo", + "removes any html tags from description" + ); + }); +}); diff --git a/app/models/tag.rb b/app/models/tag.rb index f3bdedc41a6..a8205d0e88b 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -3,6 +3,7 @@ class Tag < ActiveRecord::Base include Searchable include HasDestroyedWebHook + include HasSanitizableFields self.ignored_columns = [ "topic_count", # TODO(tgxworld): Remove on 1 July 2023 @@ -56,6 +57,8 @@ class Tag < ActiveRecord::Base has_many :synonyms, class_name: "Tag", foreign_key: "target_tag_id", dependent: :destroy has_many :sidebar_section_links, as: :linkable, dependent: :delete_all + before_save :sanitize_description + after_save :index_search after_save :update_synonym_associations @@ -244,6 +247,9 @@ class Tag < ActiveRecord::Base private + def sanitize_description + self.description = sanitize_field(self.description) if description_changed? + end def name_validator errors.add(:name, :invalid) if name.present? && RESERVED_TAGS.include?(self.name.strip.downcase) end diff --git a/spec/models/tag_spec.rb b/spec/models/tag_spec.rb index 9791733edc9..c1100d725e4 100644 --- a/spec/models/tag_spec.rb +++ b/spec/models/tag_spec.rb @@ -382,4 +382,15 @@ RSpec.describe Tag do expect(Tag.topic_count_column(Guardian.new(user))).to eq("staff_topic_count") end end + + describe "description" do + it "uses the HTMLSanitizer to remove unsafe tags and attributes" do + tag.description = + "
hi
discourse" + tag.save! + expect(tag.description.strip).to eq( + "
hi
a=0; discourse", + ) + end + end end diff --git a/spec/system/viewing_sidebar_as_anonymous_user_spec.rb b/spec/system/viewing_sidebar_as_anonymous_user_spec.rb index bac2805e04a..be7bb7f212f 100644 --- a/spec/system/viewing_sidebar_as_anonymous_user_spec.rb +++ b/spec/system/viewing_sidebar_as_anonymous_user_spec.rb @@ -54,7 +54,7 @@ describe "Viewing sidebar as anonymous user", type: :system do expect(sidebar).to have_tags_section expect(sidebar).to have_all_tags_section_link expect(sidebar).to have_tag_section_links([tag3, tag2, tag4, tag5, tag1]) - expect(sidebar).to have_tag_section_link_with_title(tag1, "tag 1 description <script>") + expect(sidebar).to have_tag_section_link_with_title(tag1, "tag 1 description ") end it "should display the site's top tags when `default_navigation_menu_tags` site setting has been set but the tags configured are hidden to the user" do @@ -66,7 +66,7 @@ describe "Viewing sidebar as anonymous user", type: :system do expect(sidebar).to have_tags_section expect(sidebar).to have_all_tags_section_link expect(sidebar).to have_tag_section_links([tag3, tag2, tag4, tag1, tag6]) - expect(sidebar).to have_tag_section_link_with_title(tag1, "tag 1 description <script>") + expect(sidebar).to have_tag_section_link_with_title(tag1, "tag 1 description ") end it "should display the tags configured in `default_navigation_menu_tags` site setting when it has been set" do diff --git a/spec/system/viewing_sidebar_spec.rb b/spec/system/viewing_sidebar_spec.rb index aaeaf75c75e..80d8487f3dc 100644 --- a/spec/system/viewing_sidebar_spec.rb +++ b/spec/system/viewing_sidebar_spec.rb @@ -162,7 +162,7 @@ describe "Viewing sidebar as logged in user", type: :system do expect(sidebar).to have_tags_section expect(sidebar).to have_tag_section_links([tag3, tag2, tag4, tag5, tag1]) expect(sidebar).to have_tag_section_link_with_title(tag3, "tag 3 description") - expect(sidebar).to have_tag_section_link_with_title(tag1, "tag 1 description <script>") + expect(sidebar).to have_tag_section_link_with_title(tag1, "tag 1 description ") expect(sidebar).to have_all_tags_section_link end @@ -177,7 +177,7 @@ describe "Viewing sidebar as logged in user", type: :system do expect(sidebar).to have_tags_section expect(sidebar).to have_tag_section_links([tag1, tag2, tag3]) expect(sidebar).to have_tag_section_link_with_title(tag3, "tag 3 description") - expect(sidebar).to have_tag_section_link_with_title(tag1, "tag 1 description <script>") + expect(sidebar).to have_tag_section_link_with_title(tag1, "tag 1 description ") expect(sidebar).to have_all_tags_section_link end end