HashiCorp go-getter Vulnerable to Arbitrary Read through Symlink Attack

@@ -24,11 +24,19 @@ func copyDir(ctx context.Context, dst string, src string, ignoreDot bool, disabl
 	// We can safely evaluate the symlinks here, even if disabled, because they
 	// will be checked before actual use in walkFn and copyFile
 	var err error
-	src, err = filepath.EvalSymlinks(src)
+	resolved, err := filepath.EvalSymlinks(src)
 	if err != nil {
 		return err
 	}
 
+	// Check if the resolved path tries to escape upward from the original
+	if disableSymlinks {
+		rel, err := filepath.Rel(filepath.Dir(src), resolved)
+		if err != nil || filepath.IsAbs(rel) || containsDotDot(rel) {
+			return ErrSymlinkCopy
+		}
+	}
+
 	walkFn := func(path string, info os.FileInfo, err error) error {
 		if err != nil {
 			return err
@@ -42,12 +50,9 @@ func copyDir(ctx context.Context, dst string, src string, ignoreDot bool, disabl
 			if fileInfo.Mode()&os.ModeSymlink == os.ModeSymlink {
 				return ErrSymlinkCopy
 			}
-			// if info.Mode()&os.ModeSymlink == os.ModeSymlink {
-			// 	return ErrSymlinkCopy
-			// }
 		}
 
-		if path == src {
+		if path == resolved {
 			return nil
 		}
 
@@ -62,16 +67,15 @@ func copyDir(ctx context.Context, dst string, src string, ignoreDot bool, disabl
 
 		// The "path" has the src prefixed to it. We need to join our
 		// destination with the path without the src on it.
-		dstPath := filepath.Join(dst, path[len(src):])
+		dstPath := filepath.Join(dst, path[len(resolved):])
 
 		// If we have a directory, make that subdirectory, then continue
 		// the walk.
 		if info.IsDir() {
-			if path == filepath.Join(src, dst) {
+			if path == filepath.Join(resolved, dst) {
 				// dst is in src; don't walk it.
 				return nil
 			}
-
 			if err := os.MkdirAll(dstPath, mode(0755, umask)); err != nil {
 				return err
 			}
@@ -84,5 +88,5 @@ func copyDir(ctx context.Context, dst string, src string, ignoreDot bool, disabl
 		return err
 	}
 
-	return filepath.Walk(src, walkFn)
+	return filepath.Walk(resolved, walkFn)
 }

@@ -302,6 +302,9 @@ func (g *GitGetter) update(ctx context.Context, dst, sshKeyFile string, u *url.U
 
 // fetchSubmodules downloads any configured submodules recursively.
 func (g *GitGetter) fetchSubmodules(ctx context.Context, dst, sshKeyFile string, depth int) error {
+	if g.client != nil {
+		g.client.DisableSymlinks = true
+	}
 	args := []string{"submodule", "update", "--init", "--recursive"}
 	if depth > 0 {
 		args = append(args, "--depth", strconv.Itoa(depth))

@@ -802,6 +802,66 @@ func TestGitGetter_subdirectory_symlink(t *testing.T) {
 
 }
 
+func TestGitGetter_subdirectory_malicious_symlink(t *testing.T) {
+	if !testHasGit {
+		t.Skip("git not found, skipping")
+	}
+
+	if runtime.GOOS == "windows" {
+		t.Skip("skipping on windows since the test requires sh")
+		return
+	}
+
+	g := new(GitGetter)
+	dst := tempDir(t)
+
+	repo := testGitRepo(t, "empty-repo")
+	repo.git("config", "commit.gpgsign", "false")
+
+	// Create a malicious symlink that tries to escape the repository
+	symlinkPath := filepath.Join(repo.dir, "root")
+	if err := os.Symlink("../../../../../../../../../../../", symlinkPath); err != nil {
+		t.Fatal(err)
+	}
+
+	repo.git("add", symlinkPath)
+	repo.git("commit", "-m", "Adding malicious symlink")
+
+	u, err := url.Parse(fmt.Sprintf("git::%s//root/etc/passwd", repo.url.String()))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	client := &Client{
+		Src: u.String(),
+		Dst: dst,
+		Pwd: ".",
+
+		Mode: ClientModeDir,
+
+		Detectors: []Detector{
+			new(GitDetector),
+		},
+		Getters: map[string]Getter{
+			"git": g,
+		},
+	}
+
+	err = client.Get()
+	if err == nil {
+		t.Fatalf("expected client get to fail")
+	}
+
+	if _, err := os.Stat(filepath.Join(dst, "etc", "passwd")); err == nil {
+		t.Fatalf("expected /etc/passwd to not exist in destination")
+	}
+
+	if !errors.Is(err, ErrSymlinkCopy) {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+}
+
 func TestGitGetter_subdirectory(t *testing.T) {
 	if !testHasGit {
 		t.Skip("git not found, skipping")