On Fri, Sep 27, 2013 at 12:23 PM, Anders Kaseorg <ande...@mit.edu> wrote:
> ‘eval "$@"’ created an extra layer of shell interpretation, which was
> probably not expected by a user who passed 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
> Stopping at '[submodule]'; script returned non-zero status.
> To fix this, if the user passed more than one argument, just execute
> "$@" directly instead of passing it to eval.
> Signed-off-by: Anders Kaseorg <ande...@mit.edu>
Acked-by: Johan Herland <jo...@herland.net>
> On Fri, 27 Sep 2013, Johan Herland wrote:
>> 2. If we are unlucky there might be existing users that work around the
>> existing behavior by adding an extra level of quoting (i.e. doing the
>> equivalent of git submodule foreach git grep "\'" in your example
>> above). Will their workaround break as a result of your change? Is that
> Anyone adding an extra level of quoting ought to realize that they should
> be passing a single argument to submodule foreach, so that the reason for
> the extra quoting is clear:
> git submodule foreach "git grep \'"
> will not break. If someone is actually doing
> git submodule foreach git grep "\'"
> then this will change in behavior. I think this change is important.
> (One could even imagine someone feeding untrusted input to
> git submodule foreach git grep "$variable"
> which, without my patch, results in a nonobvious shell code injection
> I considered an alternative fix where the first argument is always
> shell-evaulated and any others are not (i.e. cmd=$1 && shift && eval
> "$cmd \"\$@\""), which is potentially more useful in case the command
> needs to use $path. But that may be too confusing, and this way has some
> precedent (e.g. perl’s system()).
Ok. I have nothing to add.
Johan Herland, <jo...@herland.net>
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html