OpenCloud
/
MatrixSynapse
mirror of https://github.com/matrix-org/synapse
1
0
Fork 0
Code Issues Releases Wiki Activity

Configurable limits on avatars (#11846) 

Only allow files which file size and content types match configured
limits to be set as avatar.

Most of the inspiration from the non-test code comes from matrix-org/synapse-dinsic#19
pull/11860/head
Brendan Abolivier 2022-01-28 14:41:33 +00:00 committed by GitHub
parent 6d482ba259
commit bf60da1a60
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 363 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
changelog.d/11846.feature Normal file
View File

@ -0,0 +1 @@
Allow configuring a maximum file size as well as a list of allowed content types for avatars.

14
docs/sample_config.yaml
View File

@ -471,6 +471,20 @@ limit_remote_rooms:
#
#allow_per_room_profiles: false
# The largest allowed file size for a user avatar. Defaults to no restriction.
#
# Note that user avatar changes will not work if this is set without
# using Synapse's media repository.
#
#max_avatar_size: 10M
# The MIME types allowed for user avatars. Defaults to no restriction.
#
# Note that user avatar changes will not work if this is set without
# using Synapse's media repository.
#
#allowed_avatar_mimetypes: ["image/png", "image/jpeg", "image/gif"]
# How long to keep redacted events in unredacted form in the database. After
# this period redacted events get replaced with their redacted form in the DB.
#

27
synapse/config/server.py
View File

@ -489,6 +489,19 @@ class ServerConfig(Config):
# events with profile information that differ from the target's global profile.
self.allow_per_room_profiles = config.get("allow_per_room_profiles", True)
# The maximum size an avatar can have, in bytes.
self.max_avatar_size = config.get("max_avatar_size")
if self.max_avatar_size is not None:
self.max_avatar_size = self.parse_size(self.max_avatar_size)
# The MIME types allowed for an avatar.
self.allowed_avatar_mimetypes = config.get("allowed_avatar_mimetypes")
if self.allowed_avatar_mimetypes and not isinstance(
self.allowed_avatar_mimetypes,
list,
):
raise ConfigError("allowed_avatar_mimetypes must be a list")
self.listeners = [parse_listener_def(x) for x in config.get("listeners", [])]
# no_tls is not really supported any more, but let's grandfather it in
@ -1168,6 +1181,20 @@ class ServerConfig(Config):
#
#allow_per_room_profiles: false
# The largest allowed file size for a user avatar. Defaults to no restriction.
#
# Note that user avatar changes will not work if this is set without
# using Synapse's media repository.
#
#max_avatar_size: 10M
# The MIME types allowed for user avatars. Defaults to no restriction.
#
# Note that user avatar changes will not work if this is set without
# using Synapse's media repository.
#
#allowed_avatar_mimetypes: ["image/png", "image/jpeg", "image/gif"]
# How long to keep redacted events in unredacted form in the database. After
# this period redacted events get replaced with their redacted form in the DB.
#

67
synapse/handlers/profile.py
View File

@ -31,6 +31,8 @@ from synapse.types import (
create_requester,
get_domain_from_id,
)
from synapse.util.caches.descriptors import cached
from synapse.util.stringutils import parse_and_validate_mxc_uri
if TYPE_CHECKING:
from synapse.server import HomeServer
@ -64,6 +66,11 @@ class ProfileHandler:
self.user_directory_handler = hs.get_user_directory_handler()
self.request_ratelimiter = hs.get_request_ratelimiter()
self.max_avatar_size = hs.config.server.max_avatar_size
self.allowed_avatar_mimetypes = hs.config.server.allowed_avatar_mimetypes
self.server_name = hs.config.server.server_name
if hs.config.worker.run_background_tasks:
self.clock.looping_call(
self._update_remote_profile_cache, self.PROFILE_UPDATE_MS
@ -286,6 +293,9 @@ class ProfileHandler:
400, "Avatar URL is too long (max %i)" % (MAX_AVATAR_URL_LEN,)
)
if not await self.check_avatar_size_and_mime_type(new_avatar_url):
raise SynapseError(403, "This avatar is not allowed", Codes.FORBIDDEN)
avatar_url_to_set: Optional[str] = new_avatar_url
if new_avatar_url == "":
avatar_url_to_set = None
@ -307,6 +317,63 @@ class ProfileHandler:
await self._update_join_states(requester, target_user)
@cached()
async def check_avatar_size_and_mime_type(self, mxc: str) -> bool:
"""Check that the size and content type of the avatar at the given MXC URI are
within the configured limits.
Args:
mxc: The MXC URI at which the avatar can be found.
Returns:
A boolean indicating whether the file can be allowed to be set as an avatar.
"""
if not self.max_avatar_size and not self.allowed_avatar_mimetypes:
return True
server_name, _, media_id = parse_and_validate_mxc_uri(mxc)
if server_name == self.server_name:
media_info = await self.store.get_local_media(media_id)
else:
media_info = await self.store.get_cached_remote_media(server_name, media_id)
if media_info is None:
# Both configuration options need to access the file's metadata, and
# retrieving remote avatars just for this becomes a bit of a faff, especially
# if e.g. the file is too big. It's also generally safe to assume most files
# used as avatar are uploaded locally, or if the upload didn't happen as part
# of a PUT request on /avatar_url that the file was at least previewed by the
# user locally (and therefore downloaded to the remote media cache).
logger.warning("Forbidding avatar change to %s: avatar not on server", mxc)
return False
if self.max_avatar_size:
# Ensure avatar does not exceed max allowed avatar size
if media_info["media_length"] > self.max_avatar_size:
logger.warning(
"Forbidding avatar change to %s: %d bytes is above the allowed size "
"limit",
mxc,
media_info["media_length"],
)
return False
if self.allowed_avatar_mimetypes:
# Ensure the avatar's file type is allowed
if (
self.allowed_avatar_mimetypes
and media_info["media_type"] not in self.allowed_avatar_mimetypes
):
logger.warning(
"Forbidding avatar change to %s: mimetype %s not allowed",
mxc,
media_info["media_type"],
)
return False
return True
async def on_profile_query(self, args: JsonDict) -> JsonDict:
"""Handles federation profile query requests."""

6
synapse/handlers/room_member.py
View File

@ -590,6 +590,12 @@ class RoomMemberHandler(metaclass=abc.ABCMeta):
errcode=Codes.BAD_JSON,
)
if "avatar_url" in content:
if not await self.profile_handler.check_avatar_size_and_mime_type(
content["avatar_url"],
):
raise SynapseError(403, "This avatar is not allowed", Codes.FORBIDDEN)
# The event content should *not* include the authorising user as
# it won't be properly signed. Strip it out since it might come
# back from a client updating a display name / avatar.

94
tests/handlers/test_profile.py
View File

@ -11,12 +11,13 @@
# 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 typing import Any, Dict
from unittest.mock import Mock
import synapse.types
from synapse.api.errors import AuthError, SynapseError
from synapse.rest import admin
from synapse.server import HomeServer
from synapse.types import UserID
from tests import unittest
@ -46,7 +47,7 @@ class ProfileTestCase(unittest.HomeserverTestCase):
)
return hs
def prepare(self, reactor, clock, hs):
def prepare(self, reactor, clock, hs: HomeServer):
self.store = hs.get_datastore()
self.frank = UserID.from_string("@1234abcd:test")
@ -248,3 +249,92 @@ class ProfileTestCase(unittest.HomeserverTestCase):
),
SynapseError,
)
def test_avatar_constraints_no_config(self):
"""Tests that the method to check an avatar against configured constraints skips
all of its check if no constraint is configured.
"""
# The first check that's done by this method is whether the file exists; if we
# don't get an error on a non-existing file then it means all of the checks were
# successfully skipped.
res = self.get_success(
self.handler.check_avatar_size_and_mime_type("mxc://test/unknown_file")
)
self.assertTrue(res)
@unittest.override_config({"max_avatar_size": 50})
def test_avatar_constraints_missing(self):
"""Tests that an avatar isn't allowed if the file at the given MXC URI couldn't
be found.
"""
res = self.get_success(
self.handler.check_avatar_size_and_mime_type("mxc://test/unknown_file")
)
self.assertFalse(res)
@unittest.override_config({"max_avatar_size": 50})
def test_avatar_constraints_file_size(self):
"""Tests that a file that's above the allowed file size is forbidden but one
that's below it is allowed.
"""
self._setup_local_files(
{
"small": {"size": 40},
"big": {"size": 60},
}
)
res = self.get_success(
self.handler.check_avatar_size_and_mime_type("mxc://test/small")
)
self.assertTrue(res)
res = self.get_success(
self.handler.check_avatar_size_and_mime_type("mxc://test/big")
)
self.assertFalse(res)
@unittest.override_config({"allowed_avatar_mimetypes": ["image/png"]})
def test_avatar_constraint_mime_type(self):
"""Tests that a file with an unauthorised MIME type is forbidden but one with
an authorised content type is allowed.
"""
self._setup_local_files(
{
"good": {"mimetype": "image/png"},
"bad": {"mimetype": "application/octet-stream"},
}
)
res = self.get_success(
self.handler.check_avatar_size_and_mime_type("mxc://test/good")
)
self.assertTrue(res)
res = self.get_success(
self.handler.check_avatar_size_and_mime_type("mxc://test/bad")
)
self.assertFalse(res)
def _setup_local_files(self, names_and_props: Dict[str, Dict[str, Any]]):
"""Stores metadata about files in the database.
Args:
names_and_props: A dictionary with one entry per file, with the key being the
file's name, and the value being a dictionary of properties. Supported
properties are "mimetype" (for the file's type) and "size" (for the
file's size).
"""
store = self.hs.get_datastore()
for name, props in names_and_props.items():
self.get_success(
store.store_local_media(
media_id=name,
media_type=props.get("mimetype", "image/png"),
time_now_ms=self.clock.time_msec(),
upload_name=None,
media_length=props.get("size", 50),
user_id=UserID.from_string("@rin:test"),
)
)

156
tests/rest/client/test_profile.py
View File

@ -13,8 +13,12 @@
# limitations under the License.
"""Tests REST events for /profile paths."""
from typing import Any, Dict
from synapse.api.errors import Codes
from synapse.rest import admin
from synapse.rest.client import login, profile, room
from synapse.types import UserID
from tests import unittest
@ -25,6 +29,7 @@ class ProfileTestCase(unittest.HomeserverTestCase):
admin.register_servlets_for_client_rest_resource,
login.register_servlets,
profile.register_servlets,
room.register_servlets,
]
def make_homeserver(self, reactor, clock):
@ -150,6 +155,157 @@ class ProfileTestCase(unittest.HomeserverTestCase):
self.assertEqual(channel.code, 200, channel.result)
return channel.json_body.get("avatar_url")
@unittest.override_config({"max_avatar_size": 50})
def test_avatar_size_limit_global(self):
"""Tests that the maximum size limit for avatars is enforced when updating a
global profile.
"""
self._setup_local_files(
{
"small": {"size": 40},
"big": {"size": 60},
}
)
channel = self.make_request(
"PUT",
f"/profile/{self.owner}/avatar_url",
content={"avatar_url": "mxc://test/big"},
access_token=self.owner_tok,
)
self.assertEqual(channel.code, 403, channel.result)
self.assertEqual(
channel.json_body["errcode"], Codes.FORBIDDEN, channel.json_body
)
channel = self.make_request(
"PUT",
f"/profile/{self.owner}/avatar_url",
content={"avatar_url": "mxc://test/small"},
access_token=self.owner_tok,
)
self.assertEqual(channel.code, 200, channel.result)
@unittest.override_config({"max_avatar_size": 50})
def test_avatar_size_limit_per_room(self):
"""Tests that the maximum size limit for avatars is enforced when updating a
per-room profile.
"""
self._setup_local_files(
{
"small": {"size": 40},
"big": {"size": 60},
}
)
room_id = self.helper.create_room_as(tok=self.owner_tok)
channel = self.make_request(
"PUT",
f"/rooms/{room_id}/state/m.room.member/{self.owner}",
content={"membership": "join", "avatar_url": "mxc://test/big"},
access_token=self.owner_tok,
)
self.assertEqual(channel.code, 403, channel.result)
self.assertEqual(
channel.json_body["errcode"], Codes.FORBIDDEN, channel.json_body
)
channel = self.make_request(
"PUT",
f"/rooms/{room_id}/state/m.room.member/{self.owner}",
content={"membership": "join", "avatar_url": "mxc://test/small"},
access_token=self.owner_tok,
)
self.assertEqual(channel.code, 200, channel.result)
@unittest.override_config({"allowed_avatar_mimetypes": ["image/png"]})
def test_avatar_allowed_mime_type_global(self):
"""Tests that the MIME type whitelist for avatars is enforced when updating a
global profile.
"""
self._setup_local_files(
{
"good": {"mimetype": "image/png"},
"bad": {"mimetype": "application/octet-stream"},
}
)
channel = self.make_request(
"PUT",
f"/profile/{self.owner}/avatar_url",
content={"avatar_url": "mxc://test/bad"},
access_token=self.owner_tok,
)
self.assertEqual(channel.code, 403, channel.result)
self.assertEqual(
channel.json_body["errcode"], Codes.FORBIDDEN, channel.json_body
)
channel = self.make_request(
"PUT",
f"/profile/{self.owner}/avatar_url",
content={"avatar_url": "mxc://test/good"},
access_token=self.owner_tok,
)
self.assertEqual(channel.code, 200, channel.result)
@unittest.override_config({"allowed_avatar_mimetypes": ["image/png"]})
def test_avatar_allowed_mime_type_per_room(self):
"""Tests that the MIME type whitelist for avatars is enforced when updating a
per-room profile.
"""
self._setup_local_files(
{
"good": {"mimetype": "image/png"},
"bad": {"mimetype": "application/octet-stream"},
}
)
room_id = self.helper.create_room_as(tok=self.owner_tok)
channel = self.make_request(
"PUT",
f"/rooms/{room_id}/state/m.room.member/{self.owner}",
content={"membership": "join", "avatar_url": "mxc://test/bad"},
access_token=self.owner_tok,
)
self.assertEqual(channel.code, 403, channel.result)
self.assertEqual(
channel.json_body["errcode"], Codes.FORBIDDEN, channel.json_body
)
channel = self.make_request(
"PUT",
f"/rooms/{room_id}/state/m.room.member/{self.owner}",
content={"membership": "join", "avatar_url": "mxc://test/good"},
access_token=self.owner_tok,
)
self.assertEqual(channel.code, 200, channel.result)
def _setup_local_files(self, names_and_props: Dict[str, Dict[str, Any]]):
"""Stores metadata about files in the database.
Args:
names_and_props: A dictionary with one entry per file, with the key being the
file's name, and the value being a dictionary of properties. Supported
properties are "mimetype" (for the file's type) and "size" (for the
file's size).
"""
store = self.hs.get_datastore()
for name, props in names_and_props.items():
self.get_success(
store.store_local_media(
media_id=name,
media_type=props.get("mimetype", "image/png"),
time_now_ms=self.clock.time_msec(),
upload_name=None,
media_length=props.get("size", 50),
user_id=UserID.from_string("@rin:test"),
)
)
class ProfilesRestrictedTestCase(unittest.HomeserverTestCase):