Skip to content

Commit 6569f1f

Browse files
author
Earl Warren
committed
Merge pull request '[v9.0/forgejo] fix: 15 November 2024 security fixes batch' (go-gitea#5975) from earl-warren/forgejo:wip-v9.0-security-15-11 into v9.0/forgejo
Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/5975 Reviewed-by: Otto <[email protected]>
2 parents fd4a68b + 2f72bec commit 6569f1f

File tree

38 files changed

+931
-289
lines changed

38 files changed

+931
-289
lines changed

models/auth/auth_token.go

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,31 @@ import (
1515
"code.gitea.io/gitea/modules/util"
1616
)
1717

18+
type AuthorizationPurpose string
19+
20+
var (
21+
// Used to store long term authorization tokens.
22+
LongTermAuthorization AuthorizationPurpose = "long_term_authorization"
23+
24+
// Used to activate a user account.
25+
UserActivation AuthorizationPurpose = "user_activation"
26+
27+
// Used to reset the password.
28+
PasswordReset AuthorizationPurpose = "password_reset"
29+
)
30+
31+
// Used to activate the specified email address for a user.
32+
func EmailActivation(email string) AuthorizationPurpose {
33+
return AuthorizationPurpose("email_activation:" + email)
34+
}
35+
1836
// AuthorizationToken represents a authorization token to a user.
1937
type AuthorizationToken struct {
2038
ID int64 `xorm:"pk autoincr"`
2139
UID int64 `xorm:"INDEX"`
2240
LookupKey string `xorm:"INDEX UNIQUE"`
2341
HashedValidator string
42+
Purpose AuthorizationPurpose `xorm:"NOT NULL DEFAULT 'long_term_authorization'"`
2443
Expiry timeutil.TimeStamp
2544
}
2645

@@ -41,7 +60,7 @@ func (authToken *AuthorizationToken) IsExpired() bool {
4160
// GenerateAuthToken generates a new authentication token for the given user.
4261
// It returns the lookup key and validator values that should be passed to the
4362
// user via a long-term cookie.
44-
func GenerateAuthToken(ctx context.Context, userID int64, expiry timeutil.TimeStamp) (lookupKey, validator string, err error) {
63+
func GenerateAuthToken(ctx context.Context, userID int64, expiry timeutil.TimeStamp, purpose AuthorizationPurpose) (lookupKey, validator string, err error) {
4564
// Request 64 random bytes. The first 32 bytes will be used for the lookupKey
4665
// and the other 32 bytes will be used for the validator.
4766
rBytes, err := util.CryptoRandomBytes(64)
@@ -56,14 +75,15 @@ func GenerateAuthToken(ctx context.Context, userID int64, expiry timeutil.TimeSt
5675
Expiry: expiry,
5776
LookupKey: lookupKey,
5877
HashedValidator: HashValidator(rBytes[32:]),
78+
Purpose: purpose,
5979
})
6080
return lookupKey, validator, err
6181
}
6282

6383
// FindAuthToken will find a authorization token via the lookup key.
64-
func FindAuthToken(ctx context.Context, lookupKey string) (*AuthorizationToken, error) {
84+
func FindAuthToken(ctx context.Context, lookupKey string, purpose AuthorizationPurpose) (*AuthorizationToken, error) {
6585
var authToken AuthorizationToken
66-
has, err := db.GetEngine(ctx).Where("lookup_key = ?", lookupKey).Get(&authToken)
86+
has, err := db.GetEngine(ctx).Where("lookup_key = ? AND purpose = ?", lookupKey, purpose).Get(&authToken)
6787
if err != nil {
6888
return nil, err
6989
} else if !has {
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
-
2+
id: 1001
3+
owner_id: 33
4+
owner_name: user33
5+
lower_name: repo1001
6+
name: repo1001
7+
default_branch: main
8+
num_watches: 0
9+
num_stars: 0
10+
num_forks: 0
11+
num_issues: 0
12+
num_closed_issues: 0
13+
num_pulls: 0
14+
num_closed_pulls: 0
15+
num_milestones: 0
16+
num_closed_milestones: 0
17+
num_projects: 0
18+
num_closed_projects: 0
19+
is_private: false
20+
is_empty: false
21+
is_archived: false
22+
is_mirror: false
23+
status: 0
24+
is_fork: false
25+
fork_id: 0
26+
is_template: false
27+
template_id: 0
28+
size: 0
29+
is_fsck_enabled: true
30+
close_issues_via_commit_in_any_branch: false

models/repo/fork.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"context"
88

99
"code.gitea.io/gitea/models/db"
10+
"code.gitea.io/gitea/models/unit"
1011
user_model "code.gitea.io/gitea/models/user"
1112

1213
"xorm.io/builder"
@@ -54,9 +55,9 @@ func GetUserFork(ctx context.Context, repoID, userID int64) (*Repository, error)
5455
return &forkedRepo, nil
5556
}
5657

57-
// GetForks returns all the forks of the repository
58-
func GetForks(ctx context.Context, repo *Repository, listOptions db.ListOptions) ([]*Repository, error) {
59-
sess := db.GetEngine(ctx)
58+
// GetForks returns all the forks of the repository that are visible to the user.
59+
func GetForks(ctx context.Context, repo *Repository, user *user_model.User, listOptions db.ListOptions) ([]*Repository, int64, error) {
60+
sess := db.GetEngine(ctx).Where(AccessibleRepositoryCondition(user, unit.TypeInvalid))
6061

6162
var forks []*Repository
6263
if listOptions.Page == 0 {
@@ -66,7 +67,8 @@ func GetForks(ctx context.Context, repo *Repository, listOptions db.ListOptions)
6667
sess = db.SetSessionPagination(sess, &listOptions)
6768
}
6869

69-
return forks, sess.Find(&forks, &Repository{ForkID: repo.ID})
70+
count, err := sess.FindAndCount(&forks, &Repository{ForkID: repo.ID})
71+
return forks, count, err
7072
}
7173

7274
// IncrementRepoForkNum increment repository fork number

models/repo/repo_list.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -641,12 +641,9 @@ func AccessibleRepositoryCondition(user *user_model.User, unitType unit.Type) bu
641641
// 1. Be able to see all non-private repositories that either:
642642
cond = cond.Or(builder.And(
643643
builder.Eq{"`repository`.is_private": false},
644-
// 2. Aren't in an private organisation or limited organisation if we're not logged in
644+
// 2. Aren't in an private organisation/user or limited organisation/user if the doer is not logged in.
645645
builder.NotIn("`repository`.owner_id", builder.Select("id").From("`user`").Where(
646-
builder.And(
647-
builder.Eq{"type": user_model.UserTypeOrganization},
648-
builder.In("visibility", orgVisibilityLimit)),
649-
))))
646+
builder.In("visibility", orgVisibilityLimit)))))
650647
}
651648

652649
if user != nil {

models/repo/repo_list_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,18 @@
44
package repo_test
55

66
import (
7+
"path/filepath"
8+
"slices"
79
"strings"
810
"testing"
911

1012
"code.gitea.io/gitea/models/db"
1113
repo_model "code.gitea.io/gitea/models/repo"
1214
"code.gitea.io/gitea/models/unittest"
15+
"code.gitea.io/gitea/models/user"
1316
"code.gitea.io/gitea/modules/optional"
17+
"code.gitea.io/gitea/modules/setting"
18+
"code.gitea.io/gitea/modules/structs"
1419

1520
"github.com/stretchr/testify/assert"
1621
"github.com/stretchr/testify/require"
@@ -403,3 +408,43 @@ func TestSearchRepositoryByTopicName(t *testing.T) {
403408
})
404409
}
405410
}
411+
412+
func TestSearchRepositoryIDsByCondition(t *testing.T) {
413+
defer unittest.OverrideFixtures(
414+
unittest.FixturesOptions{
415+
Dir: filepath.Join(setting.AppWorkPath, "models/fixtures/"),
416+
Base: setting.AppWorkPath,
417+
Dirs: []string{"models/repo/TestSearchRepositoryIDsByCondition/"},
418+
},
419+
)()
420+
require.NoError(t, unittest.PrepareTestDatabase())
421+
// Sanity check of the database
422+
limitedUser := unittest.AssertExistsAndLoadBean(t, &user.User{ID: 33, Visibility: structs.VisibleTypeLimited})
423+
unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1001, OwnerID: limitedUser.ID})
424+
425+
testCases := []struct {
426+
user *user.User
427+
repoIDs []int64
428+
}{
429+
{
430+
user: nil,
431+
repoIDs: []int64{1, 4, 8, 9, 10, 11, 12, 14, 17, 18, 21, 23, 25, 27, 29, 32, 33, 34, 35, 36, 37, 42, 44, 45, 46, 47, 48, 49, 50, 51, 53, 57, 58, 60, 61, 62, 1059},
432+
},
433+
{
434+
user: unittest.AssertExistsAndLoadBean(t, &user.User{ID: 4}),
435+
repoIDs: []int64{1, 3, 4, 8, 9, 10, 11, 12, 14, 17, 18, 21, 23, 25, 27, 29, 32, 33, 34, 35, 36, 37, 38, 40, 42, 44, 45, 46, 47, 48, 49, 50, 51, 53, 57, 58, 60, 61, 62, 1001, 1059},
436+
},
437+
{
438+
user: unittest.AssertExistsAndLoadBean(t, &user.User{ID: 5}),
439+
repoIDs: []int64{1, 4, 8, 9, 10, 11, 12, 14, 17, 18, 21, 23, 25, 27, 29, 32, 33, 34, 35, 36, 37, 38, 40, 42, 44, 45, 46, 47, 48, 49, 50, 51, 53, 57, 58, 60, 61, 62, 1001, 1059},
440+
},
441+
}
442+
443+
for _, testCase := range testCases {
444+
repoIDs, err := repo_model.FindUserCodeAccessibleRepoIDs(db.DefaultContext, testCase.user)
445+
require.NoError(t, err)
446+
447+
slices.Sort(repoIDs)
448+
assert.EqualValues(t, testCase.repoIDs, repoIDs)
449+
}
450+
}

models/user/email_address.go

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,8 @@ import (
1010
"net/mail"
1111
"regexp"
1212
"strings"
13-
"time"
1413

1514
"code.gitea.io/gitea/models/db"
16-
"code.gitea.io/gitea/modules/base"
1715
"code.gitea.io/gitea/modules/log"
1816
"code.gitea.io/gitea/modules/optional"
1917
"code.gitea.io/gitea/modules/setting"
@@ -307,23 +305,6 @@ func updateActivation(ctx context.Context, email *EmailAddress, activate bool) e
307305
return UpdateUserCols(ctx, user, "rands")
308306
}
309307

310-
// VerifyActiveEmailCode verifies active email code when active account
311-
func VerifyActiveEmailCode(ctx context.Context, code, email string) *EmailAddress {
312-
if user := GetVerifyUser(ctx, code); user != nil {
313-
// time limit code
314-
prefix := code[:base.TimeLimitCodeLength]
315-
data := fmt.Sprintf("%d%s%s%s%s", user.ID, email, user.LowerName, user.Passwd, user.Rands)
316-
317-
if base.VerifyTimeLimitCode(time.Now(), data, setting.Service.ActiveCodeLives, prefix) {
318-
emailAddress := &EmailAddress{UID: user.ID, Email: email}
319-
if has, _ := db.GetEngine(ctx).Get(emailAddress); has {
320-
return emailAddress
321-
}
322-
}
323-
}
324-
return nil
325-
}
326-
327308
// SearchEmailOrderBy is used to sort the results from SearchEmails()
328309
type SearchEmailOrderBy string
329310

models/user/user.go

Lines changed: 46 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@ package user
77

88
import (
99
"context"
10+
"crypto/subtle"
1011
"encoding/hex"
12+
"errors"
1113
"fmt"
1214
"net/mail"
1315
"net/url"
@@ -318,15 +320,14 @@ func (u *User) OrganisationLink() string {
318320
return setting.AppSubURL + "/org/" + url.PathEscape(u.Name)
319321
}
320322

321-
// GenerateEmailActivateCode generates an activate code based on user information and given e-mail.
322-
func (u *User) GenerateEmailActivateCode(email string) string {
323-
code := base.CreateTimeLimitCode(
324-
fmt.Sprintf("%d%s%s%s%s", u.ID, email, u.LowerName, u.Passwd, u.Rands),
325-
setting.Service.ActiveCodeLives, time.Now(), nil)
326-
327-
// Add tail hex username
328-
code += hex.EncodeToString([]byte(u.LowerName))
329-
return code
323+
// GenerateEmailAuthorizationCode generates an activation code based for the user for the specified purpose.
324+
// The standard expiry is ActiveCodeLives minutes.
325+
func (u *User) GenerateEmailAuthorizationCode(ctx context.Context, purpose auth.AuthorizationPurpose) (string, error) {
326+
lookup, validator, err := auth.GenerateAuthToken(ctx, u.ID, timeutil.TimeStampNow().Add(int64(setting.Service.ActiveCodeLives)*60), purpose)
327+
if err != nil {
328+
return "", err
329+
}
330+
return lookup + ":" + validator, nil
330331
}
331332

332333
// GetUserFollowers returns range of user's followers.
@@ -836,35 +837,50 @@ func countUsers(ctx context.Context, opts *CountUserFilter) int64 {
836837
return count
837838
}
838839

839-
// GetVerifyUser get user by verify code
840-
func GetVerifyUser(ctx context.Context, code string) (user *User) {
841-
if len(code) <= base.TimeLimitCodeLength {
842-
return nil
840+
// VerifyUserActiveCode verifies that the code is valid for the given purpose for this user.
841+
// If delete is specified, the token will be deleted.
842+
func VerifyUserAuthorizationToken(ctx context.Context, code string, purpose auth.AuthorizationPurpose, delete bool) (*User, error) {
843+
lookupKey, validator, found := strings.Cut(code, ":")
844+
if !found {
845+
return nil, nil
843846
}
844847

845-
// use tail hex username query user
846-
hexStr := code[base.TimeLimitCodeLength:]
847-
if b, err := hex.DecodeString(hexStr); err == nil {
848-
if user, err = GetUserByName(ctx, string(b)); user != nil {
849-
return user
848+
authToken, err := auth.FindAuthToken(ctx, lookupKey, purpose)
849+
if err != nil {
850+
if errors.Is(err, util.ErrNotExist) {
851+
return nil, nil
850852
}
851-
log.Error("user.getVerifyUser: %v", err)
853+
return nil, err
852854
}
853855

854-
return nil
855-
}
856+
if authToken.IsExpired() {
857+
return nil, auth.DeleteAuthToken(ctx, authToken)
858+
}
859+
860+
rawValidator, err := hex.DecodeString(validator)
861+
if err != nil {
862+
return nil, err
863+
}
864+
865+
if subtle.ConstantTimeCompare([]byte(authToken.HashedValidator), []byte(auth.HashValidator(rawValidator))) == 0 {
866+
return nil, errors.New("validator doesn't match")
867+
}
856868

857-
// VerifyUserActiveCode verifies active code when active account
858-
func VerifyUserActiveCode(ctx context.Context, code string) (user *User) {
859-
if user = GetVerifyUser(ctx, code); user != nil {
860-
// time limit code
861-
prefix := code[:base.TimeLimitCodeLength]
862-
data := fmt.Sprintf("%d%s%s%s%s", user.ID, user.Email, user.LowerName, user.Passwd, user.Rands)
863-
if base.VerifyTimeLimitCode(time.Now(), data, setting.Service.ActiveCodeLives, prefix) {
864-
return user
869+
u, err := GetUserByID(ctx, authToken.UID)
870+
if err != nil {
871+
if IsErrUserNotExist(err) {
872+
return nil, nil
865873
}
874+
return nil, err
866875
}
867-
return nil
876+
877+
if delete {
878+
if err := auth.DeleteAuthToken(ctx, authToken); err != nil {
879+
return nil, err
880+
}
881+
}
882+
883+
return u, nil
868884
}
869885

870886
// ValidateUser check if user is valid to insert / update into database

0 commit comments

Comments
 (0)