mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2026-05-12 22:10:25 +00:00
2026-05-12 security patches (#12493)
- fix: prevent git write to wiki repo from unauthorized user via git HTTP - fix: prevent LFS authorization token from being used for read/write access after user's access is restricted from Forgejo - fix: prevent scoped API access (OAuth tokens, Access tokens) from accessing resources beyond their permitted scope via non-API endpoints (e.g. /user/repo/raw/...) - fix: implementing missing OAuth validation checks, improve protections against race conditions - fix: prevent OAuth redirect URI spoofing via non-ascii case collision - fix: strengthen Actions Artifact V4 signature algorithm against spoofing attacks <!--start release-notes-assistant--> ## Release notes <!--URL:https://codeberg.org/forgejo/forgejo--> - Security bug fixes - [PR](https://codeberg.org/forgejo/forgejo/pulls/12493): <!--number 12493 --><!--line 0 --><!--description MjAyNi0wNS0xMiBzZWN1cml0eSBwYXRjaGVz-->2026-05-12 security patches<!--description--> <!--end release-notes-assistant--> Co-authored-by: Derzsi Dániel <daniel@tohka.us> Co-authored-by: jvoisin <julien.voisin@dustri.org> Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/12493
This commit is contained in:
parent
5b6c702f41
commit
32b8d732b8
18 changed files with 900 additions and 41 deletions
|
|
@ -4,6 +4,7 @@
|
|||
package context
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"net/http"
|
||||
"slices"
|
||||
|
||||
|
|
@ -57,6 +58,23 @@ func RequireRepoWriterOr(unitTypes ...unit.Type) func(ctx *Context) {
|
|||
// RequireRepoReader returns a middleware for requiring repository read to the specify unitType
|
||||
func RequireRepoReader(unitType unit.Type) func(ctx *Context) {
|
||||
return func(ctx *Context) {
|
||||
// Typically checks for authentication scopes won't be relevant for non-API requests where this middleware is
|
||||
// used; but, some paths like `/user/repo/raw/...` can be accessed with API authentication mechanisms. In those
|
||||
// edge cases, check that `read:repository` scope is present if the authentication method indicates a limited
|
||||
// scope.
|
||||
hasScope, scope := ctx.Authentication.Scope().Get()
|
||||
if hasScope {
|
||||
allow, err := scope.HasScope(auth_model.AccessTokenScopeReadRepository)
|
||||
if err != nil {
|
||||
ctx.ServerError("checking scope failed", err)
|
||||
return
|
||||
}
|
||||
if !allow {
|
||||
ctx.Error(http.StatusForbidden, "scopedAccessCheck", fmt.Sprintf("token does not have at least one of required scope(s): %v", auth_model.AccessTokenScopeReadRepository))
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
if !ctx.Repo.CanRead(unitType) {
|
||||
if log.IsTrace() {
|
||||
if ctx.IsSigned {
|
||||
|
|
|
|||
|
|
@ -375,7 +375,15 @@ func repoAssignment(ctx *Context, repo *repo_model.Repository) {
|
|||
return
|
||||
}
|
||||
|
||||
ctx.Repo.Permission, err = access_model.GetUserRepoPermission(ctx, repo, ctx.Doer)
|
||||
// Typically checks for authorization reducers won't be relevant for non-API requests where this middleware is used;
|
||||
// but, some paths like `/user/repo/raw/...` can be accessed with API authentication mechanisms. In those edge
|
||||
// cases, initialize `ctx.Repo.Permission` based upon the reduced permission set available.
|
||||
authorizationReducer := ctx.Authentication.Reducer()
|
||||
if authorizationReducer == nil {
|
||||
ctx.Repo.Permission, err = access_model.GetUserRepoPermission(ctx, repo, ctx.Doer)
|
||||
} else {
|
||||
ctx.Repo.Permission, err = access_model.GetUserRepoPermissionWithReducer(ctx, repo, ctx.Doer, authorizationReducer)
|
||||
}
|
||||
if err != nil {
|
||||
ctx.ServerError("GetUserRepoPermission", err)
|
||||
return
|
||||
|
|
|
|||
|
|
@ -607,6 +607,20 @@ func handleLFSToken(ctx stdCtx.Context, tokenSHA string, target *repo_model.Repo
|
|||
log.Error("Unable to GetUserById[%d]: Error: %v", claims.UserID, err)
|
||||
return nil, err
|
||||
}
|
||||
|
||||
if !u.IsAccessAllowed(ctx) {
|
||||
return nil, errors.New("user access is blocked")
|
||||
}
|
||||
|
||||
repoPerm, err := access_model.GetUserRepoPermission(ctx, target, u)
|
||||
if err != nil {
|
||||
log.Error("Unable to GetUserRepoPermission[%d]: Error: %v", claims.UserID, err)
|
||||
return nil, err
|
||||
}
|
||||
if !repoPerm.CanAccess(mode, unit.TypeCode) {
|
||||
return nil, errors.New("user does not have access to the repository")
|
||||
}
|
||||
|
||||
return u, nil
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -78,6 +78,75 @@ func testAuthenticate(t *testing.T, cfg string) {
|
|||
assert.EqualValues(t, 2, u.ID)
|
||||
})
|
||||
|
||||
t.Run("handleLFSToken nonexistent user", func(t *testing.T) {
|
||||
tokenMissing, _ := getLFSAuthTokenWithBearer(authTokenOptions{Op: "download", UserID: 999, RepoID: 1})
|
||||
_, tokenMissing, _ = strings.Cut(tokenMissing, " ")
|
||||
|
||||
u, err := handleLFSToken(ctx, tokenMissing, repo1, perm_model.AccessModeRead)
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "user does not exist")
|
||||
assert.Nil(t, u)
|
||||
})
|
||||
|
||||
t.Run("handleLFSToken nonexistent repo", func(t *testing.T) {
|
||||
tokenBadRepo, _ := getLFSAuthTokenWithBearer(authTokenOptions{Op: "download", UserID: 2, RepoID: 999})
|
||||
_, tokenBadRepo, _ = strings.Cut(tokenBadRepo, " ")
|
||||
badRepo := &repo_model.Repository{ID: 999}
|
||||
|
||||
u, err := handleLFSToken(ctx, tokenBadRepo, badRepo, perm_model.AccessModeRead)
|
||||
require.Error(t, err)
|
||||
assert.Nil(t, u)
|
||||
})
|
||||
|
||||
t.Run("handleLFSToken blocked user", func(t *testing.T) {
|
||||
tokenBlocked, _ := getLFSAuthTokenWithBearer(authTokenOptions{Op: "download", UserID: 37, RepoID: 1})
|
||||
_, tokenBlocked, _ = strings.Cut(tokenBlocked, " ")
|
||||
|
||||
u, err := handleLFSToken(ctx, tokenBlocked, repo1, perm_model.AccessModeRead)
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "user access is blocked")
|
||||
assert.Nil(t, u)
|
||||
})
|
||||
|
||||
t.Run("handleLFSToken no repo access", func(t *testing.T) {
|
||||
repo2 := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 2})
|
||||
tokenNoAccess, _ := getLFSAuthTokenWithBearer(authTokenOptions{Op: "download", UserID: 10, RepoID: 2})
|
||||
_, tokenNoAccess, _ = strings.Cut(tokenNoAccess, " ")
|
||||
|
||||
u, err := handleLFSToken(ctx, tokenNoAccess, repo2, perm_model.AccessModeRead)
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "does not have access to the repository")
|
||||
assert.Nil(t, u)
|
||||
})
|
||||
|
||||
t.Run("handleLFSToken upload write access allowed", func(t *testing.T) {
|
||||
tokenUploadRW, _ := getLFSAuthTokenWithBearer(authTokenOptions{Op: "upload", UserID: 2, RepoID: 1})
|
||||
_, tokenUploadRW, _ = strings.Cut(tokenUploadRW, " ")
|
||||
|
||||
u, err := handleLFSToken(ctx, tokenUploadRW, repo1, perm_model.AccessModeWrite)
|
||||
require.NoError(t, err)
|
||||
assert.EqualValues(t, 2, u.ID)
|
||||
})
|
||||
|
||||
t.Run("handleLFSToken upload read-only access denied", func(t *testing.T) {
|
||||
tokenUploadRO, _ := getLFSAuthTokenWithBearer(authTokenOptions{Op: "upload", UserID: 10, RepoID: 1})
|
||||
_, tokenUploadRO, _ = strings.Cut(tokenUploadRO, " ")
|
||||
|
||||
u, err := handleLFSToken(ctx, tokenUploadRO, repo1, perm_model.AccessModeWrite)
|
||||
require.Error(t, err)
|
||||
assert.Contains(t, err.Error(), "does not have access to the repository")
|
||||
assert.Nil(t, u)
|
||||
})
|
||||
|
||||
t.Run("handleLFSToken download read-only access allowed", func(t *testing.T) {
|
||||
tokenDownloadRO, _ := getLFSAuthTokenWithBearer(authTokenOptions{Op: "download", UserID: 10, RepoID: 1})
|
||||
_, tokenDownloadRO, _ = strings.Cut(tokenDownloadRO, " ")
|
||||
|
||||
u, err := handleLFSToken(ctx, tokenDownloadRO, repo1, perm_model.AccessModeRead)
|
||||
require.NoError(t, err)
|
||||
assert.EqualValues(t, 10, u.ID)
|
||||
})
|
||||
|
||||
t.Run("authenticate", func(t *testing.T) {
|
||||
const prefixBearer = "Bearer "
|
||||
assert.False(t, authenticate(ctx, repo1, "", true, false))
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue