Skip to content

Commit 11a3ef7

Browse files
wsutinawsutina
authored
fix: prevent Git argument injection RCE (#542)
## Problem Malicious Git URLs starting with `--` could inject command-line arguments into git commands, potentially executing arbitrary code during package installation or validation. ## Fixes 1. Validate URLs - Reject URLs starting with `-` 2. Argument separator - Add `--` in all git commands to separate options from arguments ## Test ``` description = "Malicious package" binaries = ["evil"] source = "--upload-pack=sh -c 'echo PWNED > /tmp/hermit-pwned' #https://github.com/fake/repo.git" version "1.0.0" { } ``` <img width="1829" height="423" alt="Screenshot 2026-02-17 at 5 03 20 pm" src="https://github.com/user-attachments/assets/d520134a-4c3f-4d78-9977-3e2dc758060d" />
1 parent d28d905 commit 11a3ef7

2 files changed

Lines changed: 122 additions & 11 deletions

File tree

‎cache/git.go‎

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,16 @@ func (s *gitSource) OpenLocal(c *Cache, checksum string) (*os.File, error) {
2323
func (s *gitSource) Download(b *ui.Task, cache *Cache, checksum string) (string, string, string, error) {
2424
base := BasePath(checksum, s.URL)
2525
checkoutDir := filepath.Join(cache.root, base)
26-
repo, tag := parseGitURL(s.URL)
27-
args := []string{"git", "clone", "--depth=1", repo, checkoutDir}
26+
repo, tag, err := parseGitURL(s.URL)
27+
if err != nil {
28+
return "", "", "", err
29+
}
30+
args := []string{"git", "clone", "--depth=1"}
2831
if tag != "" {
2932
args = append(args, "--branch="+tag)
3033
}
31-
err := util.RunInDir(b, cache.root, args...)
34+
args = append(args, "--", repo, checkoutDir)
35+
err = util.RunInDir(b, cache.root, args...)
3236
if err != nil {
3337
return "", "", "", errors.WithStack(err)
3438
}
@@ -43,11 +47,14 @@ func (s *gitSource) Download(b *ui.Task, cache *Cache, checksum string) (string,
4347
}
4448

4549
func (s *gitSource) ETag(b *ui.Task) (etag string, err error) {
46-
repo, tag := parseGitURL(s.URL)
50+
repo, tag, err := parseGitURL(s.URL)
51+
if err != nil {
52+
return "", err
53+
}
4754
if tag == "" {
4855
tag = "HEAD"
4956
}
50-
bts, err := util.Capture(b, "git", "ls-remote", repo, tag)
57+
bts, err := util.Capture(b, "git", "ls-remote", "--", repo, tag)
5158
if err != nil {
5259
return "", errors.Wrap(err, s.URL)
5360
}
@@ -61,23 +68,32 @@ func (s *gitSource) ETag(b *ui.Task) (etag string, err error) {
6168
}
6269

6370
func (s *gitSource) Validate() error {
64-
repo, tag := parseGitURL(s.URL)
71+
repo, tag, err := parseGitURL(s.URL)
72+
if err != nil {
73+
return err
74+
}
6575
if tag == "" {
6676
tag = "HEAD"
6777
}
68-
cmd := exec.Command("git", "ls-remote", repo, tag)
78+
cmd := exec.Command("git", "ls-remote", "--", repo, tag)
6979
out, err := cmd.CombinedOutput()
7080
if err != nil {
7181
return errors.Wrapf(err, "error getting remote HEAD: %s", string(out))
7282
}
7383
return nil
7484
}
7585

76-
func parseGitURL(source string) (repo, tag string) {
86+
func parseGitURL(source string) (repo, tag string, err error) {
7787
parts := strings.SplitN(source, "#", 2)
7888
repo = parts[0]
89+
90+
// Validate repo doesn't start with dash to prevent argument injection
91+
if strings.HasPrefix(repo, "-") {
92+
return "", "", errors.Errorf("invalid git URL: repository cannot start with '-': %s", repo)
93+
}
94+
7995
if len(parts) > 1 {
8096
tag = parts[1]
8197
}
82-
return
98+
return repo, tag, nil
8399
}

‎cache/source_test.go‎

Lines changed: 97 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,111 @@
11
package cache
22

33
import (
4+
"os"
5+
"os/exec"
6+
"path/filepath"
47
"testing"
58

69
"github.com/alecthomas/assert/v2"
710
)
811

912
func TestGitParseRepo(t *testing.T) {
10-
repo, tag := parseGitURL("org-49461806@github.com:squareup/orc.git")
13+
repo, tag, err := parseGitURL("org-49461806@github.com:squareup/orc.git")
14+
assert.NoError(t, err)
1115
assert.Equal(t, "org-49461806@github.com:squareup/orc.git", repo)
1216
assert.Equal(t, "", tag)
13-
repo, tag = parseGitURL("org-49461806@github.com:squareup/orc.git#v1.2.3")
17+
repo, tag, err = parseGitURL("org-49461806@github.com:squareup/orc.git#v1.2.3")
18+
assert.NoError(t, err)
1419
assert.Equal(t, "org-49461806@github.com:squareup/orc.git", repo)
1520
assert.Equal(t, "v1.2.3", tag)
1621
}
22+
23+
func TestParseGitURLArgumentInjection(t *testing.T) {
24+
tests := []struct {
25+
url string
26+
expectError bool
27+
}{
28+
{"--upload-pack=sh -c 'echo OWNED' #file:///tmp/repo/.git", true},
29+
{"--config core.sshCommand='touch /tmp/pwned' git@github.com:fake/repo.git", true},
30+
{"-v https://github.com/user/repo.git", true},
31+
{"https://github.com/cashapp/hermit.git#v1.0.0", false},
32+
{"git@github.com:cashapp/hermit.git#main", false},
33+
{"file:///path/to/repo.git", false},
34+
}
35+
36+
for _, tt := range tests {
37+
_, _, err := parseGitURL(tt.url)
38+
if tt.expectError {
39+
assert.Error(t, err, "Should reject: "+tt.url)
40+
} else {
41+
assert.NoError(t, err, "Should accept: "+tt.url)
42+
}
43+
}
44+
}
45+
46+
// TestGitSourcePreventRCE verifies all git operations reject malicious URLs.
47+
func TestGitSourcePreventRCE(t *testing.T) {
48+
tmpDir := t.TempDir()
49+
pwnedFile := filepath.Join(tmpDir, "pwned")
50+
maliciousURL := "--upload-pack=sh -c 'echo OWNED > " + pwnedFile + "' #file://" + tmpDir + "/.git"
51+
52+
src := &gitSource{URL: maliciousURL}
53+
err := src.Validate()
54+
assert.Error(t, err)
55+
56+
cache := &Cache{root: tmpDir}
57+
_, _, _, err = src.Download(nil, cache, "test")
58+
assert.Error(t, err)
59+
60+
_, err = src.ETag(nil)
61+
assert.Error(t, err)
62+
63+
_, fileErr := os.Stat(pwnedFile)
64+
assert.True(t, os.IsNotExist(fileErr))
65+
}
66+
67+
// TestGitSourceRCEAttempt simulates a real attack with an actual git repository.
68+
func TestGitSourceRCEAttempt(t *testing.T) {
69+
if _, err := exec.LookPath("git"); err != nil {
70+
t.Skip("git command not found")
71+
}
72+
73+
tmpDir := t.TempDir()
74+
repoDir := filepath.Join(tmpDir, "repo")
75+
assert.NoError(t, os.MkdirAll(repoDir, 0750))
76+
assert.NoError(t, exec.Command("git", "init", repoDir).Run())
77+
78+
pwnedFile := filepath.Join(tmpDir, "pwned")
79+
payload := "--upload-pack=sh -c 'echo OWNED > " + pwnedFile + "' #file://" + repoDir + "/.git"
80+
81+
src := &gitSource{URL: payload}
82+
err := src.Validate()
83+
84+
assert.Error(t, err)
85+
assert.Contains(t, err.Error(), "invalid git URL")
86+
87+
_, fileErr := os.Stat(pwnedFile)
88+
if !os.IsNotExist(fileErr) {
89+
t.Fatal("SECURITY FAILURE: RCE was NOT prevented!")
90+
}
91+
}
92+
93+
func TestGitURLParsing(t *testing.T) {
94+
tests := []struct {
95+
url string
96+
repo string
97+
tag string
98+
}{
99+
{"org-49461806@github.com:squareup/orc.git", "org-49461806@github.com:squareup/orc.git", ""},
100+
{"org-49461806@github.com:squareup/orc.git#v1.2.3", "org-49461806@github.com:squareup/orc.git", "v1.2.3"},
101+
{"https://github.com/cashapp/hermit.git#main", "https://github.com/cashapp/hermit.git", "main"},
102+
{"file:///home/user/repo.git#develop", "file:///home/user/repo.git", "develop"},
103+
}
104+
105+
for _, tt := range tests {
106+
repo, tag, err := parseGitURL(tt.url)
107+
assert.NoError(t, err)
108+
assert.Equal(t, tt.repo, repo)
109+
assert.Equal(t, tt.tag, tag)
110+
}
111+
}

0 commit comments

Comments
 (0)