From de1d5a5978b9599ca3dacd58bbf699e4bb4cf6bd Mon Sep 17 00:00:00 2001 From: Ayman Bagabas Date: Tue, 28 Nov 2023 14:31:04 -0500 Subject: [PATCH] git: validate reference names Check reference names format before creating branches/tags/remotes. This should probably be in a lower level somewhere in `plumbing`. Validating the names under `plumbing.NewReference*` is not possible since these functions don't return errors. Fixes: https://github.com/go-git/go-git/issues/929 --- config/branch.go | 2 +- config/config.go | 3 +- plumbing/reference.go | 89 ++++++++++++++++++++++++++++++++++++++ plumbing/reference_test.go | 59 +++++++++++++++++++++++++ repository.go | 9 +++- repository_test.go | 37 ++++++++++++++++ worktree.go | 4 ++ worktree_test.go | 24 ++++++++++ 8 files changed, 224 insertions(+), 3 deletions(-) diff --git a/config/branch.go b/config/branch.go index 652270a28..db2cb499a 100644 --- a/config/branch.go +++ b/config/branch.go @@ -54,7 +54,7 @@ func (b *Branch) Validate() error { return errBranchInvalidRebase } - return nil + return plumbing.NewBranchReferenceName(b.Name).Validate() } func (b *Branch) marshal() *format.Subsection { diff --git a/config/config.go b/config/config.go index da425a784..6d41c15dc 100644 --- a/config/config.go +++ b/config/config.go @@ -13,6 +13,7 @@ import ( "github.com/go-git/go-billy/v5/osfs" "github.com/go-git/go-git/v5/internal/url" + "github.com/go-git/go-git/v5/plumbing" format "github.com/go-git/go-git/v5/plumbing/format/config" ) @@ -614,7 +615,7 @@ func (c *RemoteConfig) Validate() error { c.Fetch = []RefSpec{RefSpec(fmt.Sprintf(DefaultFetchRefSpec, c.Name))} } - return nil + return plumbing.NewRemoteHEADReferenceName(c.Name).Validate() } func (c *RemoteConfig) unmarshal(s *format.Subsection) error { diff --git a/plumbing/reference.go b/plumbing/reference.go index 5a67f69e7..ddba93029 100644 --- a/plumbing/reference.go +++ b/plumbing/reference.go @@ -3,6 +3,7 @@ package plumbing import ( "errors" "fmt" + "regexp" "strings" ) @@ -29,6 +30,9 @@ var RefRevParseRules = []string{ var ( ErrReferenceNotFound = errors.New("reference not found") + + // ErrInvalidReferenceName is returned when a reference name is invalid. + ErrInvalidReferenceName = errors.New("invalid reference name") ) // ReferenceType reference type's @@ -124,6 +128,91 @@ func (r ReferenceName) Short() string { return res } +var ( + ctrlSeqs = regexp.MustCompile(`[\000-\037\177]`) +) + +// Validate validates a reference name. +// This follows the git-check-ref-format rules. +// See https://git-scm.com/docs/git-check-ref-format +// +// It is important to note that this function does not check if the reference +// exists in the repository. +// It only checks if the reference name is valid. +// This functions does not support the --refspec-pattern, --normalize, and +// --allow-onelevel options. +// +// Git imposes the following rules on how references are named: +// +// 1. They can include slash / for hierarchical (directory) grouping, but no +// slash-separated component can begin with a dot . or end with the +// sequence .lock. +// 2. They must contain at least one /. This enforces the presence of a +// category like heads/, tags/ etc. but the actual names are not +// restricted. If the --allow-onelevel option is used, this rule is +// waived. +// 3. They cannot have two consecutive dots .. anywhere. +// 4. They cannot have ASCII control characters (i.e. bytes whose values are +// lower than \040, or \177 DEL), space, tilde ~, caret ^, or colon : +// anywhere. +// 5. They cannot have question-mark ?, asterisk *, or open bracket [ +// anywhere. See the --refspec-pattern option below for an exception to this +// rule. +// 6. They cannot begin or end with a slash / or contain multiple consecutive +// slashes (see the --normalize option below for an exception to this rule). +// 7. They cannot end with a dot .. +// 8. They cannot contain a sequence @{. +// 9. They cannot be the single character @. +// 10. They cannot contain a \. +func (r ReferenceName) Validate() error { + s := string(r) + if len(s) == 0 { + return ErrInvalidReferenceName + } + + // HEAD is a special case + if r == HEAD { + return nil + } + + // rule 7 + if strings.HasSuffix(s, ".") { + return ErrInvalidReferenceName + } + + // rule 2 + parts := strings.Split(s, "/") + if len(parts) < 2 { + return ErrInvalidReferenceName + } + + isBranch := r.IsBranch() + isTag := r.IsTag() + for _, part := range parts { + // rule 6 + if len(part) == 0 { + return ErrInvalidReferenceName + } + + if strings.HasPrefix(part, ".") || // rule 1 + strings.Contains(part, "..") || // rule 3 + ctrlSeqs.MatchString(part) || // rule 4 + strings.ContainsAny(part, "~^:?*[ \t\n") || // rule 4 & 5 + strings.Contains(part, "@{") || // rule 8 + part == "@" || // rule 9 + strings.Contains(part, "\\") || // rule 10 + strings.HasSuffix(part, ".lock") { // rule 1 + return ErrInvalidReferenceName + } + + if (isBranch || isTag) && strings.HasPrefix(part, "-") { // branches & tags can't start with - + return ErrInvalidReferenceName + } + } + + return nil +} + const ( HEAD ReferenceName = "HEAD" Master ReferenceName = "refs/heads/master" diff --git a/plumbing/reference_test.go b/plumbing/reference_test.go index 04dfef93d..ce570752f 100644 --- a/plumbing/reference_test.go +++ b/plumbing/reference_test.go @@ -103,6 +103,65 @@ func (s *ReferenceSuite) TestIsTag(c *C) { c.Assert(r.IsTag(), Equals, true) } +func (s *ReferenceSuite) TestValidReferenceNames(c *C) { + valid := []ReferenceName{ + "refs/heads/master", + "refs/notes/commits", + "refs/remotes/origin/master", + "HEAD", + "refs/tags/v3.1.1", + "refs/pulls/1/head", + "refs/pulls/1/merge", + "refs/pulls/1/abc.123", + "refs/pulls", + "refs/-", // should this be allowed? + } + for _, v := range valid { + c.Assert(v.Validate(), IsNil) + } + + invalid := []ReferenceName{ + "refs", + "refs/", + "refs//", + "refs/heads/\\", + "refs/heads/\\foo", + "refs/heads/\\foo/bar", + "abc", + "", + "refs/heads/ ", + "refs/heads/ /", + "refs/heads/ /foo", + "refs/heads/.", + "refs/heads/..", + "refs/heads/foo..", + "refs/heads/foo.lock", + "refs/heads/foo@{bar}", + "refs/heads/foo[", + "refs/heads/foo~", + "refs/heads/foo^", + "refs/heads/foo:", + "refs/heads/foo?", + "refs/heads/foo*", + "refs/heads/foo[bar", + "refs/heads/foo\t", + "refs/heads/@", + "refs/heads/@{bar}", + "refs/heads/\n", + "refs/heads/-foo", + "refs/heads/foo..bar", + "refs/heads/-", + "refs/tags/-", + "refs/tags/-foo", + } + + for i, v := range invalid { + comment := Commentf("invalid reference name case %d: %s", i, v) + c.Assert(v.Validate(), NotNil, comment) + c.Assert(v.Validate(), ErrorMatches, "invalid reference name", comment) + } +} + func benchMarkReferenceString(r *Reference, b *testing.B) { for n := 0; n < b.N; n++ { _ = r.String() diff --git a/repository.go b/repository.go index 48988383d..1524a6913 100644 --- a/repository.go +++ b/repository.go @@ -98,6 +98,10 @@ func InitWithOptions(s storage.Storer, worktree billy.Filesystem, options InitOp options.DefaultBranch = plumbing.Master } + if err := options.DefaultBranch.Validate(); err != nil { + return nil, err + } + r := newRepository(s, worktree) _, err := r.Reference(plumbing.HEAD, false) switch err { @@ -724,7 +728,10 @@ func (r *Repository) DeleteBranch(name string) error { // CreateTag creates a tag. If opts is included, the tag is an annotated tag, // otherwise a lightweight tag is created. func (r *Repository) CreateTag(name string, hash plumbing.Hash, opts *CreateTagOptions) (*plumbing.Reference, error) { - rname := plumbing.ReferenceName(path.Join("refs", "tags", name)) + rname := plumbing.NewTagReferenceName(name) + if err := rname.Validate(); err != nil { + return nil, err + } _, err := r.Storer.Reference(rname) switch err { diff --git a/repository_test.go b/repository_test.go index 35a62f131..51df84512 100644 --- a/repository_test.go +++ b/repository_test.go @@ -75,6 +75,13 @@ func (s *RepositorySuite) TestInitWithOptions(c *C) { } +func (s *RepositorySuite) TestInitWithInvalidDefaultBranch(c *C) { + _, err := InitWithOptions(memory.NewStorage(), memfs.New(), InitOptions{ + DefaultBranch: "foo", + }) + c.Assert(err, NotNil) +} + func createCommit(c *C, r *Repository) { // Create a commit so there is a HEAD to check wt, err := r.Worktree() @@ -391,6 +398,22 @@ func (s *RepositorySuite) TestDeleteRemote(c *C) { c.Assert(alt, IsNil) } +func (s *RepositorySuite) TestEmptyCreateBranch(c *C) { + r, _ := Init(memory.NewStorage(), nil) + err := r.CreateBranch(&config.Branch{}) + + c.Assert(err, NotNil) +} + +func (s *RepositorySuite) TestInvalidCreateBranch(c *C) { + r, _ := Init(memory.NewStorage(), nil) + err := r.CreateBranch(&config.Branch{ + Name: "-foo", + }) + + c.Assert(err, NotNil) +} + func (s *RepositorySuite) TestCreateBranchAndBranch(c *C) { r, _ := Init(memory.NewStorage(), nil) testBranch := &config.Branch{ @@ -2797,6 +2820,20 @@ func (s *RepositorySuite) TestDeleteTagAnnotatedUnpacked(c *C) { c.Assert(err, Equals, plumbing.ErrObjectNotFound) } +func (s *RepositorySuite) TestInvalidTagName(c *C) { + r, err := Init(memory.NewStorage(), nil) + c.Assert(err, IsNil) + for i, name := range []string{ + "", + "foo bar", + "foo\tbar", + "foo\nbar", + } { + _, err = r.CreateTag(name, plumbing.ZeroHash, nil) + c.Assert(err, NotNil, Commentf("case %d %q", i, name)) + } +} + func (s *RepositorySuite) TestBranches(c *C) { f := fixtures.ByURL("https://github.com/git-fixtures/root-references.git").One() sto := filesystem.NewStorage(f.DotGit(), cache.NewObjectLRUDefault()) diff --git a/worktree.go b/worktree.go index f8b854dda..51795e635 100644 --- a/worktree.go +++ b/worktree.go @@ -189,6 +189,10 @@ func (w *Worktree) Checkout(opts *CheckoutOptions) error { return w.Reset(ro) } func (w *Worktree) createBranch(opts *CheckoutOptions) error { + if err := opts.Branch.Validate(); err != nil { + return err + } + _, err := w.r.Storer.Reference(opts.Branch) if err == nil { return fmt.Errorf("a branch named %q already exists", opts.Branch) diff --git a/worktree_test.go b/worktree_test.go index 180bfb0b4..5c9a4eb3a 100644 --- a/worktree_test.go +++ b/worktree_test.go @@ -785,6 +785,30 @@ func (s *WorktreeSuite) TestCheckoutCreateMissingBranch(c *C) { c.Assert(err, Equals, ErrCreateRequiresBranch) } +func (s *WorktreeSuite) TestCheckoutCreateInvalidBranch(c *C) { + w := &Worktree{ + r: s.Repository, + Filesystem: memfs.New(), + } + + for _, name := range []plumbing.ReferenceName{ + "foo", + "-", + "-foo", + "refs/heads//", + "refs/heads/..", + "refs/heads/a..b", + "refs/heads/.", + } { + err := w.Checkout(&CheckoutOptions{ + Create: true, + Branch: name, + }) + + c.Assert(err, Equals, plumbing.ErrInvalidReferenceName) + } +} + func (s *WorktreeSuite) TestCheckoutTag(c *C) { f := fixtures.ByTag("tags").One() r := s.NewRepositoryWithEmptyWorktree(f)