From ce51a8e538817b173a1e0aa3549e390d665f340b Mon Sep 17 00:00:00 2001 From: Joe Chen Date: Sun, 22 Dec 2024 15:59:03 -0500 Subject: [PATCH] repo: ignore unintended Git options for diff preview (#7871) ## Describe the pull request Fixes https://github.com/gogs/gogs/security/advisories/GHSA-9pp6-wq8c-3w2c --- internal/db/repo_editor.go | 13 +++++++------ internal/route/repo/editor.go | 3 ++- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/internal/db/repo_editor.go b/internal/db/repo_editor.go index 2b3254112..a7786822f 100644 --- a/internal/db/repo_editor.go +++ b/internal/db/repo_editor.go @@ -119,7 +119,7 @@ type UpdateRepoFileOptions struct { // UpdateRepoFile adds or updates a file in repository. func (repo *Repository) UpdateRepoFile(doer *User, opts UpdateRepoFileOptions) (err error) { - // 🚨 SECURITY: Prevent uploading files into the ".git" directory + // 🚨 SECURITY: Prevent uploading files into the ".git" directory. if isRepositoryGitPath(opts.NewTreeName) { return errors.Errorf("bad tree path %q", opts.NewTreeName) } @@ -220,7 +220,7 @@ func (repo *Repository) UpdateRepoFile(doer *User, opts UpdateRepoFileOptions) ( // GetDiffPreview produces and returns diff result of a file which is not yet committed. func (repo *Repository) GetDiffPreview(branch, treePath, content string) (diff *gitutil.Diff, err error) { - // 🚨 SECURITY: Prevent uploading files into the ".git" directory + // 🚨 SECURITY: Prevent uploading files into the ".git" directory. if isRepositoryGitPath(treePath) { return nil, errors.Errorf("bad tree path %q", treePath) } @@ -243,7 +243,8 @@ func (repo *Repository) GetDiffPreview(branch, treePath, content string) (diff * return nil, fmt.Errorf("write file: %v", err) } - cmd := exec.Command("git", "diff", treePath) + // 🚨 SECURITY: Prevent including unintended options in the path to the git command. + cmd := exec.Command("git", "diff", "--end-of-options", treePath) cmd.Dir = localPath cmd.Stderr = os.Stderr @@ -288,7 +289,7 @@ type DeleteRepoFileOptions struct { } func (repo *Repository) DeleteRepoFile(doer *User, opts DeleteRepoFileOptions) (err error) { - // 🚨 SECURITY: Prevent uploading files into the ".git" directory + // 🚨 SECURITY: Prevent uploading files into the ".git" directory. if isRepositoryGitPath(opts.TreePath) { return errors.Errorf("bad tree path %q", opts.TreePath) } @@ -513,7 +514,7 @@ func (repo *Repository) UploadRepoFiles(doer *User, opts UploadRepoFileOptions) return nil } - // 🚨 SECURITY: Prevent uploading files into the ".git" directory + // 🚨 SECURITY: Prevent uploading files into the ".git" directory. if isRepositoryGitPath(opts.TreePath) { return errors.Errorf("bad tree path %q", opts.TreePath) } @@ -554,7 +555,7 @@ func (repo *Repository) UploadRepoFiles(doer *User, opts UploadRepoFileOptions) // 🚨 SECURITY: Prevent path traversal. upload.Name = pathutil.Clean(upload.Name) - // 🚨 SECURITY: Prevent uploading files into the ".git" directory + // 🚨 SECURITY: Prevent uploading files into the ".git" directory. if isRepositoryGitPath(upload.Name) { continue } diff --git a/internal/route/repo/editor.go b/internal/route/repo/editor.go index bc9da505e..5426d0d67 100644 --- a/internal/route/repo/editor.go +++ b/internal/route/repo/editor.go @@ -302,7 +302,8 @@ func NewFilePost(c *context.Context, f form.EditRepoFile) { } func DiffPreviewPost(c *context.Context, f form.EditPreviewDiff) { - treePath := c.Repo.TreePath + // 🚨 SECURITY: Prevent path traversal. + treePath := pathutil.Clean(c.Repo.TreePath) entry, err := c.Repo.Commit.TreeEntry(treePath) if err != nil {