Add more sanity checks

Signed-off-by: Alex Chen <minecnly@gmail.com>

clientapi/routing/redact.go

@@ -81,49 +81,10 @@ func Redact(
 		return httputil.LogThenError(req, err)
 	}
 
-	// Do some basic checks e.g. ensuring the user is in the room and can send m.room.redaction events
+	if resErr := checkRedactionAllowed(
-	stateEvents := make([]*gomatrixserverlib.Event, len(queryRes.StateEvents))
+		req, queryAPI, e, queryRes.StateEvents,
-	for i := range queryRes.StateEvents {
+	); resErr != nil {
-		stateEvents[i] = &queryRes.StateEvents[i]
+		return *resErr
-	}
-	provider := gomatrixserverlib.NewAuthEvents(stateEvents)
-	if err = gomatrixserverlib.Allowed(*e, &provider); err != nil {
-		// TODO: Is the error returned with suitable HTTP status code?
-		if _, ok := err.(*gomatrixserverlib.NotAllowed); ok {
-			return util.JSONResponse{
-				Code: http.StatusForbidden,
-				JSON: jsonerror.Forbidden(err.Error()),
-			}
-		}
-
-		return httputil.LogThenError(req, err)
-	}
-
-	// Ensure the user can redact the specific event
-
-	eventReq := api.QueryEventsByIDRequest{
-		EventIDs: []string{redactedEventID},
-	}
-	var eventResp api.QueryEventsByIDResponse
-	if err = queryAPI.QueryEventsByID(req.Context(), &eventReq, &eventResp); err != nil {
-		return httputil.LogThenError(req, err)
-	}
-
-	if len(eventResp.Events) == 0 {
-		return util.JSONResponse{
-			Code: http.StatusNotFound,
-			JSON: jsonerror.NotFound("Event to redact not found"),
-		}
-	}
-
-	redactedEvent := eventResp.Events[0]
-
-	if redactedEvent.Sender() != device.UserID {
-		// TODO: Allow power users to redact others' events
-		return util.JSONResponse{
-			Code: http.StatusForbidden,
-			JSON: jsonerror.Forbidden("You are not allowed to redact this event"),
-		}
 	}
 
 	// Send the redaction event
@@ -150,3 +111,70 @@ func Redact(
 		JSON: redactResponse{eventID},
 	}
 }
+
+func checkRedactionAllowed(
+	req *http.Request, queryAPI api.RoomserverQueryAPI,
+	redactionEvent *gomatrixserverlib.Event,
+	stateEvents []gomatrixserverlib.Event,
+) *util.JSONResponse {
+	// Do some basic checks e.g. ensuring the user is in the room and can send m.room.redaction events
+	statEventPointers := make([]*gomatrixserverlib.Event, len(stateEvents))
+	for i := range stateEvents {
+		statEventPointers[i] = &stateEvents[i]
+	}
+	provider := gomatrixserverlib.NewAuthEvents(statEventPointers)
+	if err := gomatrixserverlib.Allowed(*redactionEvent, &provider); err != nil {
+		// TODO: Is the error returned with suitable HTTP status code?
+		if _, ok := err.(*gomatrixserverlib.NotAllowed); ok {
+			return &util.JSONResponse{
+				Code: http.StatusForbidden,
+				JSON: jsonerror.Forbidden(err.Error()),
+			}
+		}
+
+		res := httputil.LogThenError(req, err)
+		return &res
+	}
+
+	// Ensure the user can redact the specific event
+
+	eventReq := api.QueryEventsByIDRequest{
+		EventIDs: []string{redactionEvent.Redacts()},
+	}
+	var eventResp api.QueryEventsByIDResponse
+	if err := queryAPI.QueryEventsByID(req.Context(), &eventReq, &eventResp); err != nil {
+		res := httputil.LogThenError(req, err)
+		return &res
+	}
+
+	if len(eventResp.Events) == 0 {
+		return &util.JSONResponse{
+			Code: http.StatusNotFound,
+			JSON: jsonerror.NotFound("Event to redact not found"),
+		}
+	}
+
+	redactedEvent := eventResp.Events[0]
+
+	badEvents, _, err := common.ValidateRedaction(&redactedEvent, redactionEvent)
+	if err != nil {
+		res := httputil.LogThenError(req, err)
+		return &res
+	}
+	if badEvents {
+		return &util.JSONResponse{
+			Code: http.StatusBadRequest,
+			JSON: jsonerror.BadJSON("invalid redaction attempt"),
+		}
+	}
+
+	if redactedEvent.Sender() != redactionEvent.Sender() {
+		// TODO: Allow power users to redact others' events
+		return &util.JSONResponse{
+			Code: http.StatusForbidden,
+			JSON: jsonerror.Forbidden("You are not allowed to redact this event"),
+		}
+	}
+
+	return nil
+}

common/redaction.go

@@ -0,0 +1,40 @@
+package common
+
+import "github.com/matrix-org/gomatrixserverlib"
+
+func ValidateRedaction(
+	redacted, redaction *gomatrixserverlib.Event,
+) (badEvents, needPowerLevelCheck bool, err error) {
+	// Don't allow redaction of events in different rooms
+	if redaction.RoomID() != redacted.RoomID() {
+		return true, false, nil
+	}
+
+	// Don't allow an event to redact itself
+	if redaction.Redacts() == redaction.EventID() {
+		return true, false, nil
+	}
+
+	// Don't allow two events to redact each other
+	if redacted.Redacts() == redaction.EventID() {
+		return true, false, nil
+	}
+
+	var expectedDomain, redactorDomain gomatrixserverlib.ServerName
+	if _, expectedDomain, err = gomatrixserverlib.SplitID(
+		'@', redacted.Sender(),
+	); err != nil {
+		return false, false, err
+	}
+	if _, redactorDomain, err = gomatrixserverlib.SplitID(
+		'@', redaction.Sender(),
+	); err != nil {
+		return false, false, err
+	}
+
+	if expectedDomain != redactorDomain {
+		return false, true, err
+	}
+
+	return false, false, nil
+}

roomserver/storage/storage.go

@@ -360,20 +360,18 @@ func (d *Database) validateRedactions(
 ) (validatedRedactions map[string]types.EventNID, err error) {
 	validatedRedactions = make(map[string]types.EventNID, len(unvalidatedRedactions))
 
-	var expectedDomain, redactorDomain gomatrixserverlib.ServerName
 	for redactedEventID, redactedByNID := range unvalidatedRedactions {
-		if _, expectedDomain, err = gomatrixserverlib.SplitID(
+		badEvents, needPowerLevelCheck, validationErr := common.ValidateRedaction(
-			'@', eventIDToEvent[redactedEventID].Sender(),
+			eventIDToEvent[redactedEventID], redactionNIDToEvent[redactedByNID],
-		); err != nil {
+		)
-			return nil, err
+		if validationErr != nil {
+			return nil, validationErr
 		}
-		if _, redactorDomain, err = gomatrixserverlib.SplitID(
+		if badEvents {
-			'@', redactionNIDToEvent[redactedByNID].Sender(),
+			continue
-		); err != nil {
-			return nil, err
 		}
 
-		if redactorDomain != expectedDomain {
+		if needPowerLevelCheck {
 			// TODO: Still allow power users to redact
 			continue
 		}

syncapi/storage/syncserver.go

@@ -986,20 +986,18 @@ func (d *SyncServerDatasource) validateRedactions(
 ) (validatedRedactions redactedToRedactionMap, err error) {
 	validatedRedactions = make(redactedToRedactionMap, len(unvalidatedRedactions))
 
-	var expectedDomain, redactorDomain gomatrixserverlib.ServerName
 	for redactedEventID, redactedByID := range unvalidatedRedactions {
-		if _, expectedDomain, err = gomatrixserverlib.SplitID(
+		badEvents, needPowerLevelCheck, validationErr := common.ValidateRedaction(
-			'@', eventIDToEvent[redactedEventID].Sender(),
+			eventIDToEvent[redactedEventID], redactionIDToEvent[redactedByID],
-		); err != nil {
+		)
-			return nil, err
+		if validationErr != nil {
+			return nil, validationErr
 		}
-		if _, redactorDomain, err = gomatrixserverlib.SplitID(
+		if badEvents {
-			'@', redactionIDToEvent[redactedByID].Sender(),
+			continue
-		); err != nil {
-			return nil, err
 		}
 
-		if redactorDomain != expectedDomain {
+		if needPowerLevelCheck {
 			// TODO: Still allow power users to redact
 			continue
 		}

testfile

@@ -156,3 +156,7 @@ User can create and send/receive messages in a room with version 1
 POST /createRoom ignores attempts to set the room version via creation_content
 Inbound federation rejects remote attempts to join local users to rooms
 Inbound federation rejects remote attempts to kick local users to rooms
+An event which redacts itself should be ignored
+A pair of events which redact each other should be ignored
+POST /rooms/:room_id/redact/:event_id as original message sender redacts message
+POST /rooms/:room_id/redact/:event_id as random user does not redact message