From a3d3f1c536fbb98471f91a81d995187f86ee5fb4 Mon Sep 17 00:00:00 2001
From: Jacques Distler <distler@golem.ph.utexas.edu>
Date: Sun, 23 Sep 2007 19:30:39 +0000
Subject: [PATCH] Fix XSS vulnerabilities in chunk-handling

---
 lib/chunks/category.rb            |  5 +++--
 lib/chunks/chunk.rb               |  8 ++++++++
 lib/chunks/nowiki.rb              |  6 +++++-
 lib/sanitize.rb                   | 25 +++++++++++++++++++++++++
 test/unit/chunks/category_test.rb |  8 ++++++++
 test/unit/chunks/nowiki_test.rb   |  6 ++++++
 6 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/lib/chunks/category.rb b/lib/chunks/category.rb
index d08d8636..b35bc484 100644
--- a/lib/chunks/category.rb
+++ b/lib/chunks/category.rb
@@ -16,7 +16,8 @@ class Category < Chunk::Abstract
 def initialize(match_data, content)
     super(match_data, content)
     @hidden = match_data[1]
-    @list = match_data[2].split(',').map { |c| c.strip }
+    @list = match_data[2].split(',').map { |c| c.to_s.is_utf8? ? html_escape(c. strip) : nil }
+    @list.compact!
     @unmask_text = ''
     if @hidden
       @unmask_text = ''
@@ -28,6 +29,6 @@ def initialize(match_data, content)
 
   # TODO move presentation of page metadata to controller/view
   def url(category)
-    %{<a class="category_link" href="../list/?category=#{category}">#{category}</a>}
+    %{<a class="category_link" href="../list/#{category}">#{category}</a>}
   end
 end
diff --git a/lib/chunks/chunk.rb b/lib/chunks/chunk.rb
index 18de7d0c..46382c76 100644
--- a/lib/chunks/chunk.rb
+++ b/lib/chunks/chunk.rb
@@ -74,6 +74,14 @@ module Chunk
       @content.delete_chunk(self)
     end
 
+    def  html_escape(string)
+      string.gsub( /&/, "&amp;" ).
+             gsub( /</, "&lt;" ).
+             gsub( />/, "&gt;" ).
+             gsub( /'/, "&#39;" ).
+             gsub( /"/, "&quot;" )
+    end
+
   end
 
 end
diff --git a/lib/chunks/nowiki.rb b/lib/chunks/nowiki.rb
index ef99ec0b..8bad35e6 100644
--- a/lib/chunks/nowiki.rb
+++ b/lib/chunks/nowiki.rb
@@ -13,6 +13,10 @@ require 'chunks/chunk'
 #
 # Author: Mark Reid <mark at threewordslong dot com>
 # Created: 8th June 2004
+
+require 'sanitize'
+include Sanitize
+
 class NoWiki < Chunk::Abstract
 
   NOWIKI_PATTERN = Regexp.new('<nowiki>(.*?)</nowiki>', Regexp::MULTILINE)
@@ -22,7 +26,7 @@ class NoWiki < Chunk::Abstract
 
   def initialize(match_data, content)
     super
-    @plain_text = @unmask_text = match_data[1]
+    @plain_text = @unmask_text = sanitize_html(match_data[1])
   end
 
 end
diff --git a/lib/sanitize.rb b/lib/sanitize.rb
index f4c479ec..92adf6ad 100644
--- a/lib/sanitize.rb
+++ b/lib/sanitize.rb
@@ -203,3 +203,28 @@ module Sanitize
           style = clean.join(' ')
       end
 end      
+
+# Some useful additions to the String class
+
+class String
+
+# Check whether a string is valid utf-8
+#
+# :call-seq:
+#    string.is_utf8?    -> boolean
+#
+# returns true if the sequence of bytes in string is valid utf-8
+   def is_utf8?
+     self =~  /^(
+         [\x09\x0A\x0D\x20-\x7E]            # ASCII
+       | [\xC2-\xDF][\x80-\xBF]             # non-overlong 2-byte
+       |  \xE0[\xA0-\xBF][\x80-\xBF]        # excluding overlongs
+       | [\xE1-\xEC\xEE\xEF][\x80-\xBF]{2}  # straight 3-byte
+       |  \xED[\x80-\x9F][\x80-\xBF]        # excluding surrogates
+       |  \xF0[\x90-\xBF][\x80-\xBF]{2}     # planes 1-3
+       | [\xF1-\xF3][\x80-\xBF]{3}          # planes 4-15
+       |  \xF4[\x80-\x8F][\x80-\xBF]{2}     # plane 16
+     )*$/x;
+   end
+
+end
diff --git a/test/unit/chunks/category_test.rb b/test/unit/chunks/category_test.rb
index 6bc7627f..0244ee65 100755
--- a/test/unit/chunks/category_test.rb
+++ b/test/unit/chunks/category_test.rb
@@ -19,4 +19,12 @@ class CategoryTest < Test::Unit::TestCase
 	)
   end
 
+  def test_multiple_categories_sanitized
+       match(Category, 'category: test, multiple,<span>a & b</span> <script>alert("XSS!");</script>', :list => ['test', 'multiple', '&lt;span&gt;a &amp; b&lt;/span&gt; &lt;script&gt;alert(&quot;XSS!&quot;);&lt;/script&gt;'], :hidden => nil
+)
+       match(Category, 'category : chunk test , multi category,<span>a & b</span> <script>alert("XSS!");</script>', 
+               :list => ['chunk test','multi category','&lt;span&gt;a &amp; b&lt;/span&gt; &lt;script&gt;alert(&quot;XSS!&quot;);&lt;/script&gt;'], :hidden => nil
+       )
+  end
+
 end
diff --git a/test/unit/chunks/nowiki_test.rb b/test/unit/chunks/nowiki_test.rb
index 8af5a645..fdbced54 100755
--- a/test/unit/chunks/nowiki_test.rb
+++ b/test/unit/chunks/nowiki_test.rb
@@ -12,4 +12,10 @@ class NoWikiTest < Test::Unit::TestCase
 	)
   end
 
+  def test_sanitized_nowiki
+       match(NoWiki, 'This sentence contains <nowiki><span>a b</span> <script>alert("XSS!");</script></nowiki>. Do not touch!',
+               :plain_text => '<span>a b</span> &lt;script&gt;alert("XSS!");&lt;/script&gt;'
+       )
+  end
+
 end