adding new match_link option and protecting against xss attacks via i…

…mg/url tags

README.md

@@ -40,8 +40,8 @@ Rails 2.x has a helper auto_link by default that can do this for you.  For Rails
 At the moment I use a jquery library to display smileys after the page has loaded.  The library I use https://github.com/JangoSteve/jQuery-CSSEmoticons however it would be nice to see a gem that can parse smileys out of text into appropriate html elements with specific tags.  CSS3 font-face anyone?
 
 
-#### XSS (currently under review)
-Since bbcoder outputs html we have to html_safe it for Rails 3 which can cause problems from a security point of view. I use the Sanitize gem to clean the input before bbcoder transforms it into html.  https://github.com/rgrove/sanitize
+#### XSS
+bbcoder will now do a whitelist check against img tags and url tags by default and only allow http/https links.  You can override this by putting in your own configuration if you wish.  If you find any other flaws or holes please report so we can fix.  bbcoder will not sanitize the rest of your input, it will only attempt to whitelist the actual html elements it will generate.
 
 
 #### Newlines
@@ -72,7 +72,15 @@ Configuration Examples
       tag :ol
       tag :li, :parents => [:ol, :ul]
 
-      tag :img, :match => /^.*(png|bmp|jpe?g|gif)$/ do
+      tag :url, :match_link => /^https?:\/\// do
+        if meta.nil? || meta.empty?
+          %(<a href="#{content}">#{content}</a>)
+        else
+          %(<a href="#{meta}">#{content}</a>)
+        end
+      end
+
+      tag :img, :match => /^https?:\/\/.*(png|bmp|jpe?g|gif)$/, :singular => true do
         %(<a href="#{singular? ? meta : content}"><img src="#{singular? ? meta : content}" /></a>)
       end
 
@@ -84,14 +92,6 @@ Configuration Examples
         EOS
       end
 
-      tag :url do
-        if meta.nil? || meta.empty?
-          %(<a href="#{content}">#{content}</a>)
-        else
-          %(<a href="#{meta}">#{content}</a>)
-        end
-      end
-
       remove :spoiler # Removes [spoiler]
     end
 
@@ -102,6 +102,7 @@ Options for #tag
 * :match (regex) -> convert this tag and its content to html only if the content and meta matches the regex (see img tag above for example)
 * :match_meta (regex) -> same as :match except only for meta
 * :match_content (regex) -> same as :match except only for content
+* :match_link (regex) -> special :match case, will match meta if it exists otherwise tries to match content (see url tag for usage)
 * :parents ([symbol]) -> ignore this tag if there is no open tag that matches its parents
 * :singular (true|false) -> use this if the tag does not require an ending tag
 

lib/bbcoder.rb

@@ -48,15 +48,15 @@
     EOS
   end
 
-  tag :url do
+  tag :url, :match_link => /^https?:\/\// do
     if meta.nil? || meta.empty?
       %(<a href="#{content}">#{content}</a>)
     else
       %(<a href="#{meta}">#{content}</a>)
     end
   end
 
-  tag :img, :match => /^.*(png|bmp|jpe?g|gif)$/, :singular => true do
+  tag :img, :match => /^https?:\/\/.*(png|bmp|jpe?g|gif)$/, :singular => true do
     %(<a href="#{singular? ? meta : content}"><img src="#{singular? ? meta : content}" /></a>)
   end
 

lib/bbcoder/tag.rb

@@ -8,7 +8,9 @@ def initialize(name, options = {})
     end
 
     def to_html(meta, content, singularity = false)
-      return self.class.reform(name, meta, content, singularity, true) unless content_valid?(content, singularity) && meta_valid?(meta, singularity)
+      unless content_valid?(content, singularity) && meta_valid?(meta, singularity) && link_valid?(meta, content)
+        return self.class.reform(name, meta, content, singularity, true)
+      end
 
       if options[:block].nil?
         "<#{options[:as]}>#{content}</#{options[:as]}>"
@@ -22,6 +24,19 @@ def to_html(meta, content, singularity = false)
       end
     end
 
+    def link_valid?(meta, content)
+      # only run if we have a :match_link
+      return true if options[:match_link].nil?
+
+      if meta.nil? || meta.empty?
+        return false if content.match(options[:match_link]).nil?
+      else
+        return false if meta.match(options[:match_link]).nil?
+      end
+
+      return true
+    end
+
     def meta_valid?(meta, singularity)
       return true if meta.nil?
 

spec/bbcoder_spec.rb

@@ -151,5 +151,46 @@
       "[img=image.exe]".bbcode_to_html.should == "[img=image.exe]"
     end
   end
+
+  context "with xss attacks" do
+    it "should reject anything other than http/https for url tags" do
+      "[url=javascript:alert('You got hacked!')]hacked[/url]".bbcode_to_html.should == "[url=javascript:alert('You got hacked!')]hacked[/url]"
+      "[url]javascript:alert('You got hacked!')[/url]".bbcode_to_html.should == "[url]javascript:alert('You got hacked!')[/url]"
+
+      '[url=javascript:window.alert("You got hacked!")]click[/url]'.bbcode_to_html.should == '[url=javascript:window.alert("You got hacked!")]click[/url]'
+    end
+
+    it "should reject anything other than http/https for img tags" do
+      "[img=javascript:alert('XSS');jpg]".bbcode_to_html.should == "[img=javascript:alert('XSS');jpg]"
+      "[img]javascript:alert('XSS');png[/img]".bbcode_to_html.should == "[img]javascript:alert('XSS');png[/img]"
+      '[img]javascript:window.alert("You got hacked!")//.jpg[/img]'.bbcode_to_html.should == '[img]javascript:window.alert("You got hacked!")//.jpg[/img]'
+
+      attack = "[img]
+j
+a
+v
+a
+s
+c
+r
+i
+p
+t
+:
+a
+l
+e
+r
+t
+(
+'
+X
+S
+S
+'
+);jpg[/img]"
+        attack.bbcode_to_html.should == attack
+    end
+  end
 end