Re: [PATCH v3 0/8] refs: add symref support to 'git-update-ref'

[Jeff King <peff@peff.net>]

On Tue, Apr 23, 2024 at 11:28:10PM +0200, Karthik Nayak wrote:

> Changes from v2:
> 1. We no longer have separate commands for symrefs, instead the regular
> commands learn to parse 'ref:<target>' as symref targets. This reduces
> the code in this series. Thanks Patrick for the suggestion.

Hmm. I can see how this makes things a lot simpler, but it introduces an
ambiguity, since you can pass full ref expressions to "update-ref" (like
"ref:foo" to find the "foo" entry in ref^{tree}). I see that you only
kick in the symref "ref:" handling if the regular oid lookup failed, so
there's no backwards-compatibility issue (anything that used to work
will still take precedence, and the new code only runs when the old code
would have reported an error).

But I wonder if it would let somebody cause mischief in a repository
they can push to, but which may get updates from other sources. For
example, imagine a forge like GitLab runs the equivalent of:

  echo "create refs/whatever ref:refs/heads/main" |
  git update-ref --stdin

as part of some system process. Now if I push up "refs/heads/ref" that
contains the path "refs/heads/main" in its tree, that will take
precedence, causing the system process to do something it did not
expect.

I think you'd have to pile on a lot of assumptions to get any kind of
security problem. Something like:

 1. The system has a hidden ref namespace like refs/gitlab that normal
    remote push/fetch users are not allowed to read/write to.

 2. The system tries to make a symlink within that namespace. Say,
    "refs/gitlab/metadata/HEAD" to point to
    "refs/gitlab/metadata/branches/main" or something.

 3. The user pushes up "refs/heads/ref" with a tree that contains
    "refs/gitlab/metadata/branches/main". Now when (2) happens, the
    hidden ref points to user-controlled data.

That's pretty convoluted. But we can avoid it entirely if there's no
ambiguity in the protocol at all.

-Peff