Skip to content

Commit

Permalink
Check userID for nil to prevent panic (#426)
Browse files Browse the repository at this point in the history
  • Loading branch information
devonh authored Dec 14, 2023
1 parent e97395e commit 85489e2
Show file tree
Hide file tree
Showing 3 changed files with 101 additions and 1 deletion.
2 changes: 1 addition & 1 deletion eventauth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
96 changes: 96 additions & 0 deletions eventauth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"testing"

"github.com/matrix-org/gomatrixserverlib/spec"
"github.com/stretchr/testify/assert"
)

func stateNeededEquals(a, b StateNeeded) bool {
Expand Down Expand Up @@ -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.
Expand Down
4 changes: 4 additions & 0 deletions eventcontent.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down

0 comments on commit 85489e2

Please sign in to comment.