On Tue, Aug 06, 2019 at 10:58:53PM +0000, brian m. carlson wrote: > On 2019-08-06 at 14:38:30, Jeff King wrote: > > It's hard for scripted uses of rev-list, etc, to avoid option injection > > from untrusted arguments, because revision arguments must come before > > any "--" separator. I.e.: > > > > git rev-list "$revision" -- "$path" > > > > might mistake "$revision" for an option (with rev-list, that would make > > it an error, but something like git-log would default to HEAD). > > > > This series provides an alternative to "--" to stop option parsing > > without indicating that further arguments are pathspecs. > > Sorry, I hadn't had a chance to look at this series in depth, but I was > wondering: could we not just accept two separate "--" arguments, and if > there are two of them, interpret the first with the traditional meaning > and the second with the Git-specific meaning? That would be much more > intuitive for folks, although I suspect it would take a little more work > in the options parser. That also crossed my mind, but I think it opens up some complicated corner cases. For instance, if I'm parsing left-to-right and see "--", how do I know which separator it is meant to be? I think the only rule that makes sense is that you must have two "--", like: git rev-list [options] -- [revs] -- [paths] but that means parsing the whole thing before we can interpret any of it. What kinds of tricks can an attacker play by putting "--" in the revs or paths areas? E.g., what does this mean: # expanded from "git rev-list -- $revs -- $paths" git rev-list -- --foo -- -- --bar -- I think if we at least choose the left-most "--" as the official end-of-options then they can't inject an option (they can only inject a rev as a path). I guess that's the same as with --end-of-options. But it somehow feels less clear to me than a separate marker. It also doesn't allow this: # allow paths and revs, with optional separator, but no more options git rev-list --end-of-options "$@" though I'm not sure whether anybody cares. -Peff
