Improve validation of field size limits in events. (#14664)

This commit is contained in:
reivilibre 2022-12-13 13:19:19 +00:00 committed by GitHub
parent e2a1adbf5d
commit 62ed877433
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 123 additions and 34 deletions

1
changelog.d/14664.bugfix Normal file
View file

@ -0,0 +1 @@
Improve validation of field size limits in events.

View file

@ -45,7 +45,7 @@ class PushRuleEvaluator:
notification_power_levels: Mapping[str, int], notification_power_levels: Mapping[str, int],
related_events_flattened: Mapping[str, Mapping[str, str]], related_events_flattened: Mapping[str, Mapping[str, str]],
related_event_match_enabled: bool, related_event_match_enabled: bool,
room_version_feature_flags: list[str], room_version_feature_flags: Tuple[str, ...],
msc3931_enabled: bool, msc3931_enabled: bool,
): ... ): ...
def run( def run(

View file

@ -152,6 +152,7 @@ class EduTypes:
class RejectedReason: class RejectedReason:
AUTH_ERROR: Final = "auth_error" AUTH_ERROR: Final = "auth_error"
OVERSIZED_EVENT: Final = "oversized_event"
class RoomCreationPreset: class RoomCreationPreset:

View file

@ -424,8 +424,17 @@ class ResourceLimitError(SynapseError):
class EventSizeError(SynapseError): class EventSizeError(SynapseError):
"""An error raised when an event is too big.""" """An error raised when an event is too big."""
def __init__(self, msg: str): def __init__(self, msg: str, unpersistable: bool):
"""
unpersistable:
if True, the PDU must not be persisted, not even as a rejected PDU
when received over federation.
This is notably true when the entire PDU exceeds the size limit for a PDU,
(as opposed to an individual key's size limit being exceeded).
"""
super().__init__(413, msg, Codes.TOO_LARGE) super().__init__(413, msg, Codes.TOO_LARGE)
self.unpersistable = unpersistable
class LoginError(SynapseError): class LoginError(SynapseError):

View file

@ -12,7 +12,7 @@
# See the License for the specific language governing permissions and # See the License for the specific language governing permissions and
# limitations under the License. # limitations under the License.
from typing import Callable, Dict, List, Optional from typing import Callable, Dict, Optional, Tuple
import attr import attr
@ -103,7 +103,7 @@ class RoomVersion:
# is not enough to mark it "supported": the push rule evaluator also needs to # is not enough to mark it "supported": the push rule evaluator also needs to
# support the flag. Unknown flags are ignored by the evaluator, making conditions # support the flag. Unknown flags are ignored by the evaluator, making conditions
# fail if used. # fail if used.
msc3931_push_features: List[str] # values from PushRuleRoomFlag msc3931_push_features: Tuple[str, ...] # values from PushRuleRoomFlag
class RoomVersions: class RoomVersions:
@ -124,7 +124,7 @@ class RoomVersions:
msc2716_redactions=False, msc2716_redactions=False,
msc3787_knock_restricted_join_rule=False, msc3787_knock_restricted_join_rule=False,
msc3667_int_only_power_levels=False, msc3667_int_only_power_levels=False,
msc3931_push_features=[], msc3931_push_features=(),
) )
V2 = RoomVersion( V2 = RoomVersion(
"2", "2",
@ -143,7 +143,7 @@ class RoomVersions:
msc2716_redactions=False, msc2716_redactions=False,
msc3787_knock_restricted_join_rule=False, msc3787_knock_restricted_join_rule=False,
msc3667_int_only_power_levels=False, msc3667_int_only_power_levels=False,
msc3931_push_features=[], msc3931_push_features=(),
) )
V3 = RoomVersion( V3 = RoomVersion(
"3", "3",
@ -162,7 +162,7 @@ class RoomVersions:
msc2716_redactions=False, msc2716_redactions=False,
msc3787_knock_restricted_join_rule=False, msc3787_knock_restricted_join_rule=False,
msc3667_int_only_power_levels=False, msc3667_int_only_power_levels=False,
msc3931_push_features=[], msc3931_push_features=(),
) )
V4 = RoomVersion( V4 = RoomVersion(
"4", "4",
@ -181,7 +181,7 @@ class RoomVersions:
msc2716_redactions=False, msc2716_redactions=False,
msc3787_knock_restricted_join_rule=False, msc3787_knock_restricted_join_rule=False,
msc3667_int_only_power_levels=False, msc3667_int_only_power_levels=False,
msc3931_push_features=[], msc3931_push_features=(),
) )
V5 = RoomVersion( V5 = RoomVersion(
"5", "5",
@ -200,7 +200,7 @@ class RoomVersions:
msc2716_redactions=False, msc2716_redactions=False,
msc3787_knock_restricted_join_rule=False, msc3787_knock_restricted_join_rule=False,
msc3667_int_only_power_levels=False, msc3667_int_only_power_levels=False,
msc3931_push_features=[], msc3931_push_features=(),
) )
V6 = RoomVersion( V6 = RoomVersion(
"6", "6",
@ -219,7 +219,7 @@ class RoomVersions:
msc2716_redactions=False, msc2716_redactions=False,
msc3787_knock_restricted_join_rule=False, msc3787_knock_restricted_join_rule=False,
msc3667_int_only_power_levels=False, msc3667_int_only_power_levels=False,
msc3931_push_features=[], msc3931_push_features=(),
) )
MSC2176 = RoomVersion( MSC2176 = RoomVersion(
"org.matrix.msc2176", "org.matrix.msc2176",
@ -238,7 +238,7 @@ class RoomVersions:
msc2716_redactions=False, msc2716_redactions=False,
msc3787_knock_restricted_join_rule=False, msc3787_knock_restricted_join_rule=False,
msc3667_int_only_power_levels=False, msc3667_int_only_power_levels=False,
msc3931_push_features=[], msc3931_push_features=(),
) )
V7 = RoomVersion( V7 = RoomVersion(
"7", "7",
@ -257,7 +257,7 @@ class RoomVersions:
msc2716_redactions=False, msc2716_redactions=False,
msc3787_knock_restricted_join_rule=False, msc3787_knock_restricted_join_rule=False,
msc3667_int_only_power_levels=False, msc3667_int_only_power_levels=False,
msc3931_push_features=[], msc3931_push_features=(),
) )
V8 = RoomVersion( V8 = RoomVersion(
"8", "8",
@ -276,7 +276,7 @@ class RoomVersions:
msc2716_redactions=False, msc2716_redactions=False,
msc3787_knock_restricted_join_rule=False, msc3787_knock_restricted_join_rule=False,
msc3667_int_only_power_levels=False, msc3667_int_only_power_levels=False,
msc3931_push_features=[], msc3931_push_features=(),
) )
V9 = RoomVersion( V9 = RoomVersion(
"9", "9",
@ -295,7 +295,7 @@ class RoomVersions:
msc2716_redactions=False, msc2716_redactions=False,
msc3787_knock_restricted_join_rule=False, msc3787_knock_restricted_join_rule=False,
msc3667_int_only_power_levels=False, msc3667_int_only_power_levels=False,
msc3931_push_features=[], msc3931_push_features=(),
) )
MSC3787 = RoomVersion( MSC3787 = RoomVersion(
"org.matrix.msc3787", "org.matrix.msc3787",
@ -314,7 +314,7 @@ class RoomVersions:
msc2716_redactions=False, msc2716_redactions=False,
msc3787_knock_restricted_join_rule=True, msc3787_knock_restricted_join_rule=True,
msc3667_int_only_power_levels=False, msc3667_int_only_power_levels=False,
msc3931_push_features=[], msc3931_push_features=(),
) )
V10 = RoomVersion( V10 = RoomVersion(
"10", "10",
@ -333,7 +333,7 @@ class RoomVersions:
msc2716_redactions=False, msc2716_redactions=False,
msc3787_knock_restricted_join_rule=True, msc3787_knock_restricted_join_rule=True,
msc3667_int_only_power_levels=True, msc3667_int_only_power_levels=True,
msc3931_push_features=[], msc3931_push_features=(),
) )
MSC2716v4 = RoomVersion( MSC2716v4 = RoomVersion(
"org.matrix.msc2716v4", "org.matrix.msc2716v4",
@ -352,7 +352,7 @@ class RoomVersions:
msc2716_redactions=True, msc2716_redactions=True,
msc3787_knock_restricted_join_rule=False, msc3787_knock_restricted_join_rule=False,
msc3667_int_only_power_levels=False, msc3667_int_only_power_levels=False,
msc3931_push_features=[], msc3931_push_features=(),
) )
MSC1767v10 = RoomVersion( MSC1767v10 = RoomVersion(
# MSC1767 (Extensible Events) based on room version "10" # MSC1767 (Extensible Events) based on room version "10"
@ -372,7 +372,7 @@ class RoomVersions:
msc2716_redactions=False, msc2716_redactions=False,
msc3787_knock_restricted_join_rule=True, msc3787_knock_restricted_join_rule=True,
msc3667_int_only_power_levels=True, msc3667_int_only_power_levels=True,
msc3931_push_features=[PushRuleRoomFlag.EXTENSIBLE_EVENTS], msc3931_push_features=(PushRuleRoomFlag.EXTENSIBLE_EVENTS,),
) )

View file

@ -52,6 +52,7 @@ from synapse.api.room_versions import (
KNOWN_ROOM_VERSIONS, KNOWN_ROOM_VERSIONS,
EventFormatVersions, EventFormatVersions,
RoomVersion, RoomVersion,
RoomVersions,
) )
from synapse.storage.databases.main.events_worker import EventRedactBehaviour from synapse.storage.databases.main.events_worker import EventRedactBehaviour
from synapse.types import MutableStateMap, StateMap, UserID, get_domain_from_id from synapse.types import MutableStateMap, StateMap, UserID, get_domain_from_id
@ -341,19 +342,80 @@ def check_state_dependent_auth_rules(
logger.debug("Allowing! %s", event) logger.debug("Allowing! %s", event)
# Set of room versions where Synapse did not apply event key size limits
# in bytes, but rather in codepoints.
# In these room versions, we are more lenient with event size validation.
LENIENT_EVENT_BYTE_LIMITS_ROOM_VERSIONS = {
RoomVersions.V1,
RoomVersions.V2,
RoomVersions.V3,
RoomVersions.V4,
RoomVersions.V5,
RoomVersions.V6,
RoomVersions.MSC2176,
RoomVersions.V7,
RoomVersions.V8,
RoomVersions.V9,
RoomVersions.MSC3787,
RoomVersions.V10,
RoomVersions.MSC2716v4,
RoomVersions.MSC1767v10,
}
def _check_size_limits(event: "EventBase") -> None: def _check_size_limits(event: "EventBase") -> None:
if len(event.user_id) > 255: """
raise EventSizeError("'user_id' too large") Checks the size limits in a PDU.
if len(event.room_id) > 255:
raise EventSizeError("'room_id' too large") The entire size limit of the PDU is checked first.
if event.is_state() and len(event.state_key) > 255: Then the size of fields is checked, first in codepoints and then in bytes.
raise EventSizeError("'state_key' too large")
if len(event.type) > 255: The codepoint size limits are only for Synapse compatibility.
raise EventSizeError("'type' too large")
if len(event.event_id) > 255: Raises:
raise EventSizeError("'event_id' too large") EventSizeError:
when a size limit has been violated.
unpersistable=True if Synapse never would have accepted the event and
the PDU must NOT be persisted.
unpersistable=False if a prior version of Synapse would have accepted the
event and so the PDU must be persisted as rejected to avoid
breaking the room.
"""
# Whole PDU check
if len(encode_canonical_json(event.get_pdu_json())) > MAX_PDU_SIZE: if len(encode_canonical_json(event.get_pdu_json())) > MAX_PDU_SIZE:
raise EventSizeError("event too large") raise EventSizeError("event too large", unpersistable=True)
# Codepoint size check: Synapse always enforced these limits, so apply
# them strictly.
if len(event.user_id) > 255:
raise EventSizeError("'user_id' too large", unpersistable=True)
if len(event.room_id) > 255:
raise EventSizeError("'room_id' too large", unpersistable=True)
if event.is_state() and len(event.state_key) > 255:
raise EventSizeError("'state_key' too large", unpersistable=True)
if len(event.type) > 255:
raise EventSizeError("'type' too large", unpersistable=True)
if len(event.event_id) > 255:
raise EventSizeError("'event_id' too large", unpersistable=True)
strict_byte_limits = (
event.room_version not in LENIENT_EVENT_BYTE_LIMITS_ROOM_VERSIONS
)
# Byte size check: if these fail, then be lenient to avoid breaking rooms.
if len(event.user_id.encode("utf-8")) > 255:
raise EventSizeError("'user_id' too large", unpersistable=strict_byte_limits)
if len(event.room_id.encode("utf-8")) > 255:
raise EventSizeError("'room_id' too large", unpersistable=strict_byte_limits)
if event.is_state() and len(event.state_key.encode("utf-8")) > 255:
raise EventSizeError("'state_key' too large", unpersistable=strict_byte_limits)
if len(event.type.encode("utf-8")) > 255:
raise EventSizeError("'type' too large", unpersistable=strict_byte_limits)
if len(event.event_id.encode("utf-8")) > 255:
raise EventSizeError("'event_id' too large", unpersistable=strict_byte_limits)
def _check_create(event: "EventBase") -> None: def _check_create(event: "EventBase") -> None:

View file

@ -43,6 +43,7 @@ from synapse.api.constants import (
from synapse.api.errors import ( from synapse.api.errors import (
AuthError, AuthError,
Codes, Codes,
EventSizeError,
FederationError, FederationError,
FederationPullAttemptBackoffError, FederationPullAttemptBackoffError,
HttpResponseException, HttpResponseException,
@ -1736,6 +1737,15 @@ class FederationEventHandler:
except AuthError as e: except AuthError as e:
logger.warning("Rejecting %r because %s", event, e) logger.warning("Rejecting %r because %s", event, e)
context.rejected = RejectedReason.AUTH_ERROR context.rejected = RejectedReason.AUTH_ERROR
except EventSizeError as e:
if e.unpersistable:
# This event is completely unpersistable.
raise e
# Otherwise, we are somewhat lenient and just persist the event
# as rejected, for moderate compatibility with older Synapse
# versions.
logger.warning("While validating received event %r: %s", event, e)
context.rejected = RejectedReason.OVERSIZED_EVENT
events_and_contexts_to_persist.append((event, context)) events_and_contexts_to_persist.append((event, context))
@ -1781,6 +1791,16 @@ class FederationEventHandler:
# TODO: use a different rejected reason here? # TODO: use a different rejected reason here?
context.rejected = RejectedReason.AUTH_ERROR context.rejected = RejectedReason.AUTH_ERROR
return return
except EventSizeError as e:
if e.unpersistable:
# This event is completely unpersistable.
raise e
# Otherwise, we are somewhat lenient and just persist the event
# as rejected, for moderate compatibility with older Synapse
# versions.
logger.warning("While validating received event %r: %s", event, e)
context.rejected = RejectedReason.OVERSIZED_EVENT
return
# next, check that we have all of the event's auth events. # next, check that we have all of the event's auth events.
# #

View file

@ -342,10 +342,6 @@ class BulkPushRuleEvaluator:
for user_id, level in notification_levels.items(): for user_id, level in notification_levels.items():
notification_levels[user_id] = int(level) notification_levels[user_id] = int(level)
room_version_features = event.room_version.msc3931_push_features
if not room_version_features:
room_version_features = []
evaluator = PushRuleEvaluator( evaluator = PushRuleEvaluator(
_flatten_dict(event, room_version=event.room_version), _flatten_dict(event, room_version=event.room_version),
room_member_count, room_member_count,
@ -353,7 +349,7 @@ class BulkPushRuleEvaluator:
notification_levels, notification_levels,
related_events, related_events,
self._related_event_match_enabled, self._related_event_match_enabled,
room_version_features, event.room_version.msc3931_push_features,
self.hs.config.experimental.msc1767_enabled, # MSC3931 flag self.hs.config.experimental.msc1767_enabled, # MSC3931 flag
) )