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

Reject receipt requests with empty room or event IDs #14632

Merged
merged 6 commits into from
Dec 7, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/14632.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Reject invalid read receipt requests with empty room or event IDs. Contributed by Nick @ Beeper (@fizzadar).
Copy link
Member

Choose a reason for hiding this comment

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

I'm half tempted to say this is a bugfix, but 🤷

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah good spot, agreed! fbb7b16

5 changes: 4 additions & 1 deletion synapse/rest/client/receipts.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from synapse.http.server import HttpServer
from synapse.http.servlet import RestServlet, parse_json_object_from_request
from synapse.http.site import SynapseRequest
from synapse.types import JsonDict
from synapse.types import EventID, JsonDict, RoomID

from ._base import client_patterns

Expand Down Expand Up @@ -56,6 +56,9 @@ async def on_POST(
) -> Tuple[int, JsonDict]:
requester = await self.auth.get_user_by_req(request)

if not RoomID.is_valid(room_id) or not event_id.startswith(EventID.SIGIL):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not super happy with the startswith but EventID.is_valid is broken for non-V1/2 event IDs so unsure how to proceed.

Copy link
Member

Choose a reason for hiding this comment

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

Filed #14638.

raise SynapseError(400, "A valid room ID and event ID must be specified")

if receipt_type not in self._known_receipt_types:
raise SynapseError(
400,
Expand Down
77 changes: 77 additions & 0 deletions tests/rest/client/test_receipts.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
# Copyright 2018-2019 New Vector Ltd
# Copyright 2019 The Matrix.org Foundation C.I.C.
Fizzadar marked this conversation as resolved.
Show resolved Hide resolved
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
from twisted.test.proto_helpers import MemoryReactor

import synapse.rest.admin
from synapse.rest.client import login, receipts, register
from synapse.server import HomeServer
from synapse.util import Clock

from tests import unittest


class ReceiptsTestCase(unittest.HomeserverTestCase):
servlets = [
login.register_servlets,
register.register_servlets,
receipts.register_servlets,
synapse.rest.admin.register_servlets,
]

def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
self.owner = self.register_user("owner", "pass")
self.owner_tok = self.login("owner", "pass")

def test_send_receipt(self) -> None:
channel = self.make_request(
"POST",
"/rooms/!abc:beep/receipt/m.read/$def",
content={},
access_token=self.owner_tok,
)
self.assertEqual(channel.code, 200, channel.result)

def test_send_receipt_invalid_room_id(self) -> None:
channel = self.make_request(
"POST",
"/rooms/not-a-room-id/receipt/m.read/$def",
content={},
access_token=self.owner_tok,
)
self.assertEqual(channel.code, 400, channel.result)
self.assertEqual(
channel.json_body["error"], "A valid room ID and event ID must be specified"
)

def test_send_receipt_invalid_event_id(self) -> None:
channel = self.make_request(
"POST",
"/rooms/!abc:beep/receipt/m.read/not-an-event-id",
content={},
access_token=self.owner_tok,
)
self.assertEqual(channel.code, 400, channel.result)
self.assertEqual(
channel.json_body["error"], "A valid room ID and event ID must be specified"
)

def test_send_receipt_invalid_receipt_type(self) -> None:
channel = self.make_request(
"POST",
"/rooms/!abc:beep/receipt/invalid-receipt-type/$def",
content={},
access_token=self.owner_tok,
)
self.assertEqual(channel.code, 400, channel.result)