From 5aa2c77ac1ac544ed6b3a2c5efa767e53b810c3b Mon Sep 17 00:00:00 2001 From: linoman <2051016+linoman@users.noreply.github.com> Date: Fri, 16 Sep 2022 10:46:44 +0200 Subject: [PATCH] fix CVE-2022-39229 Swap order of login fields (cherry picked from commit 5ec176cada3d8adf651f844e3f707bc469495abd) Add test for username/login field conflict (cherry picked from commit 7aabcf26944835b0418eec6b057a0b186ff206bf) Co-authored-by: linoman <2051016+linoman@users.noreply.github.com> Co-authored-by: dsotirakis diff --git a/pkg/services/sqlstore/user.go b/pkg/services/sqlstore/user.go index 3dba16a75e..d773bd9dfe 100644 --- a/pkg/services/sqlstore/user.go +++ b/pkg/services/sqlstore/user.go @@ -298,19 +298,24 @@ func GetUserByLogin(query *models.GetUserByLoginQuery) error { return models.ErrUserNotFound } - // Try and find the user by login first. - // It's not sufficient to assume that a LoginOrEmail with an "@" is an email. + var has bool + var err error user := &models.User{Login: query.LoginOrEmail} - has, err := x.Get(user) - if err != nil { - return err + // Since username can be an email address, attempt login with email address + // first if the login field has the "@" symbol. + if strings.Contains(query.LoginOrEmail, "@") { + user = &models.User{Email: query.LoginOrEmail} + has, err = x.Get(user) + + if err != nil { + return err + } } - if !has && strings.Contains(query.LoginOrEmail, "@") { - // If the user wasn't found, and it contains an "@" fallback to finding the - // user by email. - user = &models.User{Email: query.LoginOrEmail} + // Lookup the login field instead of email field + if !has { + user = &models.User{Login: query.LoginOrEmail} has, err = x.Get(user) } diff --git a/pkg/services/sqlstore/user_test.go b/pkg/services/sqlstore/user_test.go index aa796ffb02..7fb9d9be2a 100644 --- a/pkg/services/sqlstore/user_test.go +++ b/pkg/services/sqlstore/user_test.go @@ -42,6 +43,45 @@ func TestUserDataAccess(t *testing.T) { }) }) + Convey("Get User by login - user_2 uses user_1.email as login", func() { + ss = InitTestDB(t) + + // create user_1 + cmd1 := &models.CreateUserCommand{ + Email: "user_1@mail.com", + Name: "user_1", + Login: "user_1", + Password: "user_1_password", + IsDisabled: true, + } + err := CreateUser(context.Background(), cmd1) + So(err, ShouldBeNil) + + // create user_2 + cmd2 := &models.CreateUserCommand{ + Email: "user_2@mail.com", + Name: "user_2", + Login: "user_1@mail.com", + Password: "user_2_password", + IsDisabled: true, + } + err = CreateUser(context.Background(), cmd2) + So(err, ShouldBeNil) + + // query user database for user_1 email + query := models.GetUserByLoginQuery{LoginOrEmail: "user_1@mail.com"} + err = GetUserByLogin(&query) + So(err, ShouldBeNil) + + // expect user_1 as result + So(query.Result.Email, ShouldEqual, cmd1.Email) + So(query.Result.Login, ShouldEqual, cmd1.Login) + So(query.Result.Name, ShouldEqual, cmd1.Name) + So(query.Result.Email, ShouldNotEqual, cmd2.Email) + So(query.Result.Login, ShouldNotEqual, cmd2.Login) + So(query.Result.Name, ShouldNotEqual, cmd2.Name) + }) + Convey("Creates disabled user", func() { cmd := &models.CreateUserCommand{ Email: "usertest@test.com",