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

lib/brakeman/checks/check_regex_dos.rb

@@ -11,12 +11,20 @@ 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]
+    COERCES_STRING_TO_REGEX.each do |coercion_method|
+      calls.concat tracker.find_call(:method => [coercion_method]).select { |call| string?(call[:target]) }
+    end
 
     Brakeman.debug "Processing dynamic regexes"
     calls.each do |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",

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

@@ -82,6 +82,14 @@ 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_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 177, @@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 => 62 }
   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 => 94,
       :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 => 96,
       :message => /^Symbol\ conversion\ from\ unsafe\ string\ in pa/,
       :confidence => 1,
       :file => /other_controller\.rb/,
@@ -1429,6 +1429,28 @@ 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_unsafe_symbol_creation_from_param
     assert_warning :type => :warning,
       :warning_code => 59,
@@ -1518,7 +1540,7 @@ def expected
       :controller => 1,
       :model => 4,
       :template => 47,
-      :generic => 60 }
+      :generic => 62 }
   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 114, @@report.lines.to_a.count, 'Did you add new vulnerabilities to the Rails 2 app?'
   end
 end