Check userID for nil to prevent panic (#426)

Author: devonh

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)
 		}
 	}

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.

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
 }