On Tue, Jun 26, 2018 at 02:27:39PM -0400, Jeff King wrote:
> 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
