Passing command line arguments correctly by string slice (#21168)

Using `append(args, strings.Fields(arg)...)` is dangerous, it may
generate incorrect results.

For example: `arg1 "the dangerous"` will be splitted to 3 arguments:
`arg1`, `"the`, `dangerous"`. In some cases the incorrect arguments may
lead to security problems.

2 files changed, 13 insertions, 17 deletions

diff --git a/modules/git/repo_branch_nogogit.go b/modules/git/repo_branch_nogogit.go
index f5fb244cb4..91112d0190 100644
--- a/modules/git/repo_branch_nogogit.go
+++ b/modules/git/repo_branch_nogogit.go
@@ -63,34 +63,32 @@ func (repo *Repository) IsBranchExist(name string) bool {
 // GetBranchNames returns branches from the repository, skipping skip initial branches and
 // returning at most limit branches, or all branches if limit is 0.
 func (repo *Repository) GetBranchNames(skip, limit int) ([]string, int, error) {
-	return callShowRef(repo.Ctx, repo.Path, BranchPrefix, BranchPrefix+" --sort=-committerdate", skip, limit)
+	return callShowRef(repo.Ctx, repo.Path, BranchPrefix, []string{BranchPrefix, "--sort=-committerdate"}, skip, limit)
 }
 
 // WalkReferences walks all the references from the repository
 func WalkReferences(ctx context.Context, repoPath string, walkfn func(sha1, refname string) error) (int, error) {
-	return walkShowRef(ctx, repoPath, "", 0, 0, walkfn)
+	return walkShowRef(ctx, repoPath, nil, 0, 0, walkfn)
 }
 
 // WalkReferences walks all the references from the repository
 // refType should be empty, ObjectTag or ObjectBranch. All other values are equivalent to empty.
 func (repo *Repository) WalkReferences(refType ObjectType, skip, limit int, walkfn func(sha1, refname string) error) (int, error) {
-	var arg string
+	var args []string
 	switch refType {
 	case ObjectTag:
-		arg = TagPrefix + " --sort=-taggerdate"
+		args = []string{TagPrefix, "--sort=-taggerdate"}
 	case ObjectBranch:
-		arg = BranchPrefix + " --sort=-committerdate"
-	default:
-		arg = ""
+		args = []string{BranchPrefix, "--sort=-committerdate"}
 	}
 
-	return walkShowRef(repo.Ctx, repo.Path, arg, skip, limit, walkfn)
+	return walkShowRef(repo.Ctx, repo.Path, args, skip, limit, walkfn)
 }
 
 // callShowRef return refs, if limit = 0 it will not limit
-func callShowRef(ctx context.Context, repoPath, prefix, arg string, skip, limit int) (branchNames []string, countAll int, err error) {
-	countAll, err = walkShowRef(ctx, repoPath, arg, skip, limit, func(_, branchName string) error {
-		branchName = strings.TrimPrefix(branchName, prefix)
+func callShowRef(ctx context.Context, repoPath, trimPrefix string, extraArgs []string, skip, limit int) (branchNames []string, countAll int, err error) {
+	countAll, err = walkShowRef(ctx, repoPath, extraArgs, skip, limit, func(_, branchName string) error {
+		branchName = strings.TrimPrefix(branchName, trimPrefix)
 		branchNames = append(branchNames, branchName)
 
 		return nil
@@ -98,7 +96,7 @@ func callShowRef(ctx context.Context, repoPath, prefix, arg string, skip, limit
 	return branchNames, countAll, err
 }
 
-func walkShowRef(ctx context.Context, repoPath, arg string, skip, limit int, walkfn func(sha1, refname string) error) (countAll int, err error) {
+func walkShowRef(ctx context.Context, repoPath string, extraArgs []string, skip, limit int, walkfn func(sha1, refname string) error) (countAll int, err error) {
 	stdoutReader, stdoutWriter := io.Pipe()
 	defer func() {
 		_ = stdoutReader.Close()
@@ -108,9 +106,7 @@ func walkShowRef(ctx context.Context, repoPath, arg string, skip, limit int, wal
 	go func() {
 		stderrBuilder := &strings.Builder{}
 		args := []string{"for-each-ref", "--format=%(objectname) %(refname)"}
-		if arg != "" {
-			args = append(args, strings.Fields(arg)...)
-		}
+		args = append(args, extraArgs...)
 		err := NewCommand(ctx, args...).Run(&RunOpts{
 			Dir:    repoPath,
 			Stdout: stdoutWriter,
@@ -194,7 +190,7 @@ func walkShowRef(ctx context.Context, repoPath, arg string, skip, limit int, wal
 // GetRefsBySha returns all references filtered with prefix that belong to a sha commit hash
 func (repo *Repository) GetRefsBySha(sha, prefix string) ([]string, error) {
 	var revList []string
-	_, err := walkShowRef(repo.Ctx, repo.Path, "", 0, 0, func(walkSha, refname string) error {
+	_, err := walkShowRef(repo.Ctx, repo.Path, nil, 0, 0, func(walkSha, refname string) error {
 		if walkSha == sha && strings.HasPrefix(refname, prefix) {
 			revList = append(revList, refname)
 		}
diff --git a/modules/git/repo_tag_nogogit.go b/modules/git/repo_tag_nogogit.go
index c2df8b14fd..72c1c979dc 100644
--- a/modules/git/repo_tag_nogogit.go
+++ b/modules/git/repo_tag_nogogit.go
@@ -26,7 +26,7 @@ func (repo *Repository) IsTagExist(name string) bool {
 // GetTags returns all tags of the repository.
 // returning at most limit tags, or all if limit is 0.
 func (repo *Repository) GetTags(skip, limit int) (tags []string, err error) {
-	tags, _, err = callShowRef(repo.Ctx, repo.Path, TagPrefix, TagPrefix+" --sort=-taggerdate", skip, limit)
+	tags, _, err = callShowRef(repo.Ctx, repo.Path, TagPrefix, []string{TagPrefix, "--sort=-taggerdate"}, skip, limit)
 	return tags, err
 }