From 85489e2db3f55110bd0821eedcb0922619b5a21d Mon Sep 17 00:00:00 2001 From: devonh Date: Thu, 14 Dec 2023 16:52:34 +0000 Subject: [PATCH] Check userID for nil to prevent panic (#426) --- eventauth.go | 2 +- eventauth_test.go | 96 +++++++++++++++++++++++++++++++++++++++++++++++ eventcontent.go | 4 ++ 3 files changed, 101 insertions(+), 1 deletion(-) diff --git a/eventauth.go b/eventauth.go index ee1fd24e..0610ac0e 100644 --- a/eventauth.go +++ b/eventauth.go @@ -526,7 +526,7 @@ func (a *allowerContext) powerLevelsEventAllowed(event PDU) error { if err != nil { return err } - if !isValidUserID(sender.String()) { + if sender == nil || !isValidUserID(sender.String()) { return errorf("Not a valid user ID: %q", senderID) } } diff --git a/eventauth_test.go b/eventauth_test.go index 88773fc4..74463a3d 100644 --- a/eventauth_test.go +++ b/eventauth_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/matrix-org/gomatrixserverlib/spec" + "github.com/stretchr/testify/assert" ) func stateNeededEquals(a, b StateNeeded) bool { @@ -1116,6 +1117,101 @@ func TestDemoteUserDefaultPowerLevelBelowOwn(t *testing.T) { } } +func NilUserIDForBadSenderTest(roomID spec.RoomID, senderID spec.SenderID) (*spec.UserID, error) { + if senderID == "@baduser" { + return nil, nil + } + + return spec.NewUserID(string(senderID), true) +} + +var nilPowerLevelTestRoom = &testAuthEvents{ + CreateJSON: json.RawMessage(`{ + "type": "m.room.create", + "state_key": "", + "sender": "@baduser", + "room_id": "!r1:a", + "event_id": "$e1:a", + "content": { + "room_version": "1" + } + }`), + PowerLevelsJSON: json.RawMessage(`{ + "type": "m.room.power_levels", + "state_key": "", + "sender": "@u1:a", + "room_id": "!r1:a", + "event_id": "$e3:a", + "content": { + "users_default": 100, + "users": { + "@u1:a": 100 + }, + "redact": 100 + } + }`), + MemberJSON: map[string]json.RawMessage{ + "@u1:a": json.RawMessage(`{ + "type": "m.room.member", + "state_key": "@u1:a", + "sender": "@u1:a", + "room_id": "!r1:a", + "event_id": "$e2:a", + "content": { + "membership": "join" + } + }`), + }, +} + +func TestPowerLevelCheckShouldNotPanic(t *testing.T) { + powerChangeBadUser, err := MustGetRoomVersion(RoomVersionV1).NewEventFromTrustedJSON(spec.RawJSON(`{ + "type": "m.room.power_levels", + "state_key": "", + "sender": "@u1:a", + "room_id": "!r1:a", + "event_id": "$e5:a", + "content": { + "users_default": 50, + "users": { + "@baduser": 0 + }, + "redact": 100 + } + }`), false) + if err != nil { + t.Fatal(err) + } + assert.NotPanics(t, func() { + if err := Allowed(powerChangeBadUser, powerLevelTestRoom, NilUserIDForBadSenderTest); err == nil { + panic("Event should not be allowed") + } + }, "") + + powerChange, err := MustGetRoomVersion(RoomVersionV1).NewEventFromTrustedJSON(spec.RawJSON(`{ + "type": "m.room.power_levels", + "state_key": "", + "sender": "@u1:a", + "room_id": "!r1:a", + "event_id": "$e5:a", + "content": { + "users_default": 50, + "users": { + "@u1:a": 0 + }, + "redact": 100 + } + }`), false) + if err != nil { + t.Fatal(err) + } + assert.NotPanics(t, func() { + if err := Allowed(powerChange, nilPowerLevelTestRoom, NilUserIDForBadSenderTest); err == nil { + panic("Event should not be allowed") + } + }, "") +} + func TestPromoteUserDefaultLevelAboveOwn(t *testing.T) { // User shouldn't be able to promote the user default // level above their own effective level. diff --git a/eventcontent.go b/eventcontent.go index 879b4980..05b80bea 100644 --- a/eventcontent.go +++ b/eventcontent.go @@ -76,6 +76,10 @@ func NewCreateContentFromAuthEvents(authEvents AuthEventProvider, userIDForSende err = errorf("invalid sender userID: %s", err.Error()) return } + if sender == nil { + err = errorf("userID not found for sender: %s in room %s", createEvent.SenderID(), createEvent.RoomID().String()) + return + } c.senderDomain = string(sender.Domain()) return }