From 27c96a4044f05a338d6ac6187135e6b9ac487e9f Mon Sep 17 00:00:00 2001 From: Michael Muré Date: Sat, 4 Mar 2023 13:10:38 +0100 Subject: repo: improve support for gitdir indirection - add a limited reader to avoid abuse - support recusive indirection (up to depth 10) - check that the pointed to repo does exist --- repository/gogit.go | 19 ++++++++++++++----- repository/gogit_test.go | 22 +++++++++------------- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/repository/gogit.go b/repository/gogit.go index 01eace6d..96d62665 100644 --- a/repository/gogit.go +++ b/repository/gogit.go @@ -5,6 +5,7 @@ import ( "bytes" "errors" "fmt" + "io" "io/ioutil" "os" "path/filepath" @@ -56,7 +57,7 @@ type GoGitRepo struct { // of "~/myrepo" and a namespace of "git-bug", local storage for the // GoGitRepo will be configured at "~/myrepo/.git/git-bug". func OpenGoGitRepo(path, namespace string, clockLoaders []ClockLoader) (*GoGitRepo, error) { - path, err := detectGitPath(path) + path, err := detectGitPath(path, 0) if err != nil { return nil, err } @@ -160,7 +161,11 @@ func InitBareGoGitRepo(path, namespace string) (*GoGitRepo, error) { }, nil } -func detectGitPath(path string) (string, error) { +func detectGitPath(path string, depth int) (string, error) { + if depth >= 10 { + return "", fmt.Errorf("gitdir loop detected") + } + // normalize the path path, err := filepath.Abs(path) if err != nil { @@ -179,18 +184,22 @@ func detectGitPath(path string) (string, error) { } // We aren't going to defer the dotfile.Close, because we might keep looping, so we have to be sure to // clean up before returning an error - reader := bufio.NewReader(dotfile) + reader := bufio.NewReader(io.LimitReader(dotfile, 2048)) line, _, err := reader.ReadLine() _ = dotfile.Close() if err != nil { - return "", fmt.Errorf(".git exists but is not a direcctory and cannot be read: %w", err) + return "", fmt.Errorf(".git exists but is not a directory and cannot be read: %w", err) } dotContent := string(line) if strings.HasPrefix(dotContent, "gitdir:") { // This is a submodule parent path link. Strip the prefix, clean the string of whitespace just to // be safe, and return dotContent = strings.TrimSpace(strings.TrimPrefix(dotContent, "gitdir: ")) - return dotContent, nil + p, err := detectGitPath(dotContent, depth+1) + if err != nil { + return "", fmt.Errorf(".git gitdir error: %w", err) + } + return p, nil } return "", fmt.Errorf(".git exist but is not a directory or module/workspace file") } diff --git a/repository/gogit_test.go b/repository/gogit_test.go index 2bec97a5..21acd5df 100644 --- a/repository/gogit_test.go +++ b/repository/gogit_test.go @@ -1,6 +1,7 @@ package repository import ( + "fmt" "os" "path" "path/filepath" @@ -84,19 +85,14 @@ func TestGoGitRepo_Indexes(t *testing.T) { } func TestGoGit_DetectsSubmodules(t *testing.T) { - expectedPath := "../foo/bar" - submoduleData := "gitdir: " + expectedPath + repo := CreateGoGitTestRepo(t, false) + expected := filepath.Join(goGitRepoDir(t, repo), "/.git") + d := t.TempDir() - if f, err := os.Create(filepath.Join(d, ".git")); err != nil { - t.Fatal("could not create necessary temp file:", err) - } else { - t.Log(f.Name()) - if _, err := f.Write([]byte(submoduleData)); err != nil { - t.Fatal("could not write necessary data to temp file:", err) - } - _ = f.Close() - } - result, err := detectGitPath(d) + err := os.WriteFile(filepath.Join(d, ".git"), []byte(fmt.Sprintf("gitdir: %s", expected)), 0600) + require.NoError(t, err) + + result, err := detectGitPath(d, 0) assert.Empty(t, err) - assert.Equal(t, expectedPath, result) + assert.Equal(t, expected, result) } -- cgit