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

Fix inconsistent handling of upper and lower cases of email addresses. #7021

Merged
merged 15 commits into from
Jul 3, 2020
Merged
Show file tree
Hide file tree
Changes from 14 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/7021.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix inconsistent handling of upper and lower case in email addresses when used as identifiers for login, etc. Contributed by @dklimpel.
5 changes: 3 additions & 2 deletions synapse/handlers/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
from synapse.module_api import ModuleApi
from synapse.push.mailer import load_jinja2_templates
from synapse.types import Requester, UserID
from synapse.util.threepids import canonicalise_email

from ._base import BaseHandler

Expand Down Expand Up @@ -928,7 +929,7 @@ async def add_threepid(
# for the presence of an email address during password reset was
# case sensitive).
if medium == "email":
address = address.lower()
address = canonicalise_email(address)

await self.store.user_add_threepid(
user_id, medium, address, validated_at, self.hs.get_clock().time_msec()
Expand Down Expand Up @@ -956,7 +957,7 @@ async def delete_threepid(

# 'Canonicalise' email addresses as per above
if medium == "email":
address = address.lower()
address = canonicalise_email(address)

identity_handler = self.hs.get_handlers().identity_handler
result = await identity_handler.try_unbind_threepid(
Expand Down
12 changes: 8 additions & 4 deletions synapse/rest/client/v1/login.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
from synapse.rest.well_known import WellKnownBuilder
from synapse.types import UserID
from synapse.util.msisdn import phone_number_to_msisdn
from synapse.util.threepids import canonicalise_email

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -203,11 +204,14 @@ async def _do_other_login(self, login_submission):
if medium is None or address is None:
raise SynapseError(400, "Invalid thirdparty identifier")

# For emails, canonicalise the address.
# We store all email addresses canonicalised in the DB.
# (See add_threepid in synapse/handlers/auth.py)
if medium == "email":
# For emails, transform the address to lowercase.
# We store all email addreses as lowercase in the DB.
# (See add_threepid in synapse/handlers/auth.py)
address = address.lower()
try:
address = canonicalise_email(address)
except ValueError as e:
raise SynapseError(400, str(e))

# We also apply account rate limiting using the 3PID as a key, as
# otherwise using 3PID bypasses the ratelimiting based on user ID.
Expand Down
40 changes: 31 additions & 9 deletions synapse/rest/client/v2_alpha/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
from synapse.push.mailer import Mailer, load_jinja2_templates
from synapse.util.msisdn import phone_number_to_msisdn
from synapse.util.stringutils import assert_valid_client_secret, random_string
from synapse.util.threepids import check_3pid_allowed
from synapse.util.threepids import canonicalise_email, check_3pid_allowed

from ._base import client_patterns, interactive_auth_handler

Expand Down Expand Up @@ -83,7 +83,15 @@ async def on_POST(self, request):
client_secret = body["client_secret"]
assert_valid_client_secret(client_secret)

email = body["email"]
# Canonicalise the email address. The addresses are all stored canonicalised
# in the database. This allows the user to reset his password without having to
# know the exact spelling (eg. upper and lower case) of address in the database.
# Stored in the database "foo@bar.com"
# User requests with "FOO@bar.com" would raise a Not Found error
try:
email = canonicalise_email(body["email"])
except ValueError as e:
raise SynapseError(400, str(e))
send_attempt = body["send_attempt"]
next_link = body.get("next_link") # Optional param

Expand All @@ -94,6 +102,10 @@ async def on_POST(self, request):
Codes.THREEPID_DENIED,
)

# 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
# canonicalisation, matches the one in our database.
existing_user_id = await self.hs.get_datastore().get_user_id_by_threepid(
"email", email
)
Expand Down Expand Up @@ -275,10 +287,13 @@ async def on_POST(self, request):
if "medium" not in threepid or "address" not in threepid:
raise SynapseError(500, "Malformed threepid")
if threepid["medium"] == "email":
# For emails, transform the address to lowercase.
# We store all email addreses as lowercase in the DB.
# For emails, canonicalise the address.
# We store all email addresses canonicalised in the DB.
# (See add_threepid in synapse/handlers/auth.py)
threepid["address"] = threepid["address"].lower()
try:
threepid["address"] = canonicalise_email(threepid["address"])
except ValueError as e:
raise SynapseError(400, str(e))
# if using email, we must know about the email they're authing with!
threepid_user_id = await self.datastore.get_user_id_by_threepid(
threepid["medium"], threepid["address"]
Expand Down Expand Up @@ -393,7 +408,16 @@ async def on_POST(self, request):
client_secret = body["client_secret"]
assert_valid_client_secret(client_secret)

email = body["email"]
# Canonicalise the email address. The addresses are all stored canonicalised
# in the database.
# This ensures that the validation email is sent to the canonicalised address
# as it will later be entered into the database.
# Otherwise the email will be sent to "FOO@bar.com" and stored as
# "foo@bar.com" in database.
try:
email = canonicalise_email(body["email"])
except ValueError as e:
raise SynapseError(400, str(e))
send_attempt = body["send_attempt"]
next_link = body.get("next_link") # Optional param

Expand All @@ -404,9 +428,7 @@ async def on_POST(self, request):
Codes.THREEPID_DENIED,
)

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

if existing_user_id is not None:
if self.config.request_token_inhibit_3pid_errors:
Expand Down
22 changes: 19 additions & 3 deletions synapse/rest/client/v2_alpha/register.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
from synapse.util.msisdn import phone_number_to_msisdn
from synapse.util.ratelimitutils import FederationRateLimiter
from synapse.util.stringutils import assert_valid_client_secret, random_string
from synapse.util.threepids import check_3pid_allowed
from synapse.util.threepids import canonicalise_email, check_3pid_allowed

from ._base import client_patterns, interactive_auth_handler

Expand Down Expand Up @@ -116,7 +116,14 @@ async def on_POST(self, request):
client_secret = body["client_secret"]
assert_valid_client_secret(client_secret)

email = body["email"]
# For emails, canonicalise the address.
# We store all email addresses canonicalised in the DB.
# (See on_POST in EmailThreepidRequestTokenRestServlet
# in synapse/rest/client/v2_alpha/account.py)
try:
email = canonicalise_email(body["email"])
except ValueError as e:
raise SynapseError(400, str(e))
send_attempt = body["send_attempt"]
next_link = body.get("next_link") # Optional param

Expand All @@ -128,7 +135,7 @@ async def on_POST(self, request):
)

existing_user_id = await self.hs.get_datastore().get_user_id_by_threepid(
"email", body["email"]
"email", email
)

if existing_user_id is not None:
Expand Down Expand Up @@ -554,6 +561,15 @@ async def on_POST(self, request):
if login_type in auth_result:
medium = auth_result[login_type]["medium"]
address = auth_result[login_type]["address"]
# For emails, canonicalise the address.
# We store all email addresses canonicalised in the DB.
# (See on_POST in EmailThreepidRequestTokenRestServlet
# in synapse/rest/client/v2_alpha/account.py)
if medium == "email":
try:
address = canonicalise_email(address)
except ValueError as e:
raise SynapseError(400, str(e))

existing_user_id = await self.store.get_user_id_by_threepid(
medium, address
Expand Down
24 changes: 24 additions & 0 deletions synapse/util/threepids.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,27 @@ def check_3pid_allowed(hs, medium, address):
return True

return False


def canonicalise_email(address: str) -> str:
"""'Canonicalise' email address
Case folding of local part of email address and lowercase domain part
See MSC2265, https://github.com/matrix-org/matrix-doc/pull/2265

Args:
address: email address to be canonicalised
Returns:
The canonical form of the email address
Raises:
ValueError if the address could not be parsed.
"""

address = address.strip()

if len(address.split("@")) != 2:
logger.debug("Couldn't parse email address %s", address)
raise ValueError("Unable to parse email address")

# double check, split starting from the right and only at first `@`
address = address.rsplit("@", 1)
return address[0].casefold() + "@" + address[1].lower()
richvdh marked this conversation as resolved.
Show resolved Hide resolved
Loading