diff --git a/changelog.d/15311.misc b/changelog.d/15311.misc
new file mode 100644
index 0000000000..ce03cb9523
--- /dev/null
+++ b/changelog.d/15311.misc
@@ -0,0 +1 @@
+Reject events with an invalid "mentions" property pert [MSC3952](https://github.com/matrix-org/matrix-spec-proposals/pull/3952).
diff --git a/synapse/events/validator.py b/synapse/events/validator.py
index 6f0e4386d3..47203209db 100644
--- a/synapse/events/validator.py
+++ b/synapse/events/validator.py
@@ -12,11 +12,17 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import collections.abc
-from typing import Iterable, Type, Union, cast
+from typing import Iterable, List, Type, Union, cast
import jsonschema
+from pydantic import Field, StrictBool, StrictStr
-from synapse.api.constants import MAX_ALIAS_LENGTH, EventTypes, Membership
+from synapse.api.constants import (
+ MAX_ALIAS_LENGTH,
+ EventContentFields,
+ EventTypes,
+ Membership,
+)
from synapse.api.errors import Codes, SynapseError
from synapse.api.room_versions import EventFormatVersions
from synapse.config.homeserver import HomeServerConfig
@@ -28,6 +34,8 @@ from synapse.events.utils import (
validate_canonicaljson,
)
from synapse.federation.federation_server import server_matches_acl_event
+from synapse.http.servlet import validate_json_object
+from synapse.rest.models import RequestBodyModel
from synapse.types import EventID, JsonDict, RoomID, UserID
@@ -88,27 +96,27 @@ class EventValidator:
Codes.INVALID_PARAM,
)
- if event.type == EventTypes.Retention:
+ elif event.type == EventTypes.Retention:
self._validate_retention(event)
- if event.type == EventTypes.ServerACL:
+ elif event.type == EventTypes.ServerACL:
if not server_matches_acl_event(config.server.server_name, event):
raise SynapseError(
400, "Can't create an ACL event that denies the local server"
)
- if event.type == EventTypes.PowerLevels:
+ elif event.type == EventTypes.PowerLevels:
try:
jsonschema.validate(
instance=event.content,
schema=POWER_LEVELS_SCHEMA,
- cls=plValidator,
+ cls=POWER_LEVELS_VALIDATOR,
)
except jsonschema.ValidationError as e:
if e.path:
# example: "users_default": '0' is not of type 'integer'
# cast safety: path entries can be integers, if we fail to validate
- # items in an array. However the POWER_LEVELS_SCHEMA doesn't expect
+ # items in an array. However, the POWER_LEVELS_SCHEMA doesn't expect
# to see any arrays.
message = (
'"' + cast(str, e.path[-1]) + '": ' + e.message # noqa: B306
@@ -125,6 +133,15 @@ class EventValidator:
errcode=Codes.BAD_JSON,
)
+ # If the event contains a mentions key, validate it.
+ if (
+ EventContentFields.MSC3952_MENTIONS in event.content
+ and config.experimental.msc3952_intentional_mentions
+ ):
+ validate_json_object(
+ event.content[EventContentFields.MSC3952_MENTIONS], Mentions
+ )
+
def _validate_retention(self, event: EventBase) -> None:
"""Checks that an event that defines the retention policy for a room respects the
format enforced by the spec.
@@ -253,10 +270,15 @@ POWER_LEVELS_SCHEMA = {
}
+class Mentions(RequestBodyModel):
+ user_ids: List[StrictStr] = Field(default_factory=list)
+ room: StrictBool = False
+
+
# This could return something newer than Draft 7, but that's the current "latest"
# validator.
-def _create_power_level_validator() -> Type[jsonschema.Draft7Validator]:
- validator = jsonschema.validators.validator_for(POWER_LEVELS_SCHEMA)
+def _create_validator(schema: JsonDict) -> Type[jsonschema.Draft7Validator]:
+ validator = jsonschema.validators.validator_for(schema)
# by default jsonschema does not consider a immutabledict to be an object so
# we need to use a custom type checker
@@ -268,4 +290,4 @@ def _create_power_level_validator() -> Type[jsonschema.Draft7Validator]:
return jsonschema.validators.extend(validator, type_checker=type_checker)
-plValidator = _create_power_level_validator()
+POWER_LEVELS_VALIDATOR = _create_validator(POWER_LEVELS_SCHEMA)
diff --git a/synapse/http/servlet.py b/synapse/http/servlet.py
index 0070bd2940..fc62793628 100644
--- a/synapse/http/servlet.py
+++ b/synapse/http/servlet.py
@@ -778,17 +778,13 @@ def parse_json_object_from_request(
Model = TypeVar("Model", bound=BaseModel)
-def parse_and_validate_json_object_from_request(
- request: Request, model_type: Type[Model]
-) -> Model:
- """Parse a JSON object from the body of a twisted HTTP request, then deserialise and
- validate using the given pydantic model.
+def validate_json_object(content: JsonDict, model_type: Type[Model]) -> Model:
+ """Validate a deserialized JSON object using the given pydantic model.
Raises:
SynapseError if the request body couldn't be decoded as JSON or
if it wasn't a JSON object.
"""
- content = parse_json_object_from_request(request, allow_empty_body=False)
try:
instance = model_type.parse_obj(content)
except ValidationError as e:
@@ -811,6 +807,20 @@ def parse_and_validate_json_object_from_request(
return instance
+def parse_and_validate_json_object_from_request(
+ request: Request, model_type: Type[Model]
+) -> Model:
+ """Parse a JSON object from the body of a twisted HTTP request, then deserialise and
+ validate using the given pydantic model.
+
+ Raises:
+ SynapseError if the request body couldn't be decoded as JSON or
+ if it wasn't a JSON object.
+ """
+ content = parse_json_object_from_request(request, allow_empty_body=False)
+ return validate_json_object(content, model_type)
+
+
def assert_params_in_dict(body: JsonDict, required: Iterable[str]) -> None:
absent = []
for k in required:
diff --git a/tests/push/test_bulk_push_rule_evaluator.py b/tests/push/test_bulk_push_rule_evaluator.py
index 46df0102f7..9501096a77 100644
--- a/tests/push/test_bulk_push_rule_evaluator.py
+++ b/tests/push/test_bulk_push_rule_evaluator.py
@@ -243,22 +243,28 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
)
# Non-dict mentions should be ignored.
- mentions: Any
- for mentions in (None, True, False, 1, "foo", []):
- self.assertFalse(
- self._create_and_process(
- bulk_evaluator, {EventContentFields.MSC3952_MENTIONS: mentions}
+ #
+ # Avoid C-S validation as these aren't expected.
+ with patch(
+ "synapse.events.validator.EventValidator.validate_new",
+ new=lambda s, event, config: True,
+ ):
+ mentions: Any
+ for mentions in (None, True, False, 1, "foo", []):
+ self.assertFalse(
+ self._create_and_process(
+ bulk_evaluator, {EventContentFields.MSC3952_MENTIONS: mentions}
+ )
)
- )
- # A non-list should be ignored.
- for mentions in (None, True, False, 1, "foo", {}):
- self.assertFalse(
- self._create_and_process(
- bulk_evaluator,
- {EventContentFields.MSC3952_MENTIONS: {"user_ids": mentions}},
+ # A non-list should be ignored.
+ for mentions in (None, True, False, 1, "foo", {}):
+ self.assertFalse(
+ self._create_and_process(
+ bulk_evaluator,
+ {EventContentFields.MSC3952_MENTIONS: {"user_ids": mentions}},
+ )
)
- )
# The Matrix ID appearing anywhere in the list should notify.
self.assertTrue(
@@ -291,26 +297,32 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
)
# Invalid entries in the list are ignored.
- self.assertFalse(
- self._create_and_process(
- bulk_evaluator,
- {
- EventContentFields.MSC3952_MENTIONS: {
- "user_ids": [None, True, False, {}, []]
- }
- },
+ #
+ # Avoid C-S validation as these aren't expected.
+ with patch(
+ "synapse.events.validator.EventValidator.validate_new",
+ new=lambda s, event, config: True,
+ ):
+ self.assertFalse(
+ self._create_and_process(
+ bulk_evaluator,
+ {
+ EventContentFields.MSC3952_MENTIONS: {
+ "user_ids": [None, True, False, {}, []]
+ }
+ },
+ )
)
- )
- self.assertTrue(
- self._create_and_process(
- bulk_evaluator,
- {
- EventContentFields.MSC3952_MENTIONS: {
- "user_ids": [None, True, False, {}, [], self.alice]
- }
- },
+ self.assertTrue(
+ self._create_and_process(
+ bulk_evaluator,
+ {
+ EventContentFields.MSC3952_MENTIONS: {
+ "user_ids": [None, True, False, {}, [], self.alice]
+ }
+ },
+ )
)
- )
# The legacy push rule should not mention if the mentions field exists.
self.assertFalse(
@@ -351,14 +363,20 @@ class TestBulkPushRuleEvaluator(HomeserverTestCase):
)
# Invalid data should not notify.
- mentions: Any
- for mentions in (None, False, 1, "foo", [], {}):
- self.assertFalse(
- self._create_and_process(
- bulk_evaluator,
- {EventContentFields.MSC3952_MENTIONS: {"room": mentions}},
+ #
+ # Avoid C-S validation as these aren't expected.
+ with patch(
+ "synapse.events.validator.EventValidator.validate_new",
+ new=lambda s, event, config: True,
+ ):
+ mentions: Any
+ for mentions in (None, False, 1, "foo", [], {}):
+ self.assertFalse(
+ self._create_and_process(
+ bulk_evaluator,
+ {EventContentFields.MSC3952_MENTIONS: {"room": mentions}},
+ )
)
- )
# The legacy push rule should not mention if the mentions field exists.
self.assertFalse(
|
