From 07063a2652af34f5a25a7b2df4bb06c1e8911393 Mon Sep 17 00:00:00 2001
From: Anna Song <annasong@google.com>
Date: Wed, 23 Nov 2022 14:29:22 -0800
Subject: [PATCH] Prevent repoSpec path from exiting repo

---
 api/internal/git/repospec.go      |  5 +++++
 api/internal/git/repospec_test.go |  4 ++++
 api/loader/fileloader.go          |  7 +++++++
 api/loader/fileloader_test.go     | 22 ++++++++++++++++++++++
 4 files changed, 38 insertions(+)

diff --git a/api/internal/git/repospec.go b/api/internal/git/repospec.go
index 20ca82ab5a..daaf730a60 100644
--- a/api/internal/git/repospec.go
+++ b/api/internal/git/repospec.go
@@ -93,6 +93,11 @@ func NewRepoSpecFromURL(n string) (*RepoSpec, error) {
 	if host == "" {
 		return nil, fmt.Errorf("url lacks host: %s", n)
 	}
+	cleanedPath := filepath.Clean(strings.TrimPrefix(path, string(filepath.Separator)))
+	if pathElements := strings.Split(cleanedPath, string(filepath.Separator)); len(pathElements) > 0 &&
+		pathElements[0] == filesys.ParentDir {
+		return nil, fmt.Errorf("url path exits repo: %s", n)
+	}
 	return &RepoSpec{
 		raw: n, Host: host, OrgRepo: orgRepo,
 		Dir: notCloned, Path: path, Ref: gitRef, GitSuffix: suffix,
diff --git a/api/internal/git/repospec_test.go b/api/internal/git/repospec_test.go
index 220b326f1e..57bc297065 100644
--- a/api/internal/git/repospec_test.go
+++ b/api/internal/git/repospec_test.go
@@ -101,6 +101,10 @@ func TestNewRepoSpecFromUrlErrors(t *testing.T) {
 			"https://host?ref=group/version/minor_version",
 			"url lacks orgRepo",
 		},
+		"path_exits_repo": {
+			"https://github.com/org/repo.git//path/../../exits/repo",
+			"url path exits repo",
+		},
 	}
 
 	for name, testCase := range badData {
diff --git a/api/loader/fileloader.go b/api/loader/fileloader.go
index 82f62aff17..672ca29b3c 100644
--- a/api/loader/fileloader.go
+++ b/api/loader/fileloader.go
@@ -222,6 +222,13 @@ func newLoaderAtGitClone(
 			"'%s' refers to file '%s'; expecting directory",
 			repoSpec.AbsPath(), f)
 	}
+	// Path in repo can contain symlinks that exit repo. We can only
+	// check for this after cloning repo.
+	if !root.HasPrefix(repoSpec.CloneDir()) {
+		_ = cleaner()
+		return nil, fmt.Errorf("%q refers to directory outside of repo %q", repoSpec.AbsPath(),
+			repoSpec.CloneDir())
+	}
 	return &fileLoader{
 		// Clones never allowed to escape root.
 		loadRestrictor: RestrictionRootOnly,
diff --git a/api/loader/fileloader_test.go b/api/loader/fileloader_test.go
index a13ca62bed..0cfb4679fe 100644
--- a/api/loader/fileloader_test.go
+++ b/api/loader/fileloader_test.go
@@ -5,6 +5,7 @@ package loader
 
 import (
 	"bytes"
+	"fmt"
 	"io"
 	"net/http"
 	"os"
@@ -444,6 +445,27 @@ func TestLoaderDisallowsLocalBaseFromRemoteOverlay(t *testing.T) {
 		"base '/whatever/highBase' is outside '/whatever/someClone'")
 }
 
+func TestLoaderDisallowsRemoteBaseExitRepo(t *testing.T) {
+	fSys := filesys.MakeFsOnDisk()
+	dir, err := filesys.NewTmpConfirmedDir()
+	require.NoError(t, err)
+	t.Cleanup(func() {
+		_ = fSys.RemoveAll(dir.String())
+	})
+	repo := dir.Join("repo")
+	require.NoError(t, fSys.Mkdir(repo))
+
+	base := filepath.Join(repo, "base")
+	require.NoError(t, os.Symlink(dir.String(), base))
+
+	repoSpec, err := git.NewRepoSpecFromURL("https://github.com/org/repo/base")
+	require.NoError(t, err)
+
+	_, err = newLoaderAtGitClone(repoSpec, fSys, nil, git.DoNothingCloner(filesys.ConfirmedDir(repo)))
+	require.Error(t, err)
+	require.Contains(t, err.Error(), fmt.Sprintf("%q refers to directory outside of repo %q", base, repo))
+}
+
 func TestLocalLoaderReferencingGitBase(t *testing.T) {
 	require := require.New(t)