added the ability to add safe html tags to input (i.e., images)
author<mako@atdot.cc> <>
Mon, 11 Feb 2008 15:49:30 +0000 (10:49 -0500)
committer<mako@atdot.cc> <>
Mon, 11 Feb 2008 15:49:30 +0000 (10:49 -0500)
- added new white_list_plugin
- changed it so that it is being used for candidate names in quickvotes

16 files changed:
app/helpers/application_helper.rb
app/views/common/_methodinfo_ssd.rhtml
app/views/common/_pref_tables.rhtml
app/views/common/_result.rhtml
app/views/common/_sortable_vote.rhtml
app/views/quickvote/_approval_table.rhtml
app/views/quickvote/_candidate_list.rhtml
app/views/quickvote/results.rhtml
app/views/quickvote/thanks.rhtml
app/views/voter/details.rhtml
test/functional/quickvote_controller_test.rb
vendor/plugins/white_list/README [new file with mode: 0644]
vendor/plugins/white_list/Rakefile [new file with mode: 0644]
vendor/plugins/white_list/init.rb [new file with mode: 0644]
vendor/plugins/white_list/lib/white_list_helper.rb [new file with mode: 0644]
vendor/plugins/white_list/test/white_list_test.rb [new file with mode: 0644]

index 4752f008facadb66fb89eec6e9753adc10134817..22a7940eb213fec161352153800c9c6948c8b43c 100644 (file)
@@ -1,4 +1,3 @@
 # Methods added to this helper will be available to all templates in the application.
 module ApplicationHelper
 end
index 0c8dc9560d5f02d37e3e0fe851328a97334393f7..5ec8b89e6c24ced67b5b0ebd4eefdf25d83c6522 100644 (file)
@@ -3,7 +3,7 @@ preference (from most preferred to least preferred):</p>
 
 <ol>
 <% @election.ssd_result.ranked_candidates.each do |place|  %>
-  <li><%= h(place.collect {|c| @names[c].capitalize}.join( " <em>and</em> " )) %>
+  <li><%= white_list place.collect {|c| @names[c].capitalize}.join( " <em>and</em> " ) %>
       <%= "<strong>(TIE)</strong>" if place.length > 1 %></li>
 <% end %>
 </ol>
index 7701985ec49b5e5a3c7a1ce965c410242e046c75..567873b594652cb624a32ed3faeb6d6d503a84b0 100644 (file)
@@ -14,13 +14,13 @@ top of the left column.</p>
   <tr>
        <td></td>
        <% candidates.each do |candidate| -%>
-         <th><%=h @names[candidate] -%></th>
+         <th><%= white_list(@names[candidate]) -%></th>
   <% end -%>
  </tr>
 
 <% candidates.each do |winner| -%>
   <tr>
-       <th><%=h @names[winner] %></th>
+       <th><%= white_list(@names[winner]) %></th>
   <% candidates.each do |loser| -%> 
     <% if winner == loser -%>
       <td> -- </td>
@@ -46,10 +46,10 @@ parenthesis.</p>
 <table class="preftable">
   <% candidates.each do |victor| %>
   <tr>
-    <th><%=h @names[victor] %></th>
+    <th><%= white_list(@names[victor]) %></th>
        <% victories[victor].keys.each do |loser| %>
        <% margin = victories[victor][loser]%>
-       <td><%=h @names[loser] %> 
+       <td><%= white_list(@names[loser]) %> 
            <% if margin == 0%>
                  Tied!
                <% else -%>
index b0a3a8522363fcdd8dde2eb70eb7c261ca7e6cb2..ea0ec40820c118091ec28f79410d18cca19d2ed8 100644 (file)
@@ -2,9 +2,9 @@
 <p class="winner_text">
 <% if result.winner? and result.winners.length == 1 -%>
   The winner is:
-     <strong><%=h @candidates[result.winner].name.capitalize %></strong>
+     <strong><%= white_list(@candidates[result.winner].name.capitalize) %></strong>
 <% elsif result.winner? and result.winners.length > 1 %>
-  There was a tie. The winners are: <strong><%=h( result.winners.collect {|w| @candidates[w].to_s.capitalize}.join(", ") )%></strong>
+  There was a tie. The winners are: <strong><%= white_list(result.winners.collect {|w| @candidates[w].to_s.capitalize}.join(", ") )%></strong>
 <% else %>
   <p>There is no winner using this method. </strong>
 <% end %>
index 6876966f0f44dbf6f24aae5aa996c49acfa3e4f7..f026c4216378f65dfcd832b8699ce9beb6c5bb08 100644 (file)
@@ -2,7 +2,7 @@
 <ol id="rankings-list">
   <% for ranking in @voter.vote.rankings %>
     <li class="moveable" id="ranking_<%= ranking.candidate.id %>">
-      <%=h ranking.candidate.name.capitalize %></li>
+      <%= white_list(ranking.candidate.name.capitalize) %></li>
   <% end %>
 </ol>
 </div>
index 85185eedf3925aaf41072720c01a175b853a9930..1c16d1d724ae97bfc42012e9d7304106708b83cf 100644 (file)
@@ -2,7 +2,7 @@
   <tr>
        <td>Candidate</td>
          <% @election.approval_result.points.keys.sort.each do |candidate| %>
-               <th><%=h @names[candidate] %></th>
+               <th><%= white_list(@names[candidate]) %></th>
          <% end -%>
   </tr>
        
@@ -12,4 +12,4 @@
                <td><%= points %></td>
        <% end -%>
   </tr>
-</table>
\ No newline at end of file
+</table>
index 4ec3db8b24014e9be500974187a93e2fa2132367..9ccb3b1ecb53d2d83011982e1214c2f2ca9b908f 100644 (file)
@@ -2,7 +2,7 @@
 <% if flash[:candidate_names] %>
   <ul>
   <% for cand in flash[:candidate_names] %>
-    <li><%=h cand.capitalize %></li>
+    <li><%= white_list(cand.capitalize) %></li>
   <% end %>
   </ul>
 <% end %>
index 8d8bf5f15d88b9413d534e6930ae8bf5314107d6..78e74e89191103180e249cece519720474be61b5 100644 (file)
@@ -23,7 +23,7 @@
 
 <ol>
   <% for candidate in @election.candidates.sort %>
-    <li><%=h candidate.name.capitalize %></li>
+    <li><%= white_list(candidate.name.capitalize) %></li>
   <% end %>
 </ol>
 
index bf98ef89ec1619e37b075dcece0ff6d29dbc5976..d6ed8e0afeded815eb021b842223de8f95141681 100644 (file)
@@ -8,7 +8,7 @@ preferences:</p>
 
 <ol>
   <% for rank in @voter.vote.rankings.sort %>
-    <li><%=h rank.candidate.name.capitalize %> </li>
+    <li><%= white_list(rank.candidate.name.capitalize) %> </li>
   <% end %>
 </ol>
 
index 584c4084dca817df12a1c7d7b7c8436f30837bd2..cf73f4189c7ae52865b0e163585bb46baa1aeac6 100644 (file)
@@ -4,7 +4,7 @@
 </div>
 
 <p>This page contains information useful for auditing elections and
-verify that votes were tabulated correctly.</p>
+verifying that votes were tabulated correctly.</p>
 
 <p>The following invididuals (in random order) voted in this
 election:</p>
@@ -20,15 +20,17 @@ election:</p>
 <p>The column marked <em>Verification Token</em> lists tokens that were
 given to voters at the time of voting. Voters can check to see that the
 vote that corresponds to their token was recorded correctly. The column
-marks "vote" lists the candidates in order of the voter's preference. To
-read these votes, please refer to the key below.</p>
+marked <em>Vote</em> lists the candidates in order of the voter's
+preference. To read these votes, refer to the key below.</p>
 
 <table class="preftable">
 <tr>
+<th></th>
 <th>Verification Token</th>
 <th>Vote</th>
-<%- @votes.each do |vote| -%>
+<%- @votes.each_with_index do |vote, i| -%>
 <tr>
+<td><%= i + 1 %></td>
 <td><%= vote.token %></td><td><%= vote.votestring%></td>
 </tr>
 <%- end -%>
index 60ddb1b86f036e66362074f755914f90dc9cfa72..e116c0d7ada00a5335eb7044eca262dd6f350c4e 100644 (file)
@@ -115,24 +115,40 @@ class QuickvoteControllerTest < Test::Unit::TestCase
     post :confirm, { 'ident' => 'variable', 'rankings-list' => votes.sort_by {rand} }
     assert_redirected_to :controller => 'quickvote', :ident => 'variable'
   end
+
   def test_display_tainted_quickvote
+    # create quickvote with tainted data
     test_create_quickvote
     qv=QuickVote.ident_to_quickvote('variable')
     qv.description="<object>foo</object>"
-    qv.candidate_names = ["<object>foo", "bar<object>", "<foobar>"]
+    qv.candidate_names = ["<object>foo", "bar<object>", "<foobar>",
+                          '<img src="foo" alt="bar" />']
     qv.save!
+
+    # display the vote/index page and check for bad tags and the ability
+    # to make an image tag
     get :index, { 'ident' => 'variable' }
     assert_response :success
     assert_no_tag :tag => "object"
     assert_no_tag :tag => "foobar"
+    assert_tag :tag => "img",
+               :parent => { :tag => "li", :attributes => { :class => "moveable" } }
+
+    # actually vote
     votes = QuickVote.ident_to_quickvote('variable').candidates.collect { |c| c.id}
     post :confirm, { 'ident' => 'variable', 'rankings-list' => votes.sort_by {rand} }
+
+    # check for bad/good tags
     assert_template('quickvote/thanks')
     assert_no_tag :tag => "object"
     assert_no_tag :tag => "foobar"
+    assert_tag :tag => "img", :parent => { :tag => "li" }
+
+    # get the results page and check for good/bad tags
     get :results, { 'ident' => 'variable' }
     assert_response :success
     assert_no_tag :tag => "object"
     assert_no_tag :tag => "foobar"
+    assert_tag :tag => "img", :parent => { :tag => "li" }
   end
 end
diff --git a/vendor/plugins/white_list/README b/vendor/plugins/white_list/README
new file mode 100644 (file)
index 0000000..84bb1ee
--- /dev/null
@@ -0,0 +1,29 @@
+WhiteList
+=========
+
+This White Listing helper will html encode all tags and strip all attributes that aren't specifically allowed.  
+It also strips href/src tags with invalid protocols, like javascript: especially.  It does its best to counter any
+tricks that hackers may use, like throwing in unicode/ascii/hex values to get past the javascript: filters.  Check out
+the extensive test suite.
+
+  <%= white_list @article.body %>
+
+You can add or remove tags/attributes if you want to customize it a bit.
+
+Add table tags
+  
+  WhiteListHelper.tags.merge %w(table td th)
+
+Remove tags
+  
+  WhiteListHelper.tags.delete 'div'
+
+Change allowed attributes
+
+  WhiteListHelper.attributes.merge %w(id class style)
+
+white_list accepts a block for custom tag escaping.  Shown below is the default block that white_list uses if none is given.
+The block is called for all bad tags, and every text node.  node is an instance of HTML::Node (either HTML::Tag or HTML::Text).  
+bad is nil for text nodes inside good tags, or is the tag name of the bad tag.  
+
+  <%= white_list(@article.body) { |node, bad| white_listed_bad_tags.include?(bad) ? nil : node.to_s.gsub(/</, '&lt;') } %>
\ No newline at end of file
diff --git a/vendor/plugins/white_list/Rakefile b/vendor/plugins/white_list/Rakefile
new file mode 100644 (file)
index 0000000..ce067be
--- /dev/null
@@ -0,0 +1,22 @@
+require 'rake'
+require 'rake/testtask'
+require 'rake/rdoctask'
+
+desc 'Default: run unit tests.'
+task :default => :test
+
+desc 'Test the white_list plugin.'
+Rake::TestTask.new(:test) do |t|
+  t.libs << 'lib'
+  t.pattern = 'test/**/*_test.rb'
+  t.verbose = true
+end
+
+desc 'Generate documentation for the white_list plugin.'
+Rake::RDocTask.new(:rdoc) do |rdoc|
+  rdoc.rdoc_dir = 'rdoc'
+  rdoc.title    = 'WhiteList'
+  rdoc.options << '--line-numbers' << '--inline-source'
+  rdoc.rdoc_files.include('README')
+  rdoc.rdoc_files.include('lib/**/*.rb')
+end
diff --git a/vendor/plugins/white_list/init.rb b/vendor/plugins/white_list/init.rb
new file mode 100644 (file)
index 0000000..92bf8ea
--- /dev/null
@@ -0,0 +1,2 @@
+require 'white_list_helper'
+ActionView::Base.send :include, WhiteListHelper
\ No newline at end of file
diff --git a/vendor/plugins/white_list/lib/white_list_helper.rb b/vendor/plugins/white_list/lib/white_list_helper.rb
new file mode 100644 (file)
index 0000000..52bbb91
--- /dev/null
@@ -0,0 +1,97 @@
+module WhiteListHelper
+  @@protocol_attributes = Set.new %w(src href)
+  @@protocol_separator  = /:|(&#0*58)|(&#x70)|(%|&#37;)3A/
+  mattr_reader :protocol_attributes, :protocol_separator
+
+  def self.contains_bad_protocols?(white_listed_protocols, value)
+    value =~ protocol_separator && !white_listed_protocols.include?(value.split(protocol_separator).first)
+  end
+
+  klass = class << self; self; end
+  klass_methods = []
+  inst_methods  = []
+  [:bad_tags, :tags, :attributes, :protocols].each do |attr|
+    # Add class methods to the module itself
+    klass_methods << <<-EOS
+      def #{attr}=(value) @@#{attr} = Set.new(value) end
+      def #{attr}() @@#{attr} end
+    EOS
+    
+    # prefix the instance methods with white_listed_*
+    inst_methods << "def white_listed_#{attr}() ::WhiteListHelper.#{attr} end"
+  end
+  
+  klass.class_eval klass_methods.join("\n"), __FILE__, __LINE__
+  class_eval       inst_methods.join("\n"),  __FILE__, __LINE__
+
+  # This White Listing helper will html encode all tags and strip all attributes that aren't specifically allowed.  
+  # It also strips href/src tags with invalid protocols, like javascript: especially.  It does its best to counter any
+  # tricks that hackers may use, like throwing in unicode/ascii/hex values to get past the javascript: filters.  Check out
+  # the extensive test suite.
+  #
+  #   <%= white_list @article.body %>
+  # 
+  # You can add or remove tags/attributes if you want to customize it a bit.
+  # 
+  # Add table tags
+  #   
+  #   WhiteListHelper.tags.merge %w(table td th)
+  # 
+  # Remove tags
+  #   
+  #   WhiteListHelper.tags.delete 'div'
+  # 
+  # Change allowed attributes
+  # 
+  #   WhiteListHelper.attributes.merge %w(id class style)
+  # 
+  # white_list accepts a block for custom tag escaping.  Shown below is the default block that white_list uses if none is given.
+  # The block is called for all bad tags, and every text node.  node is an instance of HTML::Node (either HTML::Tag or HTML::Text).  
+  # bad is nil for text nodes inside good tags, or is the tag name of the bad tag.  
+  # 
+  #   <%= white_list(@article.body) { |node, bad| white_listed_bad_tags.include?(bad) ? nil : node.to_s.gsub(/</, '&lt;') } %>
+  #
+  def white_list(html, options = {}, &block)
+    return html if html.blank? || !html.include?('<')
+    attrs   = Set.new(options[:attributes]).merge(white_listed_attributes)
+    tags    = Set.new(options[:tags]      ).merge(white_listed_tags)
+    block ||= lambda { |node, bad| white_listed_bad_tags.include?(bad) ? nil : node.to_s.gsub(/</, '&lt;') }
+    returning [] do |new_text|
+      tokenizer = HTML::Tokenizer.new(html)
+      bad       = nil
+      while token = tokenizer.next
+        node = HTML::Node.parse(nil, 0, 0, token, false)
+        new_text << case node
+          when HTML::Tag
+            node.attributes.keys.each do |attr_name|
+              value = node.attributes[attr_name].to_s
+              if !attrs.include?(attr_name) || (protocol_attributes.include?(attr_name) && contains_bad_protocols?(value))
+                node.attributes.delete(attr_name)
+              else
+                node.attributes[attr_name] = CGI::escapeHTML(value)
+              end
+            end if node.attributes
+            if tags.include?(node.name)
+              bad = nil
+              node
+            else
+              bad = node.name
+              block.call node, bad
+            end
+          else
+            block.call node, bad
+        end
+      end
+    end.join
+  end
+  
+  protected
+    def contains_bad_protocols?(value)
+      WhiteListHelper.contains_bad_protocols?(white_listed_protocols, value)
+    end
+end
+
+WhiteListHelper.bad_tags   = %w(script)
+WhiteListHelper.tags       = %w(strong em b i p code pre tt output samp kbd var sub sup dfn cite big small address hr br div span h1 h2 h3 h4 h5 h6 ul ol li dt dd abbr acronym a img blockquote del ins fieldset legend)
+WhiteListHelper.attributes = %w(href src width height alt cite datetime title class)
+WhiteListHelper.protocols  = %w(ed2k ftp http https irc mailto news gopher nntp telnet webcal xmpp callto feed)
\ No newline at end of file
diff --git a/vendor/plugins/white_list/test/white_list_test.rb b/vendor/plugins/white_list/test/white_list_test.rb
new file mode 100644 (file)
index 0000000..c5fda43
--- /dev/null
@@ -0,0 +1,132 @@
+require 'test/unit'
+require File.expand_path(File.join(File.dirname(__FILE__), '../../../../config/environment.rb'))
+
+class WhiteListTest < Test::Unit::TestCase
+  include WhiteListHelper
+  public :contains_bad_protocols?
+
+  WhiteListHelper.tags.each do |tag_name|
+    define_method "test_should_allow_#{tag_name}_tag" do
+      assert_white_listed "start <#{tag_name} title=\"1\" name=\"foo\">foo <bad>bar</bad> baz</#{tag_name}> end", %(start <#{tag_name} title="1">foo &lt;bad>bar&lt;/bad> baz</#{tag_name}> end)
+    end
+  end
+
+  def test_should_allow_anchors
+    assert_white_listed %(<a href="foo" onclick="bar"><script>baz</script></a>), %(<a href="foo"></a>)
+  end
+
+  %w(src width height alt).each do |img_attr|
+    define_method "test_should_allow_image_#{img_attr}_attribute" do
+      assert_white_listed %(<img #{img_attr}="foo" onclick="bar" />), %(<img #{img_attr}="foo" />)
+    end
+  end
+
+  def test_should_handle_non_html
+    assert_white_listed 'abc'
+  end
+
+  def test_should_handle_blank_text
+    assert_white_listed nil
+    assert_white_listed ''
+  end
+
+  def test_should_allow_custom_tags
+    text = "<u>foo</u>"
+    assert_equal(text, white_list(text, :tags => %w(u)))
+  end
+
+  def test_should_allow_custom_tags_with_attributes
+    text = %(<fieldset foo="bar">foo</fieldset>)
+    assert_equal(text, white_list(text, :attributes => ['foo']))
+  end
+
+  [%w(img src), %w(a href)].each do |(tag, attr)|
+    define_method "test_should_strip_#{attr}_attribute_in_#{tag}_with_bad_protocols" do
+      assert_white_listed %(<#{tag} #{attr}="javascript:bang" title="1">boo</#{tag}>), %(<#{tag} title="1">boo</#{tag}>)
+    end
+  end
+
+  def test_should_flag_bad_protocols
+    %w(about chrome data disk hcp help javascript livescript lynxcgi lynxexec ms-help ms-its mhtml mocha opera res resource shell vbscript view-source vnd.ms.radio wysiwyg).each do |proto|
+      assert contains_bad_protocols?("#{proto}://bad")
+    end
+  end
+
+  def test_should_accept_good_protocols
+    WhiteListHelper.protocols.each do |proto|
+      assert !contains_bad_protocols?("#{proto}://good")
+    end
+  end
+
+  def test_should_reject_hex_codes_in_protocol
+    assert contains_bad_protocols?("%6A%61%76%61%73%63%72%69%70%74%3A%61%6C%65%72%74%28%22%58%53%53%22%29")
+    assert_white_listed %(<a href="&#37;6A&#37;61&#37;76&#37;61&#37;73&#37;63&#37;72&#37;69&#37;70&#37;74&#37;3A&#37;61&#37;6C&#37;65&#37;72&#37;74&#37;28&#37;22&#37;58&#37;53&#37;53&#37;22&#37;29">1</a>), "<a>1</a>"
+  end
+
+  def test_should_block_script_tag
+    assert_white_listed %(<SCRIPT\nSRC=http://ha.ckers.org/xss.js></SCRIPT>), ""
+  end
+
+  [%(<IMG SRC="javascript:alert('XSS');">), 
+   %(<IMG SRC=javascript:alert('XSS')>), 
+   %(<IMG SRC=JaVaScRiPt:alert('XSS')>), 
+   %(<IMG """><SCRIPT>alert("XSS")</SCRIPT>">),
+   %(<IMG SRC=javascript:alert(&quot;XSS&quot;)>),
+   %(<IMG SRC=javascript:alert(String.fromCharCode(88,83,83))>),
+   %(<IMG SRC=&#106;&#97;&#118;&#97;&#115;&#99;&#114;&#105;&#112;&#116;&#58;&#97;&#108;&#101;&#114;&#116;&#40;&#39;&#88;&#83;&#83;&#39;&#41;>),
+   %(<IMG SRC=&#0000106&#0000097&#0000118&#0000097&#0000115&#0000099&#0000114&#0000105&#0000112&#0000116&#0000058&#0000097&#0000108&#0000101&#0000114&#0000116&#0000040&#0000039&#0000088&#0000083&#0000083&#0000039&#0000041>),
+   %(<IMG SRC=&#x6A&#x61&#x76&#x61&#x73&#x63&#x72&#x69&#x70&#x74&#x3A&#x61&#x6C&#x65&#x72&#x74&#x28&#x27&#x58&#x53&#x53&#x27&#x29>),
+   %(<IMG SRC="jav\tascript:alert('XSS');">),
+   %(<IMG SRC="jav&#x09;ascript:alert('XSS');">),
+   %(<IMG SRC="jav&#x0A;ascript:alert('XSS');">),
+   %(<IMG SRC="jav&#x0D;ascript:alert('XSS');">),
+   %(<IMG SRC=" &#14;  javascript:alert('XSS');">),
+   %(<IMG SRC=`javascript:alert("RSnake says, 'XSS'")`>)].each_with_index do |img_hack, i|
+    define_method "test_should_not_fall_for_xss_image_hack_#{i}" do
+      assert_white_listed img_hack, "<img>"
+    end
+  end
+  
+  def test_should_sanitize_tag_broken_up_by_null
+    assert_white_listed %(<SCR\0IPT>alert(\"XSS\")</SCR\0IPT>), "&lt;scr>alert(\"XSS\")&lt;/scr>"
+  end
+  
+  def test_should_sanitize_invalid_script_tag
+    assert_white_listed %(<SCRIPT/XSS SRC="http://ha.ckers.org/xss.js"></SCRIPT>), ""
+  end
+  
+  def test_should_sanitize_script_tag_with_multiple_open_brackets
+    assert_white_listed %(<<SCRIPT>alert("XSS");//<</SCRIPT>), "&lt;"
+    assert_white_listed %(<iframe src=http://ha.ckers.org/scriptlet.html\n<), %(&lt;iframe src="http:" />&lt;)
+  end
+  
+  def test_should_sanitize_unclosed_script
+    assert_white_listed %(<SCRIPT SRC=http://ha.ckers.org/xss.js?<B>), "<b>"
+  end
+  
+  def test_should_sanitize_half_open_scripts
+    assert_white_listed %(<IMG SRC="javascript:alert('XSS')"), "<img>"
+  end
+  
+  def test_should_not_fall_for_ridiculous_hack
+    img_hack = %(<IMG\nSRC\n=\n"\nj\na\nv\na\ns\nc\nr\ni\np\nt\n:\na\nl\ne\nr\nt\n(\n'\nX\nS\nS\n'\n)\n"\n>)
+    assert_white_listed img_hack, "<img>"
+  end
+
+  def test_should_allow_custom_block
+    html = %(<SCRIPT type="javascript">foo</SCRIPT><img>blah</img><blink>blah</blink>)
+    safe = white_list html do |node, bad|
+      bad == 'script' ? nil : node
+    end
+    assert_equal "<img>blah</img><blink>blah</blink>", safe
+  end
+
+  def test_should_sanitize_attributes
+    assert_white_listed %(<SPAN title="'><script>alert()</script>">blah</SPAN>), %(<span title="'&gt;&lt;script&gt;alert()&lt;/script&gt;">blah</span>)
+  end
+
+  protected
+    def assert_white_listed(text, expected = nil)
+      assert_equal((expected || text), white_list(text))
+    end
+end

Benjamin Mako Hill || Want to submit a patch?