Merge pull request #1195 from presidentbeef/flag_first_string_interpo…

…lation

BaseCheck#include_interp? should return first string interp

lib/brakeman/checks/base_check.rb

@@ -119,7 +119,10 @@ def process_cookies exp
 
   #Does not actually process string interpolation, but notes that it occurred.
   def process_dstr exp
-    @string_interp = Match.new(:interp, exp)
+    unless @string_interp # don't overwrite existing value
+      @string_interp = Match.new(:interp, exp)
+    end
+
     process_default exp
   end
 

test/apps/rails5.2/lib/shell.rb

@@ -17,4 +17,9 @@ def process_pid
     # should not warn
     `something #{Process.pid}`
   end
+
+  def nested_system_interp
+    filename = Shellwords.escape("#{file_prefix}.txt")
+    system "echo #{filename}"
+  end
 end

test/tests/rails52.rb

@@ -92,6 +92,19 @@ def test_command_injection_shellwords
       :user_input => s(:call, s(:const, :Shellwords), :shellescape, s(:lvar, :ip))
   end
 
+  def test_command_injection_nested_shellwords
+    assert_no_warning :type => :warning,
+      :warning_code => 14,
+      :fingerprint => "acb19548a2a44c3070d35c62754216f4b3365f372d6004470417cca587af0f47",
+      :warning_type => "Command Injection",
+      :line => 23,
+      :message => /^Possible\ command\ injection/,
+      :confidence => 1,
+      :relative_path => "lib/shell.rb",
+      :code => s(:call, nil, :system, s(:dstr, "echo ", s(:evstr, s(:call, s(:const, :Shellwords), :escape, s(:dstr, "", s(:evstr, s(:call, nil, :file_prefix)), s(:str, ".txt")))))),
+      :user_input => s(:call, nil, :file_prefix)
+  end
+
   def test_command_injection_as_target
     assert_warning :type => :warning,
       :warning_code => 14,