IntenseWebs/discourse
3
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2024-11-23 01:16:38 -06:00
Code Issues Packages Projects Releases Wiki Activity

SECURITY: fix XSS in excerpt parser

Browse Source
This commit is contained in:
Sam 2014-07-25 12:15:43 +10:00
parent fbbd4999b6
commit 6e9f5f5584
3 changed files with 37 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
lib/excerpt_parser.rb
View File

@ -23,8 +23,15 @@ class ExcerptParser < Nokogiri::XML::SAX::Document
me.excerpt
end
def escape_attribute(v)
v.gsub("&", "&amp;")
.gsub("\"", "&#34;")
.gsub("<", "&lt;")
.gsub(">", "&gt;")
end
def include_tag(name, attributes)
characters("<#{name} #{attributes.map{|k,v| "#{k}='#{v}'"}.join(' ')}>", false, false, false)
characters("<#{name} #{attributes.map{|k,v| "#{k}=\"#{escape_attribute(v)}\""}.join(' ')}>", false, false, false)
end
def start_element(name, attributes=[])

24
spec/components/pretty_text_spec.rb
View File

@ -75,6 +75,15 @@ describe PrettyText do
describe "Excerpt" do
it "sanitizes attempts to inject invalid attributes" do
spinner = "<a href=\"http://thedailywtf.com/\" data-bbcode=\"' class='fa fa-spin\">WTF</a>"
PrettyText.excerpt(spinner, 20).should match_html spinner
spinner = %q{<a href="http://thedailywtf.com/" title="' class=&quot;fa fa-spin&quot;&gt;&lt;img src='http://thedailywtf.com/Resources/Images/Primary/logo.gif"></a>}
PrettyText.excerpt(spinner, 20).should match_html spinner
end
context "images" do
it "should dump images" do
@ -94,8 +103,8 @@ describe PrettyText do
end
it "should keep spoilers" do
PrettyText.excerpt("<div class='spoiler'><img src='http://cnn.com/a.gif'></div>", 100).should == "<span class='spoiler'>[image]</span>"
PrettyText.excerpt("<span class='spoiler'>spoiler</div>", 100).should == "<span class='spoiler'>spoiler</span>"
PrettyText.excerpt("<div class='spoiler'><img src='http://cnn.com/a.gif'></div>", 100).should match_html "<span class='spoiler'>[image]</span>"
PrettyText.excerpt("<span class='spoiler'>spoiler</div>", 100).should match_html "<span class='spoiler'>spoiler</span>"
end
end
@ -104,7 +113,7 @@ describe PrettyText do
end
it "should preserve links" do
PrettyText.excerpt("<a href='http://cnn.com'>cnn</a>",100).should == "<a href='http://cnn.com'>cnn</a>"
PrettyText.excerpt("<a href='http://cnn.com'>cnn</a>",100).should match_html "<a href='http://cnn.com'>cnn</a>"
end
it "should deal with special keys properly" do
@ -125,15 +134,15 @@ describe PrettyText do
end
it "should not count the surrounds of a link" do
PrettyText.excerpt("<a href='http://cnn.com'>cnn</a>",3).should == "<a href='http://cnn.com'>cnn</a>"
PrettyText.excerpt("<a href='http://cnn.com'>cnn</a>",3).should match_html "<a href='http://cnn.com'>cnn</a>"
end
it "uses an ellipsis instead of html entities if provided with the option" do
PrettyText.excerpt("<a href='http://cnn.com'>cnn</a>", 2, text_entities: true).should == "<a href='http://cnn.com'>cn...</a>"
PrettyText.excerpt("<a href='http://cnn.com'>cnn</a>", 2, text_entities: true).should match_html "<a href='http://cnn.com'>cn...</a>"
end
it "should truncate links" do
PrettyText.excerpt("<a href='http://cnn.com'>cnn</a>",2).should == "<a href='http://cnn.com'>cn&hellip;</a>"
PrettyText.excerpt("<a href='http://cnn.com'>cnn</a>",2).should match_html "<a href='http://cnn.com'>cn&hellip;</a>"
end
it "doesn't extract empty quotes as links" do
@ -294,9 +303,6 @@ describe PrettyText do
PrettyText.cook("**你hello**").should match_html "<p><strong>你hello</strong></p>"
end
it "sanitizes attempts to inject invalid attributes" do
PrettyText.cook("<a href=\"http://thedailywtf.com/\" data-bbcode=\"' class='fa fa-spin\">WTF</a>").should == "<p><a href=\"http://thedailywtf.com/\" rel=\"nofollow\">WTF</a></p>"
end
end
end

28
spec/models/user_profile_spec.rb
View File

@ -57,7 +57,7 @@ describe UserProfile do
end
context 'with a user that has a link in their bio' do
let(:user_profile) { Fabricate.build(:user_profile, bio_raw: "im sissy and i love http://ponycorns.com") }
let(:user_profile) { Fabricate.build(:user_profile, bio_raw: "I love http://discourse.org") }
let(:user) do
user = Fabricate.build(:user, user_profile: user_profile)
user_profile.user = user
@ -66,22 +66,22 @@ describe UserProfile do
let(:created_user) do
user = Fabricate(:user)
user.user_profile.bio_raw = 'im sissy and i love http://ponycorns.com'
user.user_profile.bio_raw = 'I love http://discourse.org'
user.user_profile.save!
user
end
it 'includes the link as nofollow if the user is not new' do
user.user_profile.send(:cook)
expect(user_profile.bio_excerpt).to eq("im sissy and i love <a href='http://ponycorns.com' rel='nofollow'>http://ponycorns.com</a>")
expect(user_profile.bio_processed).to eq("<p>im sissy and i love <a href=\"http://ponycorns.com\" rel=\"nofollow\">http://ponycorns.com</a></p>")
expect(user_profile.bio_excerpt).to match_html("I love <a href='http://discourse.org' rel='nofollow'>http://discourse.org</a>")
expect(user_profile.bio_processed).to match_html("<p>I love <a href=\"http://discourse.org\" rel=\"nofollow\">http://discourse.org</a></p>")
end
it 'removes the link if the user is new' do
user.trust_level = TrustLevel.levels[:newuser]
user_profile.send(:cook)
expect(user_profile.bio_excerpt).to eq("im sissy and i love http://ponycorns.com")
expect(user_profile.bio_processed).to eq("<p>im sissy and i love http://ponycorns.com</p>")
expect(user_profile.bio_excerpt).to match_html("I love http://discourse.org")
expect(user_profile.bio_processed).to eq("<p>I love http://discourse.org</p>")
end
context 'leader_links_no_follow is false' do
@ -90,22 +90,22 @@ describe UserProfile do
it 'includes the link without nofollow if the user is trust level 3 or higher' do
user.trust_level = TrustLevel.levels[:leader]
user_profile.send(:cook)
expect(user_profile.bio_excerpt).to eq("im sissy and i love <a href='http://ponycorns.com'>http://ponycorns.com</a>")
expect(user_profile.bio_processed).to eq("<p>im sissy and i love <a href=\"http://ponycorns.com\">http://ponycorns.com</a></p>")
expect(user_profile.bio_excerpt).to match_html("I love <a href='http://discourse.org'>http://discourse.org</a>")
expect(user_profile.bio_processed).to match_html("<p>I love <a href=\"http://discourse.org\">http://discourse.org</a></p>")
end
it 'removes nofollow from links in bio when trust level is increased' do
created_user.change_trust_level!(:leader)
expect(created_user.user_profile.bio_excerpt).to eq("im sissy and i love <a href='http://ponycorns.com'>http://ponycorns.com</a>")
expect(created_user.user_profile.bio_processed).to eq("<p>im sissy and i love <a href=\"http://ponycorns.com\">http://ponycorns.com</a></p>")
expect(created_user.user_profile.bio_excerpt).to match_html("I love <a href='http://discourse.org'>http://discourse.org</a>")
expect(created_user.user_profile.bio_processed).to match_html("<p>I love <a href=\"http://discourse.org\">http://discourse.org</a></p>")
end
it 'adds nofollow to links in bio when trust level is decreased' do
created_user.trust_level = TrustLevel.levels[:leader]
created_user.save
created_user.change_trust_level!(:regular)
expect(created_user.user_profile.bio_excerpt).to eq("im sissy and i love <a href='http://ponycorns.com' rel='nofollow'>http://ponycorns.com</a>")
expect(created_user.user_profile.bio_processed).to eq("<p>im sissy and i love <a href=\"http://ponycorns.com\" rel=\"nofollow\">http://ponycorns.com</a></p>")
expect(created_user.user_profile.bio_excerpt).to match_html("I love <a href='http://discourse.org' rel='nofollow'>http://discourse.org</a>")
expect(created_user.user_profile.bio_processed).to match_html("<p>I love <a href=\"http://discourse.org\" rel=\"nofollow\">http://discourse.org</a></p>")
end
end
@ -115,8 +115,8 @@ describe UserProfile do
it 'includes the link with nofollow if the user is trust level 3 or higher' do
user.trust_level = TrustLevel.levels[:leader]
user_profile.send(:cook)
expect(user_profile.bio_excerpt).to eq("im sissy and i love <a href='http://ponycorns.com' rel='nofollow'>http://ponycorns.com</a>")
expect(user_profile.bio_processed).to eq("<p>im sissy and i love <a href=\"http://ponycorns.com\" rel=\"nofollow\">http://ponycorns.com</a></p>")
expect(user_profile.bio_excerpt).to match_html("I love <a href='http://discourse.org' rel='nofollow'>http://discourse.org</a>")
expect(user_profile.bio_processed).to match_html("<p>I love <a href=\"http://discourse.org\" rel=\"nofollow\">http://discourse.org</a></p>")
end
end
end