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
