github-mirror/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2025-06-06 23:07:28 +08:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

SECURITY: correct XSS on long topic titles

Browse Source
This commit is contained in:
Sam
2018-09-18 08:54:44 +10:00
parent 0e9841b995
commit 7d6b348d0b
2 changed files with 13 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
app/models/topic.rb
View File

@ -221,8 +221,9 @@ class Topic < ActiveRecord::Base
unless skip_callbacks unless skip_callbacks
ensure_topic_has_a_category ensure_topic_has_a_category
end end
if title_changed? if title_changed?
write_attribute :fancy_title, Topic.fancy_title(title) write_attribute(:fancy_title, Topic.fancy_title(title))
end end
if category_id_changed? || new_record? if category_id_changed? || new_record?
@ -346,10 +347,9 @@ class Topic < ActiveRecord::Base
end end
def self.fancy_title(title) def self.fancy_title(title)
escaped = ERB::Util.html_escape(title) return unless escaped = ERB::Util.html_escape(title)
return unless escaped
fancy_title = Emoji.unicode_unescape(HtmlPrettify.render(escaped)) fancy_title = Emoji.unicode_unescape(HtmlPrettify.render(escaped))
fancy_title.length > Topic.max_fancy_title_length ? title : fancy_title fancy_title.length > Topic.max_fancy_title_length ? escaped : fancy_title
end end
def fancy_title def fancy_title

12
spec/models/topic_spec.rb
View File

@ -296,10 +296,17 @@ describe Topic do
expect(topic_image.fancy_title).to eq("Topic with &lt;img src=&lsquo;something&rsquo;&gt; image in its title") expect(topic_image.fancy_title).to eq("Topic with &lt;img src=&lsquo;something&rsquo;&gt; image in its title")
end end
it "always escapes title" do
topic_script.title = topic_script.title + "x" * Topic.max_fancy_title_length
expect(topic_script.fancy_title).to eq(ERB::Util.html_escape(topic_script.title))
# not really needed, but just in case
expect(topic_script.fancy_title).not_to include("<script>")
end
end end
context 'fancy title' do context 'fancy title' do
let(:topic) { Fabricate.build(:topic, title: "\"this topic\" -- has ``fancy stuff''") } let(:topic) { Fabricate.build(:topic, title: %{"this topic" -- has ``fancy stuff''}) }
context 'title_fancy_entities disabled' do context 'title_fancy_entities disabled' do
before do before do
@ -319,7 +326,6 @@ describe Topic do
it "converts the title to have fancy entities and updates" do it "converts the title to have fancy entities and updates" do
expect(topic.fancy_title).to eq("&ldquo;this topic&rdquo; &ndash; has &ldquo;fancy stuff&rdquo;") expect(topic.fancy_title).to eq("&ldquo;this topic&rdquo; &ndash; has &ldquo;fancy stuff&rdquo;")
topic.title = "this is my test hello world... yay" topic.title = "this is my test hello world... yay"
topic.user.save!
topic.save! topic.save!
topic.reload topic.reload
expect(topic.fancy_title).to eq("This is my test hello world&hellip; yay") expect(topic.fancy_title).to eq("This is my test hello world&hellip; yay")
@ -336,7 +342,7 @@ describe Topic do
end end
it "works with long title that results in lots of entities" do it "works with long title that results in lots of entities" do
long_title = "NEW STOCK PICK: PRCT - LAST PICK UP 233%, NNCO.................................................................................................................................................................. ofoum" long_title = "NEW STOCK PICK: PRCT - LAST PICK UP 233%, NNCO#{"." * 150} ofoum"
topic.title = long_title topic.title = long_title
expect { topic.save! }.to_not raise_error expect { topic.save! }.to_not raise_error