check_execute.rb in brakeman-lib-4.7.0

- old
+ new

@@ -19,10 +19,14 @@
                   s(:call, s(:const, :Process), :pid)]
 
   SHELL_ESCAPE_MODULE_METHODS = Set[:escape, :join, :shellescape, :shelljoin]
   SHELL_ESCAPE_MIXIN_METHODS = Set[:shellescape, :shelljoin]
 
+  # These are common shells that are known to allow the execution of commands
+  # via a -c flag. See dash_c_shell_command? for more info.
+  KNOWN_SHELL_COMMANDS = Set["sh", "bash", "ksh", "csh", "tcsh", "zsh"]
+
   SHELLWORDS = s(:const, :Shellwords)
 
   #Check models, controllers, and views for command injection.
   def run_check
     Brakeman.debug "Finding system calls using ``"
@@ -40,10 +44,12 @@
     calls.each do |result|
       process_result result
     end
   end
 
+  private
+
   #Processes results from Tracker#find_call.
   def process_result result
     call = result[:call]
     args = call.arglist
     first_arg = call.first_arg
@@ -52,11 +58,21 @@
     when :popen
       unless array? first_arg
         failure = include_user_input?(args) || dangerous_interp?(args)
       end
     when :system, :exec
-      failure = include_user_input?(first_arg) || dangerous_interp?(first_arg)
+      # Normally, if we're in a `system` or `exec` call, we only are worried
+      # about shell injection when there's a single argument, because comma-
+      # separated arguments are always escaped by Ruby. However, an exception is
+      # when the first two arguments are something like "bash -c" because then
+      # the third argument is effectively the command being run and might be
+      # a malicious executable if it comes (partially or fully) from user input.
+      if dash_c_shell_command?(first_arg, call.second_arg)
+        failure = include_user_input?(args[3]) || dangerous_interp?(args[3])
+      else
+        failure = include_user_input?(first_arg) || dangerous_interp?(first_arg)
+      end
     else
       failure = include_user_input?(args) || dangerous_interp?(args)
     end
 
     if failure and original? result
@@ -73,9 +89,18 @@
         :message => "Possible command injection",
         :code => call,
         :user_input => failure,
         :confidence => confidence
     end
+  end
+
+  # @return [Boolean] true iff the command given by `first_arg`, `second_arg`
+  #   invokes a new shell process via `<shell_command> -c` (like `bash -c`)
+  def dash_c_shell_command?(first_arg, second_arg)
+    string?(first_arg) &&
+    KNOWN_SHELL_COMMANDS.include?(first_arg.value) &&
+    string?(second_arg) &&
+    second_arg.value == "-c"
   end
 
   def check_open_calls
     tracker.find_call(:targets => [nil, :Kernel], :method => :open).each do |result|
       if match = dangerous_open_arg?(result[:call].first_arg)