git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Buggy handling of non-canonical ref names
@ 2011-08-24 15:49 Michael Haggerty
  2011-08-24 18:40 ` Junio C Hamano
  0 siblings, 1 reply; 10+ messages in thread
From: Michael Haggerty @ 2011-08-24 15:49 UTC (permalink / raw)
  To: git discussion list

git has inconsistencies (I would say bugs) in how it handles reference
names that are in non-canonical format (e.g., "/foo/bar", "foo///bar",
etc).  Some commands work with reference names that are in non-canonical
form, whereas others do not.  Sometimes the behavior depends on whether
the reference is loose or packed.

For example "git branch" and "git update-ref" seem to swallow them OK:

    $ git update-ref /foo/bar HEAD
    $ cat .git/foo/bar
    ef6cf90ba11dd6205f8b974692d795ea0b1c0bdd
    $ git branch /bar/baz
    $ git for-each-ref | grep baz
    ef6cf90ba11dd6205f8b974692d795ea0b1c0bdd commit refs/heads/bar/baz

But other commands do not:

    $ git rev-parse /foo/bar --
    /foo/bar
    fatal: ambiguous argument '/foo/bar': unknown revision or path not
in the working tree.
    Use '--' to separate paths from revisions
    $ git rev-parse foo///bar --
    foo///bar
    fatal: ambiguous argument 'foo///bar': unknown revision or path not
in the working tree.
    Use '--' to separate paths from revisions
    $ git rev-parse foo/bar --
    ef6cf90ba11dd6205f8b974692d795ea0b1c0bdd
    --

It seems like most of the canonicalization of reference names is via
path canonicalization using the likes of git_snpath() and git_path(),
with the exception of "git check-ref-format --print", which does the
work using its own code.  But packed references are not transformed into
paths, so they don't handle canonicalization at all.  For example, in
the following case, there is a difference between how packed and
unpacked references are handled:

    $ git update-ref refs/heads/foo/bar HEAD
    $ git update-ref -d refs/heads/foo///bar HEAD
    [Reference was really deleted]
    $ git update-ref refs/heads/foo/bar HEAD
    $ git pack-refs --all
    $ git update-ref -d refs/heads/foo///bar HEAD
    error: unable to resolve reference refs/heads/foo///bar: No such
file or directory
    $ git for-each-ref | grep foo
    ffae1b1dc75896bd89ff3cbe7037f40474e57e2a commit refs/heads/foo/bar
    [Reference was not deleted]

What is the policy about reference names and their canonicalization?  In
what situations should one be able to use non-canonical refnames, and in
what situations should it be forbidden?

Given that refnames are closely associated with filesystem paths, it is
important that every code path either canonicalize or reject
non-canonical refnames (i.e., failure to do so could be a security
problem).  In the absence of clear rules, it will be very difficult to
ensure that this is being done consistently.

I also noticed that check_ref_format() accepts ref names that start with
a "/", but that the leading slash is *not* stripped off as part of the
canonicalization:

    $ git check-ref-format /foo/bar ; echo $?
    0
    $ git check-ref-format --print /foo/bar
    /foo/bar

However, creating a reference with such a name is equivalent to creating
a reference without the leading slash:

    $ git update-ref /foo/bar HEAD
    $ cat .git/foo/bar
    ef6cf90ba11dd6205f8b974692d795ea0b1c0bdd
    $ git branch /bar/baz
    $ git for-each-ref | grep baz
    ef6cf90ba11dd6205f8b974692d795ea0b1c0bdd commit refs/heads/bar/baz

Therefore, it seems to belong in the equivalence class of "foo/bar".

The test suite for "git check-ref-format" says nothing about how leading
slashes should be handled.

Either leading slashes should not be allowed in ref names at all or they
should be canonicalized away.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-08-25 21:58 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-08-24 15:49 Buggy handling of non-canonical ref names Michael Haggerty
2011-08-24 18:40 ` Junio C Hamano
2011-08-24 21:32   ` Carlos Martín Nieto
2011-08-24 22:27   ` Junio C Hamano
2011-08-25  7:54     ` Michael Haggerty
2011-08-25  8:08       ` [PATCH] Do not allow refnames to start with a slash Michael Haggerty
2011-08-25 18:17         ` Junio C Hamano
2011-08-25 19:19           ` [PATCH] check-ref-format --print: Normalize refnames that start with slashes Michael Haggerty
2011-08-25 20:42             ` Junio C Hamano
2011-08-25 21:57               ` Michael Haggerty

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).