Elgato_dark/fleet
1
0
Fork 0
mirror of https://github.com/fleetdm/fleet synced 2026-05-24 01:18:42 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 59

Prevent escalation of user privileges via ModifyUser method

Browse source 
The ability to modify a users admin and enabled status was erroneously left in
place during development of https://github.com/kolide/fleet/pull/959. To
mitigate a privilege escalation vulnerability we need to ensure those values
can only be modified through the explicit methods.

This patch includes a unit test and fix for the vulnerability.

Thanks to 'Quikke' for submitting this vulnerability.
This commit is contained in:
Zachary Wasserman 2018-09-12 14:16:59 -07:00
parent e011cfc464
commit 21269b1dd8
2 changed files with 33 additions and 8 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
server/service/service_users.go
View file

@ -92,14 +92,6 @@ func (svc service) ModifyUser(ctx context.Context, userID uint, p kolide.UserPay
// the method assumes that the correct authorization
// has been validated higher up the stack
if p.Admin != nil {
user.Admin = *p.Admin
}
if p.Enabled != nil {
user.Enabled = *p.Enabled
}
if p.Username != nil {
user.Username = *p.Username
}

33
server/service/service_users_test.go
View file

@ -86,6 +86,39 @@ func TestModifyUserEmail(t *testing.T) {
}
func TestModifyUserCannotUpdateAdminEnabled(t *testing.T) {
// The modify user function should not be able to update the admin or
// enabled status of a user. These should only be updated explicitly
// through the ChangeUserAdmin and ChangeUserEnabled functions.
user := &kolide.User{
ID: 3,
Admin: false,
Email: "foo@bar.com",
Enabled: true,
}
user.SetPassword("password", 10, 10)
ms := new(mock.Store)
ms.UserByIDFunc = func(id uint) (*kolide.User, error) {
return user, nil
}
ms.SaveUserFunc = func(u *kolide.User) error {
assert.Equal(t, false, u.Admin, "should not be able to update admin status!")
assert.Equal(t, true, u.Enabled, "should not be able to update enabled status!")
return nil
}
svc, err := newTestService(ms, nil)
ctx := context.Background()
ctx = viewer.NewContext(ctx, viewer.Viewer{User: user})
payload := kolide.UserPayload{
Admin: boolPtr(true),
Enabled: boolPtr(false),
}
_, err = svc.ModifyUser(ctx, 3, payload)
require.Nil(t, err)
assert.True(t, ms.SaveUserFuncInvoked)
}
func TestModifyUserEmailNoPassword(t *testing.T) {
user := &kolide.User{
ID: 3,