Re: [BUG] url schemes should be case-insensitive

From: Jeff King <peff@peff.net>
Date: Jun 26, 2018, 20:03 UTC
Message-ID: <20180626200319.GA2230@sigill.intra.peff.net>
In-Reply-To: <20180626182739.GA28358@sigill.intra.peff.net>

On Tue, Jun 26, 2018 at 02:27:39PM -0400, Jeff King wrote:

Show 12 quoted lines

> So yeah, we would not want to allow EXT::"rm -rf /" to slip past the
> known-unsafe match. Any normalization should happen before then
> (probably right in transport_helper_init).
> 
> Come to think of it, that's already sort-of an issue now. If you have a
> case-insensitive filesystem, then EXT:: is going to pass this check, but
> still run git-remote-ext. We're saved there somewhat by the fact that
> the default is to reject unknown helpers in submodules (otherwise, we'd
> have that horrible submodule bug all over again).
> 
> That goes beyond just cases, too. On HFS+ I wonder if I could ask for
> "\u{0200}ext::" and run git-remote-ext.

That should be \u{200c}, of course, to get the correct sneaky character. And the good news is that no, it doesn't work. We are protected by this code in transport_get():

          /* maybe it is a foreign URL? */
          if (url) {
                  const char *p = url;

                  while (is_urlschemechar(p == url, *p))
                          p++;     
                  if (starts_with(p, "::"))
                          helper = xstrndup(url, p - url);
          }

So we'll only allow remote-helper names with valid url characters, which are basically [A-Za-z0-9+.-]. So I think we probably only have to worry about true case issues, and not any kind of weird filesystem-specific behaviors.

-Peff