Skip to content

Commit 87541b2

Browse files
fix: go-getter subdir paths (#540)
* fix: go-getter subdir symlink content * disable symlink in git client * fix disable symlink for git subdirs * ignore for windows since it defaults core.symlinks to false * Update copy_dir.go Co-authored-by: Michael Schurter <[email protected]> --------- Co-authored-by: Michael Schurter <[email protected]>
1 parent 3713030 commit 87541b2

File tree

3 files changed

+76
-9
lines changed

3 files changed

+76
-9
lines changed

copy_dir.go

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,19 @@ func copyDir(ctx context.Context, dst string, src string, ignoreDot bool, disabl
2424
// We can safely evaluate the symlinks here, even if disabled, because they
2525
// will be checked before actual use in walkFn and copyFile
2626
var err error
27-
src, err = filepath.EvalSymlinks(src)
27+
resolved, err := filepath.EvalSymlinks(src)
2828
if err != nil {
2929
return err
3030
}
3131

32+
// Check if the resolved path tries to escape upward from the original
33+
if disableSymlinks {
34+
rel, err := filepath.Rel(filepath.Dir(src), resolved)
35+
if err != nil || filepath.IsAbs(rel) || containsDotDot(rel) {
36+
return ErrSymlinkCopy
37+
}
38+
}
39+
3240
walkFn := func(path string, info os.FileInfo, err error) error {
3341
if err != nil {
3442
return err
@@ -42,12 +50,9 @@ func copyDir(ctx context.Context, dst string, src string, ignoreDot bool, disabl
4250
if fileInfo.Mode()&os.ModeSymlink == os.ModeSymlink {
4351
return ErrSymlinkCopy
4452
}
45-
// if info.Mode()&os.ModeSymlink == os.ModeSymlink {
46-
// return ErrSymlinkCopy
47-
// }
4853
}
4954

50-
if path == src {
55+
if path == resolved {
5156
return nil
5257
}
5358

@@ -62,16 +67,15 @@ func copyDir(ctx context.Context, dst string, src string, ignoreDot bool, disabl
6267

6368
// The "path" has the src prefixed to it. We need to join our
6469
// destination with the path without the src on it.
65-
dstPath := filepath.Join(dst, path[len(src):])
70+
dstPath := filepath.Join(dst, path[len(resolved):])
6671

6772
// If we have a directory, make that subdirectory, then continue
6873
// the walk.
6974
if info.IsDir() {
70-
if path == filepath.Join(src, dst) {
75+
if path == filepath.Join(resolved, dst) {
7176
// dst is in src; don't walk it.
7277
return nil
7378
}
74-
7579
if err := os.MkdirAll(dstPath, mode(0755, umask)); err != nil {
7680
return err
7781
}
@@ -84,5 +88,5 @@ func copyDir(ctx context.Context, dst string, src string, ignoreDot bool, disabl
8488
return err
8589
}
8690

87-
return filepath.Walk(src, walkFn)
91+
return filepath.Walk(resolved, walkFn)
8892
}

get_git.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -302,6 +302,9 @@ func (g *GitGetter) update(ctx context.Context, dst, sshKeyFile string, u *url.U
302302

303303
// fetchSubmodules downloads any configured submodules recursively.
304304
func (g *GitGetter) fetchSubmodules(ctx context.Context, dst, sshKeyFile string, depth int) error {
305+
if g.client != nil {
306+
g.client.DisableSymlinks = true
307+
}
305308
args := []string{"submodule", "update", "--init", "--recursive"}
306309
if depth > 0 {
307310
args = append(args, "--depth", strconv.Itoa(depth))

get_git_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -802,6 +802,66 @@ func TestGitGetter_subdirectory_symlink(t *testing.T) {
802802

803803
}
804804

805+
func TestGitGetter_subdirectory_malicious_symlink(t *testing.T) {
806+
if !testHasGit {
807+
t.Skip("git not found, skipping")
808+
}
809+
810+
if runtime.GOOS == "windows" {
811+
t.Skip("skipping on windows since the test requires sh")
812+
return
813+
}
814+
815+
g := new(GitGetter)
816+
dst := tempDir(t)
817+
818+
repo := testGitRepo(t, "empty-repo")
819+
repo.git("config", "commit.gpgsign", "false")
820+
821+
// Create a malicious symlink that tries to escape the repository
822+
symlinkPath := filepath.Join(repo.dir, "root")
823+
if err := os.Symlink("../../../../../../../../../../../", symlinkPath); err != nil {
824+
t.Fatal(err)
825+
}
826+
827+
repo.git("add", symlinkPath)
828+
repo.git("commit", "-m", "Adding malicious symlink")
829+
830+
u, err := url.Parse(fmt.Sprintf("git::%s//root/etc/passwd", repo.url.String()))
831+
if err != nil {
832+
t.Fatal(err)
833+
}
834+
835+
client := &Client{
836+
Src: u.String(),
837+
Dst: dst,
838+
Pwd: ".",
839+
840+
Mode: ClientModeDir,
841+
842+
Detectors: []Detector{
843+
new(GitDetector),
844+
},
845+
Getters: map[string]Getter{
846+
"git": g,
847+
},
848+
}
849+
850+
err = client.Get()
851+
if err == nil {
852+
t.Fatalf("expected client get to fail")
853+
}
854+
855+
if _, err := os.Stat(filepath.Join(dst, "etc", "passwd")); err == nil {
856+
t.Fatalf("expected /etc/passwd to not exist in destination")
857+
}
858+
859+
if !errors.Is(err, ErrSymlinkCopy) {
860+
t.Fatalf("unexpected error: %v", err)
861+
}
862+
863+
}
864+
805865
func TestGitGetter_subdirectory(t *testing.T) {
806866
if !testHasGit {
807867
t.Skip("git not found, skipping")

0 commit comments

Comments
 (0)