security: harden RevParse and pass --end-of-options across git commands (#142)

Author: unknwon

blob.go

@@ -32,5 +32,5 @@ func (b *Blob) Bytes() ([]byte, error) {
 // Pipeline reads the content of the blob and pipes stdout and stderr to
 // supplied io.Writer.
 func (b *Blob) Pipeline(stdout, stderr io.Writer) error {
-	return NewCommand("show", b.id.String()).RunInDirPipeline(stdout, stderr, b.parent.repo.path)
+	return NewCommand("show", "--end-of-options", b.id.String()).RunInDirPipeline(stdout, stderr, b.parent.repo.path)
 }

commit_archive.go

@@ -25,6 +25,7 @@ func (c *Commit) CreateArchive(format ArchiveFormat, dst string) error {
 		"--prefix="+prefix,
 		"--format="+string(format),
 		"-o", dst,
+		"--end-of-options",
 		c.ID.String(),
 	).RunInDir(c.repo.path)
 	return err

repo.go

@@ -189,6 +189,7 @@ func (r *Repository) Fetch(opts ...FetchOptions) error {
 	if opt.Prune {
 		cmd.AddArgs("--prune")
 	}
+	cmd.AddArgs("--end-of-options")
 
 	_, err := cmd.RunInDirWithTimeout(opt.Timeout, r.path)
 	return err
@@ -229,6 +230,7 @@ func (r *Repository) Pull(opts ...PullOptions) error {
 	if opt.All {
 		cmd.AddArgs("--all")
 	}
+	cmd.AddArgs("--end-of-options")
 	if !opt.All && opt.Remote != "" {
 		cmd.AddArgs(opt.Remote)
 		if opt.Branch != "" {
@@ -300,11 +302,9 @@ func Checkout(repoPath, branch string, opts ...CheckoutOptions) error {
 
 	cmd := NewCommand("checkout").AddOptions(opt.CommandOptions)
 	if opt.BaseBranch != "" {
-		cmd.AddArgs("-b")
-	}
-	cmd.AddArgs(branch)
-	if opt.BaseBranch != "" {
-		cmd.AddArgs(opt.BaseBranch)
+		cmd.AddArgs("-b", branch, "--end-of-options", opt.BaseBranch)
+	} else {
+		cmd.AddArgs("--end-of-options", branch)
 	}
 
 	_, err := cmd.RunInDirWithTimeout(opt.Timeout, repoPath)
@@ -348,7 +348,7 @@ func Reset(repoPath, rev string, opts ...ResetOptions) error {
 		cmd.AddArgs("--hard")
 	}
 
-	_, err := cmd.AddOptions(opt.CommandOptions).AddArgs(rev).RunInDir(repoPath)
+	_, err := cmd.AddOptions(opt.CommandOptions).AddArgs("--end-of-options", rev).RunInDir(repoPath)
 	return err
 }
 
@@ -476,7 +476,8 @@ func CreateCommit(repoPath string, committer *Signature, message string, opts ..
 	}
 	cmd = cmd.AddArgs(fmt.Sprintf("--author='%s <%s>'", opt.Author.Name, opt.Author.Email)).
 		AddArgs("-m", message).
-		AddOptions(opt.CommandOptions)
+		AddOptions(opt.CommandOptions).
+		AddArgs("--end-of-options")
 
 	_, err := cmd.RunInDirWithTimeout(opt.Timeout, repoPath)
 	// No stderr but exit status 1 means nothing to commit.
@@ -593,6 +594,15 @@ func (r *Repository) RevParse(rev string, opts ...RevParseOptions) (string, erro
 		opt = opts[0]
 	}
 
+	// A revision that starts with a dash is never a valid object and would
+	// instead be interpreted by "git rev-parse" as a command-line option,
+	// allowing argument injection. The "--end-of-options" separator does not
+	// help here because "git rev-parse" does not honor it for revisions, so
+	// reject such input outright.
+	if strings.HasPrefix(rev, "-") {
+		return "", ErrRevisionNotExist
+	}
+
 	commitID, err := NewCommand("rev-parse").
 		AddOptions(opt.CommandOptions).
 		AddArgs(rev).
@@ -640,6 +650,7 @@ func CountObjects(repoPath string, opts ...CountObjectsOptions) (*CountObject, e
 
 	stdout, err := NewCommand("count-objects", "-v").
 		AddOptions(opt.CommandOptions).
+		AddArgs("--end-of-options").
 		RunInDirWithTimeout(opt.Timeout, repoPath)
 	if err != nil {
 		return nil, err
@@ -706,7 +717,7 @@ func Fsck(repoPath string, opts ...FsckOptions) error {
 		opt = opts[0]
 	}
 
-	cmd := NewCommand("fsck").AddOptions(opt.CommandOptions)
+	cmd := NewCommand("fsck").AddOptions(opt.CommandOptions).AddArgs("--end-of-options")
 	_, err := cmd.RunInDirWithTimeout(opt.Timeout, repoPath)
 	return err
 }

repo_commit.go

@@ -555,6 +555,7 @@ func (r *Repository) LatestCommitTime(opts ...LatestCommitTimeOptions) (time.Tim
 			"--count=1",
 			"--sort=-committerdate",
 			"--format=%(committerdate:iso8601)",
+			"--end-of-options",
 		)
 	if opt.Branch != "" {
 		cmd.AddArgs(RefsHeads + opt.Branch)

repo_remote.go

@@ -216,6 +216,7 @@ func Remotes(repoPath string, opts ...RemotesOptions) ([]string, error) {
 
 	stdout, err := NewCommand("remote").
 		AddOptions(opt.CommandOptions).
+		AddArgs("--end-of-options").
 		RunInDirWithTimeout(opt.Timeout, repoPath)
 	if err != nil {
 		return nil, err

repo_tag.go

@@ -184,6 +184,7 @@ func RepoTags(repoPath string, opts ...TagsOptions) ([]string, error) {
 		sorted = true
 	}
 
+	cmd.AddArgs("--end-of-options")
 	if opt.Pattern != "" {
 		cmd.AddArgs(opt.Pattern)
 	}

repo_test.go

@@ -494,6 +494,19 @@ func TestRepository_RevParse(t *testing.T) {
 			expID:  "",
 			expErr: ErrRevisionNotExist,
 		},
+
+		// A revision starting with a dash must be rejected to prevent argument
+		// injection into "git rev-parse".
+		{
+			rev:    "--absolute-git-dir",
+			expID:  "",
+			expErr: ErrRevisionNotExist,
+		},
+		{
+			rev:    "-h",
+			expID:  "",
+			expErr: ErrRevisionNotExist,
+		},
 	}
 	for _, test := range tests {
 		t.Run("", func(t *testing.T) {

repo_tree.go

@@ -142,7 +142,7 @@ func (r *Repository) LsTree(treeID string, opts ...LsTreeOptions) (*Tree, error)
 	}
 	stdout, err := cmd.
 		AddOptions(opt.CommandOptions).
-		AddArgs(treeID).
+		AddArgs("--end-of-options", treeID).
 		RunInDirWithTimeout(opt.Timeout, r.path)
 	if err != nil {
 		return nil, err

server.go

@@ -33,6 +33,7 @@ func UpdateServerInfo(path string, opts ...UpdateServerInfoOptions) error {
 	if opt.Force {
 		cmd.AddArgs("--force")
 	}
+	cmd.AddArgs("--end-of-options")
 	_, err := cmd.RunInDirWithTimeout(opt.Timeout, path)
 	return err
 }
@@ -66,7 +67,7 @@ func ReceivePack(path string, opts ...ReceivePackOptions) ([]byte, error) {
 	if opt.HTTPBackendInfoRefs {
 		cmd.AddArgs("--http-backend-info-refs")
 	}
-	cmd.AddArgs(".")
+	cmd.AddArgs("--end-of-options", ".")
 	return cmd.RunInDirWithTimeout(opt.Timeout, path)
 }
 
@@ -108,6 +109,6 @@ func UploadPack(path string, opts ...UploadPackOptions) ([]byte, error) {
 	if opt.HTTPBackendInfoRefs {
 		cmd.AddArgs("--http-backend-info-refs")
 	}
-	cmd.AddArgs(".")
+	cmd.AddArgs("--end-of-options", ".")
 	return cmd.RunInDirWithTimeout(opt.Timeout, path)
 }