Expand Regex DoS check to include String#match and #match? coercion

lib/brakeman/checks/check_regex_dos.rb

@@ -11,13 +11,21 @@ class Brakeman::CheckRegexDoS < Brakeman::BaseCheck
     ]
   }
 
-  @description = "Searches regexes including user input"
+  COERCES_STRING_TO_REGEX = [
+    :match,
+    :match?
+  ]
+
+  @description = "Searches regexes and coerced regexes including user input"
 
   #Process calls
   def run_check
     Brakeman.debug "Finding dynamic regexes"
     calls = tracker.find_call :method => [:brakeman_regex_interp]
 
+    calls.concat(tracker.find_call(:methods => COERCES_STRING_TO_REGEX)
+                        .select { |call| string_or_modified_string?(call[:target]) })
+
     Brakeman.debug "Processing dynamic regexes"
     calls.each do |call|
       process_result call
@@ -44,7 +52,11 @@ def process_result result
       end
 
       if match
-        message = msg(msg_input(match), " used in regular expression")
+        if result[:method] == :brakeman_regex_interp
+          message = msg(msg_input(match), " used in regular expression")
+        else
+          message = msg(msg_input(match), " used in string to regular expression coercion by ", msg_code(call.method), " method")
+        end
 
         warn :result => result,
           :warning_type => "Denial of Service",
@@ -57,6 +69,14 @@ def process_result result
     end
   end
 
+  def string_or_modified_string?(sexp)
+    if call? sexp
+      string_or_modified_string? sexp.target
+    else
+      string? sexp
+    end
+  end
+
   def process_call(exp)
     if escape_methods = ESCAPES[exp.target]
       if escape_methods.include? exp.method

test/apps/rails2/app/controllers/other_controller.rb

@@ -82,6 +82,18 @@ def test_unescaped_regex
     /#{Rack::Utils.escape_html(params[:regex])}/
   end
 
+  def test_regex_match
+    "haystack".match(params[:regex])
+  end
+
+  def test_regex_match?
+    "haystack".match?(params[:regex])
+  end
+
+  def test_modified_string_match
+    "haystack".downcase.match(params[:regex].downcase)
+  end
+
   def test_intern
     x = params[:x].intern
 

test/tests/markdown_output.rb

@@ -10,6 +10,6 @@ def setup
   end
 
   def test_reported_warnings
-    assert_equal 175, @@report.lines.to_a.count, "Did you add vulnerabilities to the Rails 2 app? Update this test please!"
+    assert_equal 178, @@report.lines.to_a.count, "Did you add vulnerabilities to the Rails 2 app? Update this test please!"
   end
 end

test/tests/rails2.rb

@@ -14,7 +14,7 @@ def expected
       :controller => 1,
       :model => 4,
       :template => 47,
-      :generic => 60 }
+      :generic => 63 }
   end
 
   def report
@@ -1378,7 +1378,7 @@ def test_unsafe_symbol_creation_3
   def test_unsafe_symbol_creation_4
     assert_warning :type => :warning,
       :warning_type => "Denial of Service",
-      :line => 86,
+      :line => 98,
       :message => /^Symbol\ conversion\ from\ unsafe\ string\ in pa/,
       :confidence => 0,
       :file => /other_controller\.rb/,
@@ -1388,7 +1388,7 @@ def test_unsafe_symbol_creation_4
   def test_unsafe_symbol_creation_5
     assert_warning :type => :warning,
       :warning_type => "Denial of Service",
-      :line => 88,
+      :line => 100,
       :message => /^Symbol\ conversion\ from\ unsafe\ string\ in pa/,
       :confidence => 1,
       :file => /other_controller\.rb/,
@@ -1429,6 +1429,39 @@ def test_indirect_regex_dos
       :user_input => s(:call, s(:params), :[], s(:lit, :regex))
   end
 
+  def test_regex_match_dos
+    assert_warning :type => :warning,
+                   :warning_code => 76,
+                   :warning_type => "Denial of Service",
+                   :line => 86,
+                   :message => /^Parameter value used in string to regular expression coercion/,
+                   :confidence => 0,
+                   :relative_path => "app/controllers/other_controller.rb",
+                   :user_input => s(:call, s(:params), :[], s(:lit, :regex))
+  end
+
+  def test_regex_matchq_dos
+    assert_warning :type => :warning,
+                   :warning_code => 76,
+                   :warning_type => "Denial of Service",
+                   :line => 90,
+                   :message => /^Parameter value used in string to regular expression coercion/,
+                   :confidence => 0,
+                   :relative_path => "app/controllers/other_controller.rb",
+                   :user_input => s(:call, s(:params), :[], s(:lit, :regex))
+  end
+
+  def test_regex_modified_string_match
+    assert_warning :type => :warning,
+                   :warning_code => 76,
+                   :warning_type => "Denial of Service",
+                   :line => 94,
+                   :message => /^Parameter value used in string to regular expression coercion/,
+                   :confidence => 0,
+                   :relative_path => "app/controllers/other_controller.rb",
+                   :user_input => s(:call, s(:call, s(:params), :[], s(:lit, :regex)), :downcase)
+  end
+
   def test_unsafe_symbol_creation_from_param
     assert_warning :type => :warning,
       :warning_code => 59,
@@ -1518,7 +1551,7 @@ def expected
       :controller => 1,
       :model => 4,
       :template => 47,
-      :generic => 60 }
+      :generic => 63 }
   end
 
   def report

test/tests/tabs_output.rb

@@ -10,6 +10,6 @@ def setup
   end
 
   def test_reported_warnings
-    assert_equal 112, @@report.lines.to_a.count, 'Did you add new vulnerabilities to the Rails 2 app?'
+    assert_equal 115, @@report.lines.to_a.count, 'Did you add new vulnerabilities to the Rails 2 app?'
   end
 end