Skip to content

Commit 0ba2f53

Browse files
authored
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.
1 parent e07d089 commit 0ba2f53

File tree

2 files changed

+13
-17
lines changed

2 files changed

+13
-17
lines changed

modules/git/repo_branch_nogogit.go

+12-16
Original file line numberDiff line numberDiff line change
@@ -63,42 +63,40 @@ func (repo *Repository) IsBranchExist(name string) bool {
6363
// GetBranchNames returns branches from the repository, skipping skip initial branches and
6464
// returning at most limit branches, or all branches if limit is 0.
6565
func (repo *Repository) GetBranchNames(skip, limit int) ([]string, int, error) {
66-
return callShowRef(repo.Ctx, repo.Path, BranchPrefix, BranchPrefix+" --sort=-committerdate", skip, limit)
66+
return callShowRef(repo.Ctx, repo.Path, BranchPrefix, []string{BranchPrefix, "--sort=-committerdate"}, skip, limit)
6767
}
6868

6969
// WalkReferences walks all the references from the repository
7070
func WalkReferences(ctx context.Context, repoPath string, walkfn func(sha1, refname string) error) (int, error) {
71-
return walkShowRef(ctx, repoPath, "", 0, 0, walkfn)
71+
return walkShowRef(ctx, repoPath, nil, 0, 0, walkfn)
7272
}
7373

7474
// WalkReferences walks all the references from the repository
7575
// refType should be empty, ObjectTag or ObjectBranch. All other values are equivalent to empty.
7676
func (repo *Repository) WalkReferences(refType ObjectType, skip, limit int, walkfn func(sha1, refname string) error) (int, error) {
77-
var arg string
77+
var args []string
7878
switch refType {
7979
case ObjectTag:
80-
arg = TagPrefix + " --sort=-taggerdate"
80+
args = []string{TagPrefix, "--sort=-taggerdate"}
8181
case ObjectBranch:
82-
arg = BranchPrefix + " --sort=-committerdate"
83-
default:
84-
arg = ""
82+
args = []string{BranchPrefix, "--sort=-committerdate"}
8583
}
8684

87-
return walkShowRef(repo.Ctx, repo.Path, arg, skip, limit, walkfn)
85+
return walkShowRef(repo.Ctx, repo.Path, args, skip, limit, walkfn)
8886
}
8987

9088
// callShowRef return refs, if limit = 0 it will not limit
91-
func callShowRef(ctx context.Context, repoPath, prefix, arg string, skip, limit int) (branchNames []string, countAll int, err error) {
92-
countAll, err = walkShowRef(ctx, repoPath, arg, skip, limit, func(_, branchName string) error {
93-
branchName = strings.TrimPrefix(branchName, prefix)
89+
func callShowRef(ctx context.Context, repoPath, trimPrefix string, extraArgs []string, skip, limit int) (branchNames []string, countAll int, err error) {
90+
countAll, err = walkShowRef(ctx, repoPath, extraArgs, skip, limit, func(_, branchName string) error {
91+
branchName = strings.TrimPrefix(branchName, trimPrefix)
9492
branchNames = append(branchNames, branchName)
9593

9694
return nil
9795
})
9896
return branchNames, countAll, err
9997
}
10098

101-
func walkShowRef(ctx context.Context, repoPath, arg string, skip, limit int, walkfn func(sha1, refname string) error) (countAll int, err error) {
99+
func walkShowRef(ctx context.Context, repoPath string, extraArgs []string, skip, limit int, walkfn func(sha1, refname string) error) (countAll int, err error) {
102100
stdoutReader, stdoutWriter := io.Pipe()
103101
defer func() {
104102
_ = stdoutReader.Close()
@@ -108,9 +106,7 @@ func walkShowRef(ctx context.Context, repoPath, arg string, skip, limit int, wal
108106
go func() {
109107
stderrBuilder := &strings.Builder{}
110108
args := []string{"for-each-ref", "--format=%(objectname) %(refname)"}
111-
if arg != "" {
112-
args = append(args, strings.Fields(arg)...)
113-
}
109+
args = append(args, extraArgs...)
114110
err := NewCommand(ctx, args...).Run(&RunOpts{
115111
Dir: repoPath,
116112
Stdout: stdoutWriter,
@@ -194,7 +190,7 @@ func walkShowRef(ctx context.Context, repoPath, arg string, skip, limit int, wal
194190
// GetRefsBySha returns all references filtered with prefix that belong to a sha commit hash
195191
func (repo *Repository) GetRefsBySha(sha, prefix string) ([]string, error) {
196192
var revList []string
197-
_, err := walkShowRef(repo.Ctx, repo.Path, "", 0, 0, func(walkSha, refname string) error {
193+
_, err := walkShowRef(repo.Ctx, repo.Path, nil, 0, 0, func(walkSha, refname string) error {
198194
if walkSha == sha && strings.HasPrefix(refname, prefix) {
199195
revList = append(revList, refname)
200196
}

modules/git/repo_tag_nogogit.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ func (repo *Repository) IsTagExist(name string) bool {
2626
// GetTags returns all tags of the repository.
2727
// returning at most limit tags, or all if limit is 0.
2828
func (repo *Repository) GetTags(skip, limit int) (tags []string, err error) {
29-
tags, _, err = callShowRef(repo.Ctx, repo.Path, TagPrefix, TagPrefix+" --sort=-taggerdate", skip, limit)
29+
tags, _, err = callShowRef(repo.Ctx, repo.Path, TagPrefix, []string{TagPrefix, "--sort=-taggerdate"}, skip, limit)
3030
return tags, err
3131
}
3232

0 commit comments

Comments
 (0)