diff --git a/CHANGELOG.rdoc b/CHANGELOG.rdoc index 0ed44f95..f9d1b6c5 100644 --- a/CHANGELOG.rdoc +++ b/CHANGELOG.rdoc @@ -1,5 +1,11 @@ = Changelog +== 0.4.4 (2010-02-01) + +Bug fixes: + + * Loofah::XssFoliate was not properly escaping HTML entities when implicitly scrubbing a string attribute. GH #17 + == 0.4.3 (2010-01-29) Enhancements: diff --git a/lib/loofah/xss_foliate.rb b/lib/loofah/xss_foliate.rb index 9a0b0f99..6f3893a7 100644 --- a/lib/loofah/xss_foliate.rb +++ b/lib/loofah/xss_foliate.rb @@ -180,7 +180,7 @@ def xss_foliate_fields # :nodoc: # :text if we're here fragment = Loofah.scrub_fragment(value, :strip) - self[field] = fragment.nil? ? "" : fragment.text + self[field] = fragment.nil? ? "" : fragment.to_s end end diff --git a/test/test_ad_hoc.rb b/test/test_ad_hoc.rb index 35ce688a..9beb57eb 100644 --- a/test/test_ad_hoc.rb +++ b/test/test_ad_hoc.rb @@ -251,7 +251,7 @@ def test_removal_of_all_tags What's up doc? HTML stripped = Loofah.scrub_document(html, :prune).text - assert_equal "What's up doc?".strip, stripped.strip + assert_equal %Q(What\'s up doc?).strip, stripped.strip end def test_dont_remove_whitespace @@ -268,5 +268,4 @@ def test_removal_of_entities html = "
this is < that "&" the other > boo'ya
" assert_equal 'this is < that "&" the other > boo\'ya', Loofah.scrub_document(html, :prune).text end - end diff --git a/test/test_xss_foliate.rb b/test/test_xss_foliate.rb index 900f8fd5..08fe278e 100644 --- a/test/test_xss_foliate.rb +++ b/test/test_xss_foliate.rb @@ -56,7 +56,7 @@ def new_post(overrides={}) end context "when passed a symbol" do - should "do the right thing" do + should "calls the right scrubber" do assert_nothing_raised(ArgumentError) { Post.xss_foliate :prune => :plain_text } Loofah.expects(:scrub_fragment).with(HTML_STRING, :strip).once Loofah.expects(:scrub_fragment).with(PLAIN_TEXT, :prune).once @@ -65,7 +65,7 @@ def new_post(overrides={}) end context "when passed an array of symbols" do - should "do the right thing" do + should "calls the right scrubbers" do assert_nothing_raised(ArgumentError) { Post.xss_foliate :prune => [:plain_text, :html_string] } @@ -76,7 +76,7 @@ def new_post(overrides={}) end context "when passed a string" do - should "do the right thing" do + should "calls the right scrubber" do assert_nothing_raised(ArgumentError) { Post.xss_foliate :prune => 'plain_text' } Loofah.expects(:scrub_fragment).with(HTML_STRING, :strip).once Loofah.expects(:scrub_fragment).with(PLAIN_TEXT, :prune).once @@ -85,7 +85,7 @@ def new_post(overrides={}) end context "when passed an array of strings" do - should "do the right thing" do + should "calls the right scrubbers" do assert_nothing_raised(ArgumentError) { Post.xss_foliate :prune => ['plain_text', 'html_string'] } @@ -107,7 +107,7 @@ def new_post(overrides={}) Loofah.expects(:scrub_fragment).with(HTML_STRING, :strip).once.returns(mock_doc) Loofah.expects(:scrub_fragment).with(PLAIN_TEXT, :strip).once.returns(mock_doc) Loofah.expects(:scrub_fragment).with(INTEGER_VALUE, :strip).never - mock_doc.expects(:text).twice + mock_doc.expects(:to_s).twice assert new_post.valid? end end @@ -118,9 +118,11 @@ def new_post(overrides={}) end should "not scrub omitted field" do - Loofah.expects(:scrub_fragment).with(HTML_STRING, :strip).once + mock_doc = mock + Loofah.expects(:scrub_fragment).with(HTML_STRING, :strip).once.returns(mock_doc) Loofah.expects(:scrub_fragment).with(PLAIN_TEXT, :strip).never Loofah.expects(:scrub_fragment).with(INTEGER_VALUE, :strip).never + mock_doc.expects(:to_s).once assert new_post.valid? end end @@ -132,9 +134,11 @@ def new_post(overrides={}) end should "not that field appropriately" do - Loofah.expects(:scrub_fragment).with(HTML_STRING, :strip).once - Loofah.expects(:scrub_fragment).with(PLAIN_TEXT, method).once + mock_doc = mock + Loofah.expects(:scrub_fragment).with(HTML_STRING, :strip).once.returns(mock_doc) + Loofah.expects(:scrub_fragment).with(PLAIN_TEXT, method).once.returns(mock_doc) Loofah.expects(:scrub_fragment).with(INTEGER_VALUE, :strip).never + mock_doc.expects(:to_s).twice assert new_post.valid? end end @@ -167,5 +171,18 @@ def new_post(overrides={}) end end + context "given an XSS attempt" do + setup do + Post.xss_foliate :strip => :html_string + end + + should "escape html entities" do + hackattack = "<script>alert('evil')</script>" + post = new_post :html_string => hackattack, :plain_text => hackattack + post.valid? + assert_equal "<script>alert('evil')</script>", post.html_string + assert_equal "<script>alert('evil')</script>", post.plain_text + end + end end end