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

Add a config option for validating 'next_link' parameters against a domain whitelist #8275

Merged
merged 12 commits into from
Sep 8, 2020
18 changes: 18 additions & 0 deletions docs/sample_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,24 @@ retention:
#
#request_token_inhibit_3pid_errors: true

# A list of domains that the domain portion of 'next_link' parameters
# must match.
#
# This parameter is optionally provided by clients while requesting
# validation of an email or phone number, and maps to a link that
# users will be automatically redirected to after validation
# succeeds. Clients can make use this parameter to aid the validation
# process.
#
# The whitelist is applied whether the homeserver or an
# account_threepid_delegate is handling validation.
#
# The default value is no whitelist functionality; all domains are
# allowed. Setting this value to an empty list will instead disallow
# all domains.
#
#next_link_domain_whitelist: ["matrix.org"]


## TLS ##

Expand Down
26 changes: 26 additions & 0 deletions synapse/config/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,14 @@ class LimitRemoteRoomsConfig:
users_new_default_push_rules
) # type: set

# Whitelist of domain names that given next_link parameters must have
next_link_domain_whitelist = config.get("next_link_domain_whitelist")
if not isinstance(next_link_domain_whitelist, list):
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved
raise ConfigError("'next_link_domain_whitelist' must be a list")

# Turn the list into a set to improve lookup speed.
self.next_link_domain_whitelist = set(next_link_domain_whitelist) # type: set

def has_tls_listener(self) -> bool:
return any(listener.tls for listener in self.listeners)

Expand Down Expand Up @@ -1014,6 +1022,24 @@ def generate_config_section(
# act as if no error happened and return a fake session ID ('sid') to clients.
#
#request_token_inhibit_3pid_errors: true

# A list of domains that the domain portion of 'next_link' parameters
# must match.
#
# This parameter is optionally provided by clients while requesting
# validation of an email or phone number, and maps to a link that
# users will be automatically redirected to after validation
# succeeds. Clients can make use this parameter to aid the validation
# process.
#
# The whitelist is applied whether the homeserver or an
# account_threepid_delegate is handling validation.
babolivier marked this conversation as resolved.
Show resolved Hide resolved
#
# The default value is no whitelist functionality; all domains are
# allowed. Setting this value to an empty list will instead disallow
# all domains.
#
#next_link_domain_whitelist: ["matrix.org"]
"""
% locals()
)
Expand Down
66 changes: 57 additions & 9 deletions synapse/rest/client/v2_alpha/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@
import logging
import random
from http import HTTPStatus
from typing import TYPE_CHECKING
from urllib.parse import urlparse

if TYPE_CHECKING:
from synapse.app.homeserver import HomeServer

from synapse.api.constants import LoginType
from synapse.api.errors import (
Expand Down Expand Up @@ -98,6 +103,9 @@ async def on_POST(self, request):
Codes.THREEPID_DENIED,
)

# Raise if the provided next_link value isn't valid
assert_valid_next_link(self.hs, next_link)

# The email will be sent to the stored address.
# This avoids a potential account hijack by requesting a password reset to
# an email address which is controlled by the attacker but which, after
Expand Down Expand Up @@ -446,6 +454,9 @@ async def on_POST(self, request):
Codes.THREEPID_DENIED,
)

# Raise if the provided next_link value isn't valid
assert_valid_next_link(self.hs, next_link)

existing_user_id = await self.store.get_user_id_by_threepid("email", email)

if existing_user_id is not None:
Expand Down Expand Up @@ -517,6 +528,9 @@ async def on_POST(self, request):
Codes.THREEPID_DENIED,
)

# Raise if the provided next_link value isn't valid
assert_valid_next_link(self.hs, next_link)

existing_user_id = await self.store.get_user_id_by_threepid("msisdn", msisdn)

if existing_user_id is not None:
Expand Down Expand Up @@ -603,15 +617,10 @@ async def on_GET(self, request):

# Perform a 302 redirect if next_link is set
if next_link:
if next_link.startswith("file:///"):
logger.warning(
"Not redirecting to next_link as it is a local file: address"
)
Copy link
Member Author

Choose a reason for hiding this comment

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

This check now happens implicitly in assert_valid_next_link, as only http and https next_link values are allowed.

Additionally, next_link can only be specified during the call to /requestToken. Having this check in /submit_token, blocking the user after already approving their /requestToken call, is not great UX.

else:
request.setResponseCode(302)
request.setHeader("Location", next_link)
finish_request(request)
return None
request.setResponseCode(302)
request.setHeader("Location", next_link)
finish_request(request)
return None

# Otherwise show the success template
html = self.config.email_add_threepid_template_success_html_content
Expand Down Expand Up @@ -875,6 +884,45 @@ async def on_POST(self, request):
return 200, {"id_server_unbind_result": id_server_unbind_result}


def assert_valid_next_link(hs: "HomeServer", next_link: str):
"""
Raises a SynapseError if a given next_link value is invalid

next_link is valid if the scheme is http(s) and the next_link.domain_whitelist config
option is either empty or contains a domain that matches the one in the given next_link

Args:
hs: The homeserver object
next_link: The next_link value given by the client

Raises:
SynapseError: If the next_link is invalid
"""
valid = True

# Parse the contents of the URL
next_link_parsed = urlparse(next_link)

# Scheme must be http(s)
if next_link_parsed.scheme not in ["http", "https"]:
valid = False
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved

# If the domain whitelist is set, the domain must be in it
if (
valid
and hs.config.next_link_domain_whitelist is not None
and next_link_parsed.hostname not in hs.config.next_link_domain_whitelist
):
valid = False

if not valid:
raise SynapseError(
400,
"'next_link' domain not included in whitelist, or not http(s)",
errcode=Codes.INVALID_PARAM,
)


class WhoamiRestServlet(RestServlet):
PATTERNS = client_patterns("/account/whoami$")

Expand Down