Limit length of accepted email addresses (#9855)

changelog.d/9855.misc

@@ -0,0 +1 @@
+Limit length of accepted email addresses.

synapse/push/emailpusher.py

@@ -19,8 +19,9 @@
 from twisted.internet.interfaces import IDelayedCall
 
 from synapse.metrics.background_process_metrics import run_as_background_process
-from synapse.push import Pusher, PusherConfig, ThrottleParams
+from synapse.push import Pusher, PusherConfig, PusherConfigException, ThrottleParams
 from synapse.push.mailer import Mailer
+from synapse.util.threepids import validate_email
 
 if TYPE_CHECKING:
     from synapse.server import HomeServer
@@ -71,6 +72,12 @@ def __init__(self, hs: "HomeServer", pusher_config: PusherConfig, mailer: Mailer
 
         self._is_processing = False
 
+        # Make sure that the email is valid.
+        try:
+            validate_email(self.email)
+        except ValueError:
+            raise PusherConfigException("Invalid email")
+
     def on_started(self, should_check_for_notifs: bool) -> None:
         """Called when this pusher has been started.
 

synapse/rest/client/v2_alpha/account.py

@@ -39,7 +39,7 @@
 from synapse.push.mailer import Mailer
 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 canonicalise_email, check_3pid_allowed
+from synapse.util.threepids import check_3pid_allowed, validate_email
 
 from ._base import client_patterns, interactive_auth_handler
 
@@ -92,7 +92,7 @@ async def on_POST(self, request):
         # 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"])
+            email = validate_email(body["email"])
         except ValueError as e:
             raise SynapseError(400, str(e))
         send_attempt = body["send_attempt"]
@@ -247,7 +247,7 @@ async def on_POST(self, request):
                     # We store all email addresses canonicalised in the DB.
                     # (See add_threepid in synapse/handlers/auth.py)
                     try:
-                        threepid["address"] = canonicalise_email(threepid["address"])
+                        threepid["address"] = validate_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!
@@ -375,7 +375,7 @@ async def on_POST(self, request):
         # Otherwise the email will be sent to "FOO@bar.com" and stored as
         # "foo@bar.com" in database.
         try:
-            email = canonicalise_email(body["email"])
+            email = validate_email(body["email"])
         except ValueError as e:
             raise SynapseError(400, str(e))
         send_attempt = body["send_attempt"]

synapse/rest/client/v2_alpha/register.py

@@ -49,7 +49,11 @@
 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 canonicalise_email, check_3pid_allowed
+from synapse.util.threepids import (
+    canonicalise_email,
+    check_3pid_allowed,
+    validate_email,
+)
 
 from ._base import client_patterns, interactive_auth_handler
 
@@ -111,7 +115,7 @@ async def on_POST(self, request):
         # (See on_POST in EmailThreepidRequestTokenRestServlet
         # in synapse/rest/client/v2_alpha/account.py)
         try:
-            email = canonicalise_email(body["email"])
+            email = validate_email(body["email"])
         except ValueError as e:
             raise SynapseError(400, str(e))
         send_attempt = body["send_attempt"]

synapse/util/threepids.py

@@ -18,6 +18,16 @@
 logger = logging.getLogger(__name__)
 
 
+# it's unclear what the maximum length of an email address is. RFC3696 (as corrected
+# by errata) says:
+#    the upper limit on address lengths should normally be considered to be 254.
+#
+# In practice, mail servers appear to be more tolerant and allow 400 characters
+# or so. Let's allow 500, which should be plenty for everyone.
+#
+MAX_EMAIL_ADDRESS_LENGTH = 500
+
+
 def check_3pid_allowed(hs, medium, address):
     """Checks whether a given format of 3PID is allowed to be used on this HS
 
@@ -70,3 +80,23 @@ def canonicalise_email(address: str) -> str:
         raise ValueError("Unable to parse email address")
 
     return parts[0].casefold() + "@" + parts[1].lower()
+
+
+def validate_email(address: str) -> str:
+    """Does some basic validation on an email address.
+
+    Returns the canonicalised email, as returned by `canonicalise_email`.
+
+    Raises a ValueError if the email is invalid.
+    """
+    # First we try canonicalising in case that fails
+    address = canonicalise_email(address)
+
+    # Email addresses have to be at least 3 characters.
+    if len(address) < 3:
+        raise ValueError("Unable to parse email address")
+
+    if len(address) > MAX_EMAIL_ADDRESS_LENGTH:
+        raise ValueError("Unable to parse email address")
+
+    return address

tests/rest/client/v2_alpha/test_register.py

@@ -310,6 +310,57 @@ def test_request_token_existing_email_inhibit_error(self):
 
         self.assertIsNotNone(channel.json_body.get("sid"))
 
+    @unittest.override_config(
+        {
+            "public_baseurl": "https://test_server",
+            "email": {
+                "smtp_host": "mail_server",
+                "smtp_port": 2525,
+                "notif_from": "sender@host",
+            },
+        }
+    )
+    def test_reject_invalid_email(self):
+        """Check that bad emails are rejected"""
+
+        # Test for email with multiple @
+        channel = self.make_request(
+            "POST",
+            b"register/email/requestToken",
+            {"client_secret": "foobar", "email": "email@@email", "send_attempt": 1},
+        )
+        self.assertEquals(400, channel.code, channel.result)
+        # Check error to ensure that we're not erroring due to a bug in the test.
+        self.assertEquals(
+            channel.json_body,
+            {"errcode": "M_UNKNOWN", "error": "Unable to parse email address"},
+        )
+
+        # Test for email with no @
+        channel = self.make_request(
+            "POST",
+            b"register/email/requestToken",
+            {"client_secret": "foobar", "email": "email", "send_attempt": 1},
+        )
+        self.assertEquals(400, channel.code, channel.result)
+        self.assertEquals(
+            channel.json_body,
+            {"errcode": "M_UNKNOWN", "error": "Unable to parse email address"},
+        )
+
+        # Test for super long email
+        email = "a@" + "a" * 1000
+        channel = self.make_request(
+            "POST",
+            b"register/email/requestToken",
+            {"client_secret": "foobar", "email": email, "send_attempt": 1},
+        )
+        self.assertEquals(400, channel.code, channel.result)
+        self.assertEquals(
+            channel.json_body,
+            {"errcode": "M_UNKNOWN", "error": "Unable to parse email address"},
+        )
+
 
 class AccountValidityTestCase(unittest.HomeserverTestCase):