Skip to content

Commit 96a3124

Browse files
caarlos0unknwon
caarlos0
and
unknwon
authored
sec: added --end-of-options to prevent unintended options (#118)
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com> Co-authored-by: ᴊᴏᴇ ᴄʜᴇɴ <jc@unknwon.io>
1 parent 53cdcd7 commit 96a3124

File tree

8 files changed

+44
-31
lines changed

8 files changed

+44
-31
lines changed

‎repo.go‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ func Init(path string, opts ...InitOptions) error {
8383
if opt.Bare {
8484
cmd.AddArgs("--bare")
8585
}
86+
cmd.AddArgs("--end-of-options")
8687
_, err = cmd.RunInDirWithTimeout(opt.Timeout, path)
8788
return err
8889
}
@@ -157,6 +158,7 @@ func Clone(url, dst string, opts ...CloneOptions) error {
157158
cmd.AddArgs("--depth", strconv.FormatUint(opt.Depth, 10))
158159
}
159160

161+
cmd.AddArgs("--end-of-options")
160162
_, err = cmd.AddArgs(url, dst).RunWithTimeout(opt.Timeout)
161163
return err
162164
}
@@ -259,7 +261,7 @@ func Push(repoPath, remote, branch string, opts ...PushOptions) error {
259261
opt = opts[0]
260262
}
261263

262-
cmd := NewCommand("push").AddOptions(opt.CommandOptions).AddArgs(remote, branch)
264+
cmd := NewCommand("push").AddOptions(opt.CommandOptions).AddArgs("--end-of-options", remote, branch)
263265
_, err := cmd.RunInDirWithTimeout(opt.Timeout, repoPath)
264266
return err
265267
}
@@ -346,7 +348,7 @@ func Reset(repoPath, rev string, opts ...ResetOptions) error {
346348
cmd.AddArgs("--hard")
347349
}
348350

349-
_, err := cmd.AddOptions(opt.CommandOptions).AddArgs(rev).RunInDir(repoPath)
351+
_, err := cmd.AddOptions(opt.CommandOptions).AddArgs("--end-of-options", rev).RunInDir(repoPath)
350352
return err
351353
}
352354

@@ -382,7 +384,7 @@ func Move(repoPath, src, dst string, opts ...MoveOptions) error {
382384
opt = opts[0]
383385
}
384386

385-
_, err := NewCommand("mv").AddOptions(opt.CommandOptions).AddArgs(src, dst).RunInDirWithTimeout(opt.Timeout, repoPath)
387+
_, err := NewCommand("mv").AddOptions(opt.CommandOptions).AddArgs("--end-of-options", src, dst).RunInDirWithTimeout(opt.Timeout, repoPath)
386388
return err
387389
}
388390

@@ -549,7 +551,7 @@ func ShowNameStatus(repoPath, rev string, opts ...ShowNameStatusOptions) (*NameS
549551
stderr := new(bytes.Buffer)
550552
cmd := NewCommand("show", "--name-status", "--pretty=format:''").
551553
AddOptions(opt.CommandOptions).
552-
AddArgs(rev)
554+
AddArgs("--end-of-options", rev)
553555
err := cmd.RunInDirPipelineWithTimeout(opt.Timeout, w, stderr, repoPath)
554556
_ = w.Close() // Close writer to exit parsing goroutine
555557
if err != nil {

‎repo_commit.go‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ func (r *Repository) Log(rev string, opts ...LogOptions) ([]*Commit, error) {
221221

222222
cmd := NewCommand("log").
223223
AddOptions(opt.CommandOptions).
224-
AddArgs("--pretty="+LogFormatHashOnly, rev)
224+
AddArgs("--pretty=" + LogFormatHashOnly)
225225
if opt.MaxCount > 0 {
226226
cmd.AddArgs("--max-count=" + strconv.Itoa(opt.MaxCount))
227227
}
@@ -237,7 +237,7 @@ func (r *Repository) Log(rev string, opts ...LogOptions) ([]*Commit, error) {
237237
if opt.RegexpIgnoreCase {
238238
cmd.AddArgs("--regexp-ignore-case")
239239
}
240-
cmd.AddArgs("--")
240+
cmd.AddArgs("--end-of-options", rev, "--")
241241
if opt.Path != "" {
242242
cmd.AddArgs(escapePath(opt.Path))
243243
}
@@ -406,6 +406,7 @@ func DiffNameOnly(repoPath, base, head string, opts ...DiffNameOnlyOptions) ([]s
406406
cmd := NewCommand("diff").
407407
AddOptions(opt.CommandOptions).
408408
AddArgs("--name-only")
409+
cmd.AddArgs("--end-of-options")
409410
if opt.NeedsMergeBase {
410411
cmd.AddArgs(base + "..." + head)
411412
} else {
@@ -471,7 +472,10 @@ func (r *Repository) RevListCount(refspecs []string, opts ...RevListCountOptions
471472

472473
cmd := NewCommand("rev-list").
473474
AddOptions(opt.CommandOptions).
474-
AddArgs("--count")
475+
AddArgs(
476+
"--count",
477+
"--end-of-options",
478+
)
475479
cmd.AddArgs(refspecs...)
476480
cmd.AddArgs("--")
477481
if opt.Path != "" {
@@ -512,6 +516,7 @@ func (r *Repository) RevList(refspecs []string, opts ...RevListOptions) ([]*Comm
512516
}
513517

514518
cmd := NewCommand("rev-list").AddOptions(opt.CommandOptions)
519+
cmd.AddArgs("--end-of-options")
515520
cmd.AddArgs(refspecs...)
516521
cmd.AddArgs("--")
517522
if opt.Path != "" {

‎repo_diff.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,20 +45,20 @@ func (r *Repository) Diff(rev string, maxFiles, maxFileLines, maxLineChars int,
4545
if commit.ParentsCount() == 0 {
4646
cmd = cmd.AddArgs("show").
4747
AddOptions(opt.CommandOptions).
48-
AddArgs("--full-index", rev)
48+
AddArgs("--full-index", "--end-of-options", rev)
4949
} else {
5050
c, err := commit.Parent(0)
5151
if err != nil {
5252
return nil, err
5353
}
5454
cmd = cmd.AddArgs("diff").
5555
AddOptions(opt.CommandOptions).
56-
AddArgs("--full-index", "-M", c.ID.String(), rev)
56+
AddArgs("--full-index", "-M", c.ID.String(), "--end-of-options", rev)
5757
}
5858
} else {
5959
cmd = cmd.AddArgs("diff").
6060
AddOptions(opt.CommandOptions).
61-
AddArgs("--full-index", "-M", opt.Base, rev)
61+
AddArgs("--full-index", "-M", opt.Base, "--end-of-options", rev)
6262
}
6363

6464
stdout, w := io.Pipe()
@@ -114,29 +114,29 @@ func (r *Repository) RawDiff(rev string, diffType RawDiffFormat, w io.Writer, op
114114
if commit.ParentsCount() == 0 {
115115
cmd = cmd.AddArgs("show").
116116
AddOptions(opt.CommandOptions).
117-
AddArgs("--full-index", rev)
117+
AddArgs("--full-index", "--end-of-options", rev)
118118
} else {
119119
c, err := commit.Parent(0)
120120
if err != nil {
121121
return err
122122
}
123123
cmd = cmd.AddArgs("diff").
124124
AddOptions(opt.CommandOptions).
125-
AddArgs("--full-index", "-M", c.ID.String(), rev)
125+
AddArgs("--full-index", "-M", c.ID.String(), "--end-of-options", rev)
126126
}
127127
case RawDiffPatch:
128128
if commit.ParentsCount() == 0 {
129129
cmd = cmd.AddArgs("format-patch").
130130
AddOptions(opt.CommandOptions).
131-
AddArgs("--full-index", "--no-signoff", "--no-signature", "--stdout", "--root", rev)
131+
AddArgs("--full-index", "--no-signoff", "--no-signature", "--stdout", "--root", "--end-of-options", rev)
132132
} else {
133133
c, err := commit.Parent(0)
134134
if err != nil {
135135
return err
136136
}
137137
cmd = cmd.AddArgs("format-patch").
138138
AddOptions(opt.CommandOptions).
139-
AddArgs("--full-index", "--no-signoff", "--no-signature", "--stdout", rev+"..."+c.ID.String())
139+
AddArgs("--full-index", "--no-signoff", "--no-signature", "--stdout", "--end-of-options", rev+"..."+c.ID.String())
140140
}
141141
default:
142142
return fmt.Errorf("invalid diffType: %s", diffType)

‎repo_grep.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,11 @@ func (r *Repository) Grep(pattern string, opts ...GrepOptions) []*GrepResult {
9696
if opt.ExtendedRegexp {
9797
cmd.AddArgs("--extended-regexp")
9898
}
99-
cmd.AddArgs(pattern, opt.Tree)
99+
cmd.AddArgs(
100+
"--end-of-options",
101+
pattern,
102+
opt.Tree,
103+
)
100104
if opt.Pathspec != "" {
101105
cmd.AddArgs("--", opt.Pathspec)
102106
}

‎repo_pull.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,11 @@ func MergeBase(repoPath, base, head string, opts ...MergeBaseOptions) (string, e
3232

3333
stdout, err := NewCommand("merge-base").
3434
AddOptions(opt.CommandOptions).
35-
AddArgs(base, head).
36-
RunInDirWithTimeout(opt.Timeout, repoPath)
35+
AddArgs(
36+
"--end-of-options",
37+
base,
38+
head,
39+
).RunInDirWithTimeout(opt.Timeout, repoPath)
3740
if err != nil {
3841
if strings.Contains(err.Error(), "exit status 1") {
3942
return "", ErrNoMergeBase

‎repo_reference.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func ShowRefVerify(repoPath, ref string, opts ...ShowRefVerifyOptions) (string,
5656
opt = opts[0]
5757
}
5858

59-
cmd := NewCommand("show-ref", "--verify", ref).AddOptions(opt.CommandOptions)
59+
cmd := NewCommand("show-ref", "--verify", "--end-of-options", ref).AddOptions(opt.CommandOptions)
6060
stdout, err := cmd.RunInDirWithTimeout(opt.Timeout, repoPath)
6161
if err != nil {
6262
if strings.Contains(err.Error(), "not a valid ref") {
@@ -162,7 +162,7 @@ func SymbolicRef(repoPath string, opts ...SymbolicRefOptions) (string, error) {
162162
if opt.Name == "" {
163163
opt.Name = "HEAD"
164164
}
165-
cmd.AddArgs(opt.Name)
165+
cmd.AddArgs("--end-of-options", opt.Name)
166166
if opt.Ref != "" {
167167
cmd.AddArgs(opt.Ref)
168168
}
@@ -281,7 +281,7 @@ func DeleteBranch(repoPath, name string, opts ...DeleteBranchOptions) error {
281281
} else {
282282
cmd.AddArgs("-d")
283283
}
284-
_, err := cmd.AddArgs(name).RunInDirWithTimeout(opt.Timeout, repoPath)
284+
_, err := cmd.AddArgs("--end-of-options", name).RunInDirWithTimeout(opt.Timeout, repoPath)
285285
return err
286286
}
287287

‎repo_remote.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ func LsRemote(url string, opts ...LsRemoteOptions) ([]*Reference, error) {
4949
if opt.Refs {
5050
cmd.AddArgs("--refs")
5151
}
52-
cmd.AddArgs(url)
52+
cmd.AddArgs("--end-of-options", url)
5353
if len(opt.Patterns) > 0 {
5454
cmd.AddArgs(opt.Patterns...)
5555
}
@@ -121,7 +121,7 @@ func RemoteAdd(repoPath, name, url string, opts ...RemoteAddOptions) error {
121121
cmd.AddArgs("--mirror=fetch")
122122
}
123123

124-
_, err := cmd.AddArgs(name, url).RunInDirWithTimeout(opt.Timeout, repoPath)
124+
_, err := cmd.AddArgs("--end-of-options", name, url).RunInDirWithTimeout(opt.Timeout, repoPath)
125125
return err
126126
}
127127

@@ -166,7 +166,7 @@ func RemoteRemove(repoPath, name string, opts ...RemoteRemoveOptions) error {
166166

167167
_, err := NewCommand("remote", "remove").
168168
AddOptions(opt.CommandOptions).
169-
AddArgs(name).
169+
AddArgs("--end-of-options", name).
170170
RunInDirWithTimeout(opt.Timeout, repoPath)
171171
if err != nil {
172172
// the error status may differ from git clients
@@ -263,7 +263,7 @@ func RemoteGetURL(repoPath, name string, opts ...RemoteGetURLOptions) ([]string,
263263
cmd.AddArgs("--all")
264264
}
265265

266-
stdout, err := cmd.AddArgs(name).RunInDirWithTimeout(opt.Timeout, repoPath)
266+
stdout, err := cmd.AddArgs("--end-of-options", name).RunInDirWithTimeout(opt.Timeout, repoPath)
267267
if err != nil {
268268
return nil, err
269269
}
@@ -306,7 +306,7 @@ func RemoteSetURL(repoPath, name, newurl string, opts ...RemoteSetURLOptions) er
306306
cmd.AddArgs("--push")
307307
}
308308

309-
cmd.AddArgs(name, newurl)
309+
cmd.AddArgs("--end-of-options", name, newurl)
310310

311311
if opt.Regex != "" {
312312
cmd.AddArgs(opt.Regex)
@@ -361,7 +361,7 @@ func RemoteSetURLAdd(repoPath, name, newurl string, opts ...RemoteSetURLAddOptio
361361
cmd.AddArgs("--push")
362362
}
363363

364-
cmd.AddArgs(name, newurl)
364+
cmd.AddArgs("--end-of-options", name, newurl)
365365

366366
_, err := cmd.RunInDirWithTimeout(opt.Timeout, repoPath)
367367
if err != nil && strings.Contains(err.Error(), "Will not delete all non-push URLs") {
@@ -407,7 +407,7 @@ func RemoteSetURLDelete(repoPath, name, regex string, opts ...RemoteSetURLDelete
407407
cmd.AddArgs("--push")
408408
}
409409

410-
cmd.AddArgs(name, regex)
410+
cmd.AddArgs("--end-of-options", name, regex)
411411

412412
_, err := cmd.RunInDirWithTimeout(opt.Timeout, repoPath)
413413
if err != nil && strings.Contains(err.Error(), "Will not delete all non-push URLs") {

‎repo_tag.go‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -247,10 +247,9 @@ func (r *Repository) CreateTag(name, rev string, opts ...CreateTagOptions) error
247247
if opt.Author != nil {
248248
cmd.AddCommitter(opt.Author)
249249
}
250-
} else {
251-
// 🚨 SECURITY: Prevent including unintended options in the path to the Git command.
252250
cmd.AddArgs("--end-of-options")
253-
cmd.AddArgs(name)
251+
} else {
252+
cmd.AddArgs("--end-of-options", name)
254253
}
255254

256255
cmd.AddArgs(rev)
@@ -279,7 +278,7 @@ func (r *Repository) DeleteTag(name string, opts ...DeleteTagOptions) error {
279278
opt = opts[0]
280279
}
281280

282-
_, err := NewCommand("tag", "--delete", name).
281+
_, err := NewCommand("tag", "--delete", "--end-of-options", name).
283282
AddOptions(opt.CommandOptions).
284283
RunInDirWithTimeout(opt.Timeout, r.path)
285284
return err

0 commit comments

Comments
 (0)