mirror of
https://github.com/go-gitea/gitea.git
synced 2026-09-08 22:13:26 +09:00
fix: Various security fixes (#38103)
- Enforce org visibility on organization label read endpoints (private org labels no longer leak to non-members). - Block fork sync (`merge-upstream`) when the base repo is no longer readable (stops pulling commits after a parent goes private). - Remove `REVERSE_PROXY_LIMIT` / `REVERSE_PROXY_TRUSTED_PROXIES` from the Docker `app.ini` templates (the `= *` default allowed `X-WEBAUTH-USER` impersonation; reverse-proxy auth is now opt-in and admin-configured). - Enforce single-use TOTP passcodes across web login, password-reset, and Basic-Auth `X-Gitea-OTP` (fixes a TOCTOU race and a stateless replay). - Re-check branch write permission for every ref in a push (the pre-receive hook cached the first ref's result, letting a per-branch maintainer-edit grant escalate to full repo write). --------- Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
+16
-1
@@ -505,6 +505,21 @@ func reqOrgOwnership() func(ctx *context.APIContext) {
|
||||
}
|
||||
}
|
||||
|
||||
// reqOrgVisible requires the organization to be visible to the doer, or a site admin
|
||||
func reqOrgVisible() func(ctx *context.APIContext) {
|
||||
return func(ctx *context.APIContext) {
|
||||
if ctx.Org.Organization == nil {
|
||||
setting.PanicInDevOrTesting("reqOrgVisible: unprepared context")
|
||||
ctx.APIErrorInternal(errors.New("reqOrgVisible: unprepared context"))
|
||||
return
|
||||
}
|
||||
if !organization.HasOrgOrUserVisible(ctx, ctx.Org.Organization.AsUser(), ctx.Doer) {
|
||||
ctx.APIErrorNotFound()
|
||||
return
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func teamAccessPrivileged(ctx *context.APIContext) (orgID int64, privileged, ok bool) {
|
||||
if ctx.IsUserSiteAdmin() {
|
||||
return 0, true, true
|
||||
@@ -1728,7 +1743,7 @@ func Routes() *web.Router {
|
||||
m.Combo("/{id}").Get(reqToken(), org.GetLabel).
|
||||
Patch(reqToken(), reqOrgOwnership(), bind(api.EditLabelOption{}), org.EditLabel).
|
||||
Delete(reqToken(), reqOrgOwnership(), org.DeleteLabel)
|
||||
})
|
||||
}, reqOrgVisible())
|
||||
m.Group("/hooks", func() {
|
||||
m.Combo("").Get(org.ListHooks).
|
||||
Post(bind(api.CreateHookOption{}), org.CreateHook)
|
||||
|
||||
@@ -1336,6 +1336,9 @@ func MergeUpstream(ctx *context.APIContext) {
|
||||
} else if errors.Is(err, util.ErrNotExist) {
|
||||
ctx.APIError(http.StatusNotFound, err.Error())
|
||||
return
|
||||
} else if errors.Is(err, util.ErrPermissionDenied) {
|
||||
ctx.APIError(http.StatusForbidden, err.Error())
|
||||
return
|
||||
}
|
||||
ctx.APIErrorInternal(err)
|
||||
return
|
||||
|
||||
@@ -40,9 +40,6 @@ type preReceiveContext struct {
|
||||
canCreatePullRequest bool
|
||||
checkedCanCreatePullRequest bool
|
||||
|
||||
canWriteCode bool
|
||||
checkedCanWriteCode bool
|
||||
|
||||
protectedTags []*git_model.ProtectedTag
|
||||
gotProtectedTags bool
|
||||
|
||||
@@ -50,24 +47,36 @@ type preReceiveContext struct {
|
||||
|
||||
opts *private.HookOptions
|
||||
|
||||
branchName string
|
||||
// this context should only contain shared variables, mutable variables like "current branch name" shouldn't be put here
|
||||
canWriteCodeUnitCached *bool
|
||||
}
|
||||
|
||||
// CanWriteCode returns true if pusher can write code
|
||||
func (ctx *preReceiveContext) CanWriteCode() bool {
|
||||
if !ctx.checkedCanWriteCode {
|
||||
if !ctx.loadPusherAndPermission() {
|
||||
return false
|
||||
func (ctx *preReceiveContext) canWriteCodeUnit() bool {
|
||||
if ctx.canWriteCodeUnitCached == nil {
|
||||
var canWrite bool
|
||||
if ctx.loadPusherAndPermission() {
|
||||
canWrite = ctx.userPerm.CanWrite(unit.TypeCode) || ctx.deployKeyAccessMode >= perm_model.AccessModeWrite
|
||||
}
|
||||
ctx.canWriteCode = issues_model.CanMaintainerWriteToBranch(ctx, ctx.userPerm, ctx.branchName, ctx.user) || ctx.deployKeyAccessMode >= perm_model.AccessModeWrite
|
||||
ctx.checkedCanWriteCode = true
|
||||
ctx.canWriteCodeUnitCached = &canWrite
|
||||
}
|
||||
return ctx.canWriteCode
|
||||
return *ctx.canWriteCodeUnitCached
|
||||
}
|
||||
|
||||
// AssertCanWriteCode returns true if pusher can write code
|
||||
func (ctx *preReceiveContext) AssertCanWriteCode() bool {
|
||||
if !ctx.CanWriteCode() {
|
||||
// canWriteCodeRef returns true if pusher can write to the code ref (branch/tag/commit)
|
||||
func (ctx *preReceiveContext) canWriteCodeRef(refFullName git.RefName) bool {
|
||||
if ctx.canWriteCodeUnit() {
|
||||
return true
|
||||
}
|
||||
// then check whether if the pusher is a maintainer who can write the PR author's head repo branch
|
||||
if !refFullName.IsBranch() {
|
||||
return false
|
||||
}
|
||||
return issues_model.CanMaintainerWriteToBranch(ctx, ctx.userPerm, refFullName.BranchName(), ctx.user)
|
||||
}
|
||||
|
||||
// assertCanWriteRef returns true if pusher can write to the code ref, otherwise it responds with 403 Forbidden and returns false
|
||||
func (ctx *preReceiveContext) assertCanWriteRef(refFullName git.RefName) bool {
|
||||
if !ctx.canWriteCodeRef(refFullName) {
|
||||
if ctx.Written() {
|
||||
return false
|
||||
}
|
||||
@@ -129,7 +138,7 @@ func HookPreReceive(ctx *gitea_context.PrivateContext) {
|
||||
case git.DefaultFeatures().SupportProcReceive && refFullName.IsFor():
|
||||
preReceiveFor(ourCtx, refFullName)
|
||||
default:
|
||||
ourCtx.AssertCanWriteCode()
|
||||
ourCtx.assertCanWriteRef(refFullName)
|
||||
}
|
||||
if ctx.Written() {
|
||||
return
|
||||
@@ -141,9 +150,8 @@ func HookPreReceive(ctx *gitea_context.PrivateContext) {
|
||||
|
||||
func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, refFullName git.RefName) {
|
||||
branchName := refFullName.BranchName()
|
||||
ctx.branchName = branchName
|
||||
|
||||
if !ctx.AssertCanWriteCode() {
|
||||
if !ctx.assertCanWriteRef(refFullName) {
|
||||
return
|
||||
}
|
||||
|
||||
@@ -404,7 +412,7 @@ func preReceiveBranch(ctx *preReceiveContext, oldCommitID, newCommitID string, r
|
||||
}
|
||||
|
||||
func preReceiveTag(ctx *preReceiveContext, refFullName git.RefName) {
|
||||
if !ctx.AssertCanWriteCode() {
|
||||
if !ctx.assertCanWriteRef(refFullName) {
|
||||
return
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,70 @@
|
||||
// Copyright 2026 The Gitea Authors. All rights reserved.
|
||||
// SPDX-License-Identifier: MIT
|
||||
|
||||
package private
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
issues_model "gitea.dev/models/issues"
|
||||
"gitea.dev/models/perm/access"
|
||||
repo_model "gitea.dev/models/repo"
|
||||
"gitea.dev/models/unittest"
|
||||
"gitea.dev/modules/git"
|
||||
"gitea.dev/services/contexttest"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
// TestPreReceiveCanWriteCodePerBranch ensures the maintainer-edit write grant is evaluated against
|
||||
// the exact ref being pushed on every call, derived from that ref rather than shared mutable state.
|
||||
// Otherwise a per-branch grant (an open PR with "allow edits from maintainers") could be batched
|
||||
// together with a protected branch or a tag to escalate into full repository write.
|
||||
func TestPreReceiveCanWriteCodePerBranch(t *testing.T) {
|
||||
require.NoError(t, unittest.PrepareTestDatabase())
|
||||
|
||||
baseRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 10})
|
||||
headRepo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 11})
|
||||
require.NoError(t, baseRepo.LoadOwner(t.Context()))
|
||||
require.NoError(t, headRepo.LoadOwner(t.Context()))
|
||||
|
||||
// An open PR from the head repo owner, with maintainer edits allowed: this grants the base
|
||||
// repo owner write access to exactly this head branch and nothing else.
|
||||
pr := &issues_model.PullRequest{
|
||||
Issue: &issues_model.Issue{
|
||||
RepoID: baseRepo.ID,
|
||||
PosterID: headRepo.OwnerID,
|
||||
},
|
||||
HeadRepoID: headRepo.ID,
|
||||
BaseRepoID: baseRepo.ID,
|
||||
HeadBranch: "granted-branch",
|
||||
BaseBranch: "master",
|
||||
AllowMaintainerEdit: true,
|
||||
}
|
||||
require.NoError(t, issues_model.NewPullRequest(t.Context(), baseRepo, pr.Issue, nil, nil, pr))
|
||||
|
||||
// The pusher is the base repo owner (the maintainer) with only read access on the head repo.
|
||||
maintainer := baseRepo.Owner
|
||||
headPerm, err := access.GetIndividualUserRepoPermission(t.Context(), headRepo, maintainer)
|
||||
require.NoError(t, err)
|
||||
|
||||
mockCtx, _ := contexttest.MockPrivateContext(t, "/")
|
||||
ctx := &preReceiveContext{
|
||||
PrivateContext: mockCtx,
|
||||
loadedPusher: true,
|
||||
user: maintainer,
|
||||
userPerm: headPerm,
|
||||
}
|
||||
|
||||
// The granted branch must be writable...
|
||||
assert.True(t, ctx.canWriteCodeRef(git.RefNameFromBranch("granted-branch")))
|
||||
|
||||
// ...but another branch in the same push must NOT inherit that grant.
|
||||
assert.False(t, ctx.canWriteCodeRef(git.RefNameFromBranch("master")))
|
||||
|
||||
// ...and a tag sharing the granted branch's name must NOT inherit it either: the grant is
|
||||
// scoped to PR head branches, so a non-branch ref can never match it. (A tag ref already
|
||||
// yields an empty branch name, so this guards the per-ref evaluation, not the IsBranch check.)
|
||||
assert.False(t, ctx.canWriteCodeRef(git.RefNameFromTag("granted-branch")))
|
||||
}
|
||||
@@ -58,14 +58,14 @@ func TwoFactorPost(ctx *context.Context) {
|
||||
return
|
||||
}
|
||||
|
||||
// Validate the passcode with the stored TOTP secret.
|
||||
ok, err := twofa.ValidateTOTP(form.Passcode)
|
||||
// Validate the passcode and atomically consume it to prevent reuse/replay.
|
||||
ok, err := twofa.ValidateAndConsumeTOTP(ctx, form.Passcode)
|
||||
if err != nil {
|
||||
ctx.ServerError("UserSignIn", err)
|
||||
return
|
||||
}
|
||||
|
||||
if ok && twofa.LastUsedPasscode != form.Passcode {
|
||||
if ok {
|
||||
remember := ctx.Session.Get("twofaRemember").(bool)
|
||||
u, err := user_model.GetUserByID(ctx, id)
|
||||
if err != nil {
|
||||
@@ -81,12 +81,6 @@ func TwoFactorPost(ctx *context.Context) {
|
||||
}
|
||||
}
|
||||
|
||||
twofa.LastUsedPasscode = form.Passcode
|
||||
if err = auth.UpdateTwoFactor(ctx, twofa); err != nil {
|
||||
ctx.ServerError("UserSignIn", err)
|
||||
return
|
||||
}
|
||||
|
||||
_ = ctx.Session.Set(session.KeyUserHasTwoFactorAuth, true)
|
||||
handleSignIn(ctx, u, remember)
|
||||
return
|
||||
|
||||
@@ -177,23 +177,17 @@ func ResetPasswdPost(ctx *context.Context) {
|
||||
regenerateScratchToken = true
|
||||
} else {
|
||||
passcode := ctx.FormString("passcode")
|
||||
ok, err := twofa.ValidateTOTP(passcode)
|
||||
ok, err := twofa.ValidateAndConsumeTOTP(ctx, passcode)
|
||||
if err != nil {
|
||||
ctx.HTTPError(http.StatusInternalServerError, "ValidateTOTP", err.Error())
|
||||
ctx.HTTPError(http.StatusInternalServerError, "ValidateAndConsumeTOTP", err.Error())
|
||||
return
|
||||
}
|
||||
if !ok || twofa.LastUsedPasscode == passcode {
|
||||
if !ok {
|
||||
ctx.Data["IsResetForm"] = true
|
||||
ctx.Data["Err_Passcode"] = true
|
||||
ctx.RenderWithErrDeprecated(ctx.Tr("auth.twofa_passcode_incorrect"), tplResetPassword, nil)
|
||||
return
|
||||
}
|
||||
|
||||
twofa.LastUsedPasscode = passcode
|
||||
if err = auth.UpdateTwoFactor(ctx, twofa); err != nil {
|
||||
ctx.ServerError("ResetPasswdPost: UpdateTwoFactor", err)
|
||||
return
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -264,7 +264,7 @@ func MergeUpstream(ctx *context.Context) {
|
||||
branchName := ctx.FormString("branch")
|
||||
_, err := repo_service.MergeUpstream(ctx, ctx.Doer, ctx.Repo.Repository, branchName, false)
|
||||
if err != nil {
|
||||
if errors.Is(err, util.ErrNotExist) {
|
||||
if errors.Is(err, util.ErrNotExist) || errors.Is(err, util.ErrPermissionDenied) {
|
||||
ctx.JSONErrorNotFound()
|
||||
return
|
||||
} else if pull_service.IsErrMergeConflicts(err) {
|
||||
|
||||
Reference in New Issue
Block a user