On Tue, Jun 26, 2018 at 10:09:58AM -0700, Junio C Hamano wrote:
> > It may also interact in a funny way with our allowed-protocol code, if
> > "SSH" gets a pass as "ssh" under the default config, but actually runs
> > the otherwise-disallowed git-remote-SSH (though one would _hope_ if you
> > have such a git-remote-SSH that it behaves just like an ssh remote).
>
> True. I did not offhand recall how protocol whitelist matches the
> protocol name with config, but transport.c::get_protocol_config()
> seems to say that the <name> part of "protocol.<name>.allow" is case
> sensitive, and we match known-safe (and known-unsafe "ext::")
> protocols with strcmp() not strcasecmp(). We need to figure out the
> implications of allowing SSH:// not to error out but pretending as
> if it were ssh:// on those who have protocol.ssh.allow defined.
That function is actually a little tricky, because we feed it mostly
from string literals (so we end up in the ssh code path, and then feed
it "ssh"). But I think for remote-helpers we feed it literally from the
URL we got fed.
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.
> > I think it doesn't help much if the user does not know what a remote
> > helper is, or why Git is looking for one.
>
> True.
>
> $ git clone SSH://example.com/repo.git
> fatal: unable to handle URL that begins with SSH://
>
> would be clear enough, perhaps? At least this line of change is a
> small first step that would improve the situation without potential
> to break anybody who has been abusing the case sensitivity loophole.
Yeah, certainly the advice is orthogonal to any behavior changes. The
original report complained of:
$ git clone SSH://...
fatal: 'remote-SSH' is not a git command. See 'git --help'.
but since 6b02de3b9d in 2010 we say:
fatal: Unable to find remote helper for 'SSH'
So I actually wonder if there is something else going on. I find it hard
to believe the OP is using something older than Git v1.7.0. They did
appear to be on Windows, though. Is it possible our ENOENT detection
from start_command() is not accurate on Windows?
-Peff
