mirror of
https://github.com/element-hq/synapse.git
synced 2026-07-29 20:40:34 +00:00
Fix some paths being accessible at unintended locations with extra prefix components
Fixes: https://github.com/element-hq/synapse/security/advisories/GHSA-vh4c-pqh4-w3wq Fixes: https://github.com/matrix-org/internal-config/issues/1703 The key thing to understand is that in `synapse/util/httpresourcetree.py`, we create `UnrecognizedRequestResource` and then dangle children (with real resources) off them. Since `UnrecognizedRequestResource` returns itself as a catch-all 'dynamic child', this means any `UnrecognizedRequestResource`s with real children can have unlimited path components inserted between it and its child. So `/_matrix/INSERTED/static/client/login/style.css` or `/_matrix/INSERTED/AS/MANY/AS/I/WANT/static/client/login/style.css` would unexpectedly resolve to the resource. Client, Federation and Admin APIs wouldn't have been affected because you wouldn't get through the regex routing that they use. ----- Reviewed-on: https://github.com/element-hq/synapse-private/pull/143
This commit is contained in:
committed by
Olivier 'reivilibre
parent
e860184067
commit
e39303af40
+28
-2
@@ -33,6 +33,7 @@ from typing import (
|
||||
Any,
|
||||
Awaitable,
|
||||
Callable,
|
||||
Final,
|
||||
Iterable,
|
||||
Iterator,
|
||||
Pattern,
|
||||
@@ -673,8 +674,33 @@ class UnrecognizedRequestResource(resource.Resource):
|
||||
# or the response bytes as a return value.
|
||||
return NOT_DONE_YET
|
||||
|
||||
def getChild(self, name: str, request: Request) -> resource.Resource:
|
||||
return self
|
||||
def getChild(self, path: str, request: Request) -> resource.Resource:
|
||||
# The child of a catch-all unrecognised request handler
|
||||
# is itself another unrecognised request handler.
|
||||
# We can return any UnrecognizedRequestResource that doesn't
|
||||
# have children.
|
||||
assert len(_BLANK_LEAF_UNRECOGNISED_REQUEST_RESOURCE.children) == 0
|
||||
return _BLANK_LEAF_UNRECOGNISED_REQUEST_RESOURCE
|
||||
|
||||
|
||||
class _LeafUnrecognisedRequestResource(UnrecognizedRequestResource):
|
||||
"""
|
||||
UnrecognizedRequestResource, but with the added caveat that it can't have any children.
|
||||
This makes it safe for it to return itself as a dynamic child.
|
||||
|
||||
Constructed as a singleton; use `_BLANK_LEAF_UNRECOGNISED_REQUEST_RESOURCE`
|
||||
"""
|
||||
|
||||
def putChild(self, path: bytes, child: IResource) -> None:
|
||||
raise RuntimeError("_LeafUnrecognisedRequestResource does not accept children")
|
||||
|
||||
|
||||
_BLANK_LEAF_UNRECOGNISED_REQUEST_RESOURCE: Final[_LeafUnrecognisedRequestResource] = (
|
||||
_LeafUnrecognisedRequestResource()
|
||||
)
|
||||
"""
|
||||
An UnrecognizedRequestResource that is guaranteed not to have children.
|
||||
"""
|
||||
|
||||
|
||||
class RootRedirect(resource.Resource):
|
||||
|
||||
@@ -0,0 +1,66 @@
|
||||
#
|
||||
# This file is licensed under the Affero General Public License (AGPL) version 3.
|
||||
#
|
||||
# Copyright (C) 2026 Element Creations Ltd
|
||||
#
|
||||
# This program is free software: you can redistribute it and/or modify
|
||||
# it under the terms of the GNU Affero General Public License as
|
||||
# published by the Free Software Foundation, either version 3 of the
|
||||
# License, or (at your option) any later version.
|
||||
#
|
||||
# See the GNU Affero General Public License for more details:
|
||||
# <https://www.gnu.org/licenses/agpl-3.0.html>.
|
||||
#
|
||||
import os
|
||||
from http import HTTPStatus
|
||||
|
||||
from twisted.web.resource import Resource
|
||||
|
||||
import synapse
|
||||
from synapse.api.errors import Codes
|
||||
from synapse.api.urls import STATIC_PREFIX
|
||||
from synapse.http.server import StaticResource
|
||||
|
||||
from tests import unittest
|
||||
|
||||
|
||||
class ResourceTreeTestCase(unittest.HomeserverTestCase):
|
||||
servlets = []
|
||||
|
||||
def create_resource_dict(self) -> dict[str, Resource]:
|
||||
"""
|
||||
Register /_matrix/static for the test.
|
||||
"""
|
||||
resources = super().create_resource_dict()
|
||||
resources[STATIC_PREFIX] = StaticResource(
|
||||
# as in `synapse/app/homeserver.py` `_configure_named_resource`
|
||||
os.path.join(os.path.dirname(synapse.__file__), "static")
|
||||
)
|
||||
return resources
|
||||
|
||||
def test_inserted_segment_is_silently_swallowed(self) -> None:
|
||||
"""
|
||||
Regression test for https://github.com/element-hq/synapse/security/advisories/GHSA-vh4c-pqh4-w3wq
|
||||
|
||||
The path `/_matrix/INSERTED/static/client/login/style.css` used to resolve to the same
|
||||
as `/_matrix/static/client/login/style.css`.
|
||||
"""
|
||||
PATH_SUFFIX = "/static/client/login/style.css"
|
||||
correct_channel = self.make_request(
|
||||
"GET",
|
||||
f"/_matrix{PATH_SUFFIX}",
|
||||
shorthand=False,
|
||||
)
|
||||
# The correct path should give a 200 OK static resource
|
||||
self.assertEqual(correct_channel.code, HTTPStatus.OK, correct_channel.result)
|
||||
|
||||
wrong_channel = self.make_request(
|
||||
"GET",
|
||||
f"/_matrix/INSERTED{PATH_SUFFIX}",
|
||||
shorthand=False,
|
||||
)
|
||||
# This prefixed version of the same path should give a 404
|
||||
self.assertEqual(wrong_channel.code, HTTPStatus.NOT_FOUND, wrong_channel.result)
|
||||
self.assertEqual(
|
||||
wrong_channel.json_body["errcode"], Codes.UNRECOGNIZED, wrong_channel.result
|
||||
)
|
||||
Reference in New Issue
Block a user