Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Strictly enforce canonicaljson requirements in a new room version #7381

Merged
merged 13 commits into from
May 14, 2020
1 change: 1 addition & 0 deletions changelog.d/7381.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add an experimental room version which strictly adheres to the canonical JSON specification.
24 changes: 23 additions & 1 deletion synapse/api/room_versions.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,11 @@ class RoomVersion(object):

# bool: before MSC2261/MSC2432, m.room.aliases had special auth rules and redaction rules
special_case_aliases_auth = attr.ib(type=bool)
clokep marked this conversation as resolved.
Show resolved Hide resolved

# Strictly enforce canonicaljson, do not allow:
# * Integers outside the range of [-2 ^ 53 + 1, 2 ^ 53 - 1]
# * Floats
# * NaN, Infinity, -Infinity
strict_canonicaljson = attr.ib(type=bool)
# bool: MSC2209: Check 'notifications' key while verifying
# m.room.power_levels auth rules.
limit_notifications_power_levels = attr.ib(type=bool)
Expand All @@ -73,6 +77,7 @@ class RoomVersions(object):
StateResolutionVersions.V1,
enforce_key_validity=False,
special_case_aliases_auth=True,
strict_canonicaljson=False,
limit_notifications_power_levels=False,
)
V2 = RoomVersion(
Expand All @@ -82,6 +87,7 @@ class RoomVersions(object):
StateResolutionVersions.V2,
enforce_key_validity=False,
special_case_aliases_auth=True,
strict_canonicaljson=False,
limit_notifications_power_levels=False,
)
V3 = RoomVersion(
Expand All @@ -91,6 +97,7 @@ class RoomVersions(object):
StateResolutionVersions.V2,
enforce_key_validity=False,
special_case_aliases_auth=True,
strict_canonicaljson=False,
limit_notifications_power_levels=False,
)
V4 = RoomVersion(
Expand All @@ -100,6 +107,7 @@ class RoomVersions(object):
StateResolutionVersions.V2,
enforce_key_validity=False,
special_case_aliases_auth=True,
strict_canonicaljson=False,
limit_notifications_power_levels=False,
)
V5 = RoomVersion(
Expand All @@ -109,6 +117,7 @@ class RoomVersions(object):
StateResolutionVersions.V2,
enforce_key_validity=True,
special_case_aliases_auth=True,
strict_canonicaljson=False,
limit_notifications_power_levels=False,
)
MSC2432_DEV = RoomVersion(
Expand All @@ -118,6 +127,17 @@ class RoomVersions(object):
StateResolutionVersions.V2,
enforce_key_validity=True,
special_case_aliases_auth=False,
strict_canonicaljson=False,
limit_notifications_power_levels=False,
)
STRICT_CANONICALJSON = RoomVersion(
"org.matrix.strict_canonicaljson",
RoomDisposition.UNSTABLE,
EventFormatVersions.V3,
StateResolutionVersions.V2,
enforce_key_validity=True,
special_case_aliases_auth=True,
strict_canonicaljson=True,
limit_notifications_power_levels=False,
)
MSC2209_DEV = RoomVersion(
Expand All @@ -127,6 +147,7 @@ class RoomVersions(object):
StateResolutionVersions.V2,
enforce_key_validity=True,
special_case_aliases_auth=True,
strict_canonicaljson=False,
limit_notifications_power_levels=True,
)

Expand All @@ -140,6 +161,7 @@ class RoomVersions(object):
RoomVersions.V4,
RoomVersions.V5,
RoomVersions.MSC2432_DEV,
RoomVersions.STRICT_CANONICALJSON,
RoomVersions.MSC2209_DEV,
)
} # type: Dict[str, RoomVersion]
35 changes: 34 additions & 1 deletion synapse/events/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
# limitations under the License.
import collections
import re
from typing import Mapping, Union
from typing import Any, Mapping, Union

from six import string_types

Expand All @@ -23,6 +23,7 @@
from twisted.internet import defer

from synapse.api.constants import EventTypes, RelationTypes
from synapse.api.errors import Codes, SynapseError
from synapse.api.room_versions import RoomVersion
from synapse.util.async_helpers import yieldable_gather_results

Expand Down Expand Up @@ -449,3 +450,35 @@ def copy_power_levels_contents(
raise TypeError("Invalid power_levels value for %s: %r" % (k, v))

return power_levels


def validate_canonicaljson(value: Any):
"""
Ensure that the JSON object is valid according to the rules of canonical JSON.

See the appendix section 3.1: Canonical JSON.

This rejects JSON that has:
* An integer outside the range of [-2 ^ 53 + 1, 2 ^ 53 - 1]
* Floats
* NaN, Infinity, -Infinity
"""
if isinstance(value, int):
if value <= -(2 ** 53) or 2 ** 53 <= value:
raise SynapseError(400, "JSON integer out of range", Codes.BAD_JSON)

elif isinstance(value, float):
# Note that Infinity, -Infinity, and NaN are also considered floats.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CanonicalJSON indeed does aim to represent all floats as ints, just in case anyone was unsure about blocking all floats: https://matrix.org/docs/spec/appendices#canonical-json

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, as the description says all floats need to be rejected. I didn't find the specification super clear here unless you read into the grammar though. I think we could improve that.

raise SynapseError(400, "Bad JSON value: float", Codes.BAD_JSON)

elif isinstance(value, (dict, frozendict)):
for v in value.values():
validate_canonicaljson(v)

elif isinstance(value, (list, tuple)):
for i in value:
validate_canonicaljson(i)

elif not isinstance(value, (bool, str)) and value is not None:
# Other potential JSON values (bool, None, str) are safe.
raise SynapseError(400, "Unknown JSON value", Codes.BAD_JSON)
7 changes: 7 additions & 0 deletions synapse/events/validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from synapse.api.constants import MAX_ALIAS_LENGTH, EventTypes, Membership
from synapse.api.errors import Codes, SynapseError
from synapse.api.room_versions import EventFormatVersions
from synapse.events.utils import validate_canonicaljson
from synapse.types import EventID, RoomID, UserID


Expand Down Expand Up @@ -55,6 +56,12 @@ def validate_new(self, event, config):
if not isinstance(getattr(event, s), string_types):
raise SynapseError(400, "'%s' not a string type" % (s,))

# Depending on the room version, ensure the data is spec compliant JSON.
if event.room_version.strict_canonicaljson:
# Note that only the client controlled portion of the event is
# checked, since we trust the portions of the event we created.
validate_canonicaljson(event.content)

if event.type == EventTypes.Aliases:
if "aliases" in event.content:
for alias in event.content["aliases"]:
Expand Down
6 changes: 5 additions & 1 deletion synapse/federation/federation_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
from synapse.crypto.event_signing import check_event_content_hash
from synapse.crypto.keyring import Keyring
from synapse.events import EventBase, make_event_from_dict
from synapse.events.utils import prune_event
from synapse.events.utils import prune_event, validate_canonicaljson
from synapse.http.servlet import assert_params_in_dict
from synapse.logging.context import (
PreserveLoggingContext,
Expand Down Expand Up @@ -302,6 +302,10 @@ def event_from_pdu_json(
elif depth > MAX_DEPTH:
raise SynapseError(400, "Depth too large", Codes.BAD_JSON)

# Validate that the JSON conforms to the specification.
if room_version.strict_canonicaljson:
validate_canonicaljson(pdu_json)

event = make_event_from_dict(pdu_json, room_version)
event.internal_metadata.outlier = outlier

Expand Down
2 changes: 1 addition & 1 deletion synapse/util/frozenutils.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,5 +65,5 @@ def _handle_frozendict(obj):
)


# A JSONEncoder which is capable of encoding frozendics without barfing
# A JSONEncoder which is capable of encoding frozendicts without barfing
frozendict_json_encoder = json.JSONEncoder(default=_handle_frozendict)
67 changes: 66 additions & 1 deletion tests/handlers/test_federation.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,12 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import logging
from unittest import TestCase

from synapse.api.constants import EventTypes
from synapse.api.errors import AuthError, Codes
from synapse.api.errors import AuthError, Codes, SynapseError
from synapse.api.room_versions import RoomVersions
from synapse.events import EventBase
from synapse.federation.federation_base import event_from_pdu_json
from synapse.logging.context import LoggingContext, run_in_background
from synapse.rest import admin
Expand Down Expand Up @@ -207,3 +210,65 @@ def _build_and_send_join_event(self, other_server, other_user, room_id):
self.assertEqual(r[(EventTypes.Member, other_user)], join_event.event_id)

return join_event


class EventFromPduTestCase(TestCase):
def test_valid_json(self):
"""Valid JSON should be turned into an event."""
ev = event_from_pdu_json(
{
"type": EventTypes.Message,
"content": {"bool": True, "null": None, "int": 1, "str": "foobar"},
"room_id": "!room:test",
"sender": "@user:test",
"depth": 1,
"prev_events": [],
"auth_events": [],
"origin_server_ts": 1234,
},
RoomVersions.STRICT_CANONICALJSON,
)

self.assertIsInstance(ev, EventBase)

def test_invalid_numbers(self):
"""Invalid values for an integer should be rejected, all floats should be rejected."""
for value in [
-(2 ** 53),
2 ** 53,
1.0,
float("inf"),
float("-inf"),
float("nan"),
]:
with self.assertRaises(SynapseError):
event_from_pdu_json(
{
"type": EventTypes.Message,
"content": {"foo": value},
"room_id": "!room:test",
"sender": "@user:test",
"depth": 1,
"prev_events": [],
"auth_events": [],
"origin_server_ts": 1234,
},
RoomVersions.STRICT_CANONICALJSON,
)

def test_invalid_nested(self):
"""List and dictionaries are recursively searched."""
with self.assertRaises(SynapseError):
event_from_pdu_json(
{
"type": EventTypes.Message,
"content": {"foo": [{"bar": 2 ** 56}]},
"room_id": "!room:test",
"sender": "@user:test",
"depth": 1,
"prev_events": [],
"auth_events": [],
"origin_server_ts": 1234,
},
RoomVersions.STRICT_CANONICALJSON,
)