mirror of
https://github.com/go-gitea/gitea.git
synced 2026-08-03 11:24:39 +00:00
fix: various security fixes (#38406)
Addresses a batch of privately reported security issues, grouped by area: - **SSRF** - migration PR-patch/asset fetches, OAuth2 avatar & OpenID discovery, pull-mirror URL re-validation, and the outbound proxy path. - **Access-token scope** - prevent scope escalation on token creation; keep public-only tokens confined (feeds, packages, Actions listings, star/watch lists, limited/private owners). - **Access control / disclosure** - go-get default-branch leak, webhook authorization-header leak, watch clearing on private transitions, label/attachment scoping. - **Denial of service** - input bounds for npm dist-tags, Debian control files, Arch file lists, and SSH keys. ### 📌 Attention for site admins Not breaking - existing configs keep working - but two changes are worth a look: - **New SSRF protection** Outbound requests (migrations, OAuth2 avatars, OpenID discovery, pull mirrors, proxy path) are now validated against the allow/block host lists. If your instance legitimately reaches internal hosts, you may need to add them to `[security].ALLOWED_HOST_LIST` (and the relevant `ALLOW_LOCALNETWORKS` settings). - **Deprecation** `[webhook].ALLOWED_HOST_LIST` is deprecated and will be removed in a future release. Use `[security].ALLOWED_HOST_LIST` instead; the old key still works for now. --------- Co-authored-by: TheFox0x7 <thefox0x7@gmail.com> Co-authored-by: techknowlogick <techknowlogick@gitea.io> Co-authored-by: Lunny Xiao <xiaolunwen@gmail.com> Co-authored-by: wxiaoguang <wxiaoguang@gmail.com> Co-authored-by: Zettat123 <zettat123@gmail.com>
This commit is contained in:
@@ -627,11 +627,18 @@ func UpdateCommentAttachments(ctx context.Context, c *Comment, uuids []string) e
|
||||
return nil
|
||||
}
|
||||
return db.WithTx(ctx, func(ctx context.Context) error {
|
||||
issue, err := GetIssueByID(ctx, c.IssueID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, uuids)
|
||||
if err != nil {
|
||||
return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", uuids, err)
|
||||
}
|
||||
for i := range attachments {
|
||||
if err := validateAttachmentForIssue(ctx, issue, attachments[i]); err != nil {
|
||||
return err
|
||||
}
|
||||
attachments[i].IssueID = c.IssueID
|
||||
attachments[i].CommentID = c.ID
|
||||
if err := repo_model.UpdateAttachment(ctx, attachments[i]); err != nil {
|
||||
|
||||
@@ -62,8 +62,10 @@ func Test_UpdateCommentAttachment(t *testing.T) {
|
||||
assert.NoError(t, unittest.PrepareTestDatabase())
|
||||
|
||||
comment := unittest.AssertExistsAndLoadBean(t, &issues_model.Comment{ID: 1})
|
||||
issue := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: comment.IssueID})
|
||||
attachment := repo_model.Attachment{
|
||||
Name: "test.txt",
|
||||
RepoID: issue.RepoID, // must match the comment's repo, else the cross-repo guard rejects it
|
||||
Name: "test.txt",
|
||||
}
|
||||
assert.NoError(t, db.Insert(t.Context(), &attachment))
|
||||
|
||||
|
||||
@@ -263,14 +263,46 @@ func AddDeletePRBranchComment(ctx context.Context, doer *user_model.User, repo *
|
||||
return err
|
||||
}
|
||||
|
||||
// validateAttachmentForIssue rejects a foreign or already-linked attachment before it is linked to
|
||||
// issue: a known UUID could otherwise re-link (and expose) another repo's private attachment. A
|
||||
// legacy attachment predating repo_id-on-upload is adopted into the issue's repo.
|
||||
func validateAttachmentForIssue(ctx context.Context, issue *Issue, attachment *repo_model.Attachment) error {
|
||||
if attachment.RepoID == 0 && attachment.CreatedUnix < repo_model.LegacyAttachmentMissingRepoIDCutoff {
|
||||
attachment.RepoID = issue.RepoID
|
||||
if err := repo_model.UpdateAttachmentByUUID(ctx, attachment, "repo_id"); err != nil {
|
||||
return fmt.Errorf("update attachment repo_id [id: %d]: %w", attachment.ID, err)
|
||||
}
|
||||
}
|
||||
if attachment.RepoID != issue.RepoID {
|
||||
return util.NewPermissionDeniedErrorf("attachment belongs to a different repository")
|
||||
}
|
||||
if attachment.IssueID != 0 && attachment.IssueID != issue.ID {
|
||||
return util.NewPermissionDeniedErrorf("attachment is already linked to another issue")
|
||||
}
|
||||
if attachment.ReleaseID != 0 {
|
||||
return util.NewPermissionDeniedErrorf("attachment is already linked to a release")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// UpdateIssueAttachments update attachments by UUIDs for the issue
|
||||
func UpdateIssueAttachments(ctx context.Context, issueID int64, uuids []string) (err error) {
|
||||
if len(uuids) == 0 {
|
||||
return nil
|
||||
}
|
||||
return db.WithTx(ctx, func(ctx context.Context) error {
|
||||
issue, err := GetIssueByID(ctx, issueID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
attachments, err := repo_model.GetAttachmentsByUUIDs(ctx, uuids)
|
||||
if err != nil {
|
||||
return fmt.Errorf("getAttachmentsByUUIDs [uuids: %v]: %w", uuids, err)
|
||||
}
|
||||
for i := range attachments {
|
||||
if err := validateAttachmentForIssue(ctx, issue, attachments[i]); err != nil {
|
||||
return err
|
||||
}
|
||||
attachments[i].IssueID = issueID
|
||||
if err := repo_model.UpdateAttachment(ctx, attachments[i]); err != nil {
|
||||
return fmt.Errorf("update attachment [id: %d]: %w", attachments[i].ID, err)
|
||||
|
||||
@@ -0,0 +1,33 @@
|
||||
// Copyright 2026 The Gitea Authors. All rights reserved.
|
||||
// SPDX-License-Identifier: MIT
|
||||
|
||||
package issues_test
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
issues_model "gitea.dev/models/issues"
|
||||
repo_model "gitea.dev/models/repo"
|
||||
"gitea.dev/models/unittest"
|
||||
"gitea.dev/modules/util"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestUpdateIssueAttachmentsCrossRepo(t *testing.T) {
|
||||
require.NoError(t, unittest.PrepareTestDatabase())
|
||||
|
||||
// attachment id 2 belongs to repo 2 / issue 4; issue 1 lives in repo 1
|
||||
issue1 := unittest.AssertExistsAndLoadBean(t, &issues_model.Issue{ID: 1})
|
||||
foreign := unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: 2})
|
||||
require.NotEqual(t, issue1.RepoID, foreign.RepoID)
|
||||
|
||||
// re-linking a foreign repo's attachment by UUID must be rejected
|
||||
err := issues_model.UpdateIssueAttachments(t.Context(), issue1.ID, []string{foreign.UUID})
|
||||
assert.ErrorIs(t, err, util.ErrPermissionDenied)
|
||||
|
||||
// the foreign attachment must be left untouched
|
||||
reloaded := unittest.AssertExistsAndLoadBean(t, &repo_model.Attachment{ID: 2})
|
||||
assert.Equal(t, foreign.IssueID, reloaded.IssueID)
|
||||
}
|
||||
+13
-6
@@ -6,6 +6,7 @@ package issues
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"slices"
|
||||
"strconv"
|
||||
@@ -27,12 +28,6 @@ type ErrRepoLabelNotExist struct {
|
||||
RepoID int64
|
||||
}
|
||||
|
||||
// IsErrRepoLabelNotExist checks if an error is a RepoErrLabelNotExist.
|
||||
func IsErrRepoLabelNotExist(err error) bool {
|
||||
_, ok := err.(ErrRepoLabelNotExist)
|
||||
return ok
|
||||
}
|
||||
|
||||
func (err ErrRepoLabelNotExist) Error() string {
|
||||
return fmt.Sprintf("label does not exist [label_id: %d, repo_id: %d]", err.LabelID, err.RepoID)
|
||||
}
|
||||
@@ -312,6 +307,18 @@ func GetLabelInRepoByName(ctx context.Context, repoID int64, labelName string) (
|
||||
return l, nil
|
||||
}
|
||||
|
||||
// GetLabelInRepoOrOrgByID returns the label with labelID scoped to the repo, falling back to the
|
||||
// repo's owning organization when ownerIsOrg is set. It returns ErrRepoLabelNotExist /
|
||||
// ErrOrgLabelNotExist when the label is in neither scope, so a foreign-but-existing label ID is
|
||||
// indistinguishable from a nonexistent one (no cross-repo enumeration oracle).
|
||||
func GetLabelInRepoOrOrgByID(ctx context.Context, repoID, ownerID int64, ownerIsOrg bool, labelID int64) (*Label, error) {
|
||||
label, err := GetLabelInRepoByID(ctx, repoID, labelID)
|
||||
if err != nil && errors.Is(err, util.ErrNotExist) && ownerIsOrg {
|
||||
return GetLabelInOrgByID(ctx, ownerID, labelID)
|
||||
}
|
||||
return label, err
|
||||
}
|
||||
|
||||
// GetLabelInRepoByID returns a label by ID in given repository.
|
||||
func GetLabelInRepoByID(ctx context.Context, repoID, labelID int64) (*Label, error) {
|
||||
if labelID <= 0 || repoID <= 0 {
|
||||
|
||||
@@ -12,6 +12,7 @@ import (
|
||||
"gitea.dev/models/unittest"
|
||||
user_model "gitea.dev/models/user"
|
||||
"gitea.dev/modules/timeutil"
|
||||
"gitea.dev/modules/util"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
@@ -94,10 +95,10 @@ func TestGetLabelInRepoByName(t *testing.T) {
|
||||
assert.Equal(t, "label1", label.Name)
|
||||
|
||||
_, err = issues_model.GetLabelInRepoByName(t.Context(), 1, "")
|
||||
assert.True(t, issues_model.IsErrRepoLabelNotExist(err))
|
||||
assert.ErrorIs(t, err, util.ErrNotExist)
|
||||
|
||||
_, err = issues_model.GetLabelInRepoByName(t.Context(), unittest.NonexistentID, "nonexistent")
|
||||
assert.True(t, issues_model.IsErrRepoLabelNotExist(err))
|
||||
assert.ErrorIs(t, err, util.ErrNotExist)
|
||||
}
|
||||
|
||||
func TestGetLabelInRepoByNames(t *testing.T) {
|
||||
@@ -131,10 +132,10 @@ func TestGetLabelInRepoByID(t *testing.T) {
|
||||
assert.EqualValues(t, 1, label.ID)
|
||||
|
||||
_, err = issues_model.GetLabelInRepoByID(t.Context(), 1, -1)
|
||||
assert.True(t, issues_model.IsErrRepoLabelNotExist(err))
|
||||
assert.ErrorIs(t, err, util.ErrNotExist)
|
||||
|
||||
_, err = issues_model.GetLabelInRepoByID(t.Context(), unittest.NonexistentID, unittest.NonexistentID)
|
||||
assert.True(t, issues_model.IsErrRepoLabelNotExist(err))
|
||||
assert.ErrorIs(t, err, util.ErrNotExist)
|
||||
}
|
||||
|
||||
func TestGetLabelsInRepoByIDs(t *testing.T) {
|
||||
|
||||
Reference in New Issue
Block a user