submodule foreach: skip eval for more than one argument

'eval "$@"' creates an extra layer of shell interpretation, which is
probably not expected by a user who passes multiple arguments to git
submodule foreach:

$ git grep "'"
[searches for single quotes]
$ git submodule foreach git grep "'"
Entering '[submodule]'
/usr/lib/git-core/git-submodule: 1: eval: Syntax error: Unterminated quoted string
Stopping at '[submodule]'; script returned non-zero status.

To fix this, if the user passes more than one argument, execute "$@"
directly instead of passing it to eval.

Examples:

* Typical usage when adding an extra level of quoting is to pass a
single argument representing the entire command to be passed to the
shell.  This doesn't change that.

* One can imagine someone feeding untrusted input as an argument:

git submodule foreach git grep "$variable"

That currently results in a nonobvious shell code injection
vulnerability.  Executing the command named by the arguments
directly, as in this patch, fixes it.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>
Acked-by: Johan Herland <johan@herland.net>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>

diff --git a/git-submodule.sh b/git-submodule.sh
index 2979197087f2c6d97e2945008394d50c16a195a5..7b2a83d70f7a9a178f3f0526ea0334891d12b10e 100755 (executable)
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -545,7 +545,12 @@ cmd_foreach()
                                sm_path=$(relative_path "$sm_path") &&
                                # we make $path available to scripts ...
                                path=$sm_path &&
-                               eval "$@" &&
+                               if test $# -eq 1
+                               then
+                                       eval "$1"
+                               else
+                                       "$@"
+                               fi &&
                                if test -n "$recursive"
                                then
                                        cmd_foreach "--recursive" "$@"

diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
index be93f10cf02bc77e7c78be41bea69b4268305e66..6b2fd39aaa4890c45ef38f147ef20cf907c99da6 100755 (executable)
--- a/t/t7407-submodule-foreach.sh
+++ b/t/t7407-submodule-foreach.sh
@@ -329,4 +329,13 @@ test_expect_success 'command passed to foreach --recursive retains notion of std
        test_cmp expected actual
 '
 
+test_expect_success 'multi-argument command passed to foreach is not shell-evaluated twice' '
+       (
+               cd super &&
+               git submodule foreach "echo \\\"quoted\\\"" > ../expected &&
+               git submodule foreach echo \"quoted\" > ../actual
+       ) &&
+       test_cmp expected actual
+'
+
 test_done