From 8936925f561b0c352c2fa922d5097d7245aad00a Mon Sep 17 00:00:00 2001
From: Denis Kasak <denisk@element.io>
Date: Wed, 31 Mar 2021 15:42:23 +0000
Subject: [PATCH] Validate hostname in more places

---
 sydent/hs_federation/verifier.py              | 11 +++++++++++
 sydent/http/servlets/threepidunbindservlet.py | 13 +++++++++----
 sydent/threepid/bind.py                       | 14 +++++++++++++-
 3 files changed, 33 insertions(+), 5 deletions(-)

diff --git a/sydent/hs_federation/verifier.py b/sydent/hs_federation/verifier.py
index 1360672d..a708d407 100644
--- a/sydent/hs_federation/verifier.py
+++ b/sydent/hs_federation/verifier.py
@@ -25,6 +25,7 @@
 from signedjson.sign import SignatureVerifyException
 
 from sydent.http.httpclient import FederationHttpClient
+from sydent.util.stringutils import is_valid_hostname
 
 
 logger = logging.getLogger(__name__)
@@ -37,6 +38,13 @@ class NoAuthenticationError(Exception):
     pass
 
 
+class InvalidServerName(Exception):
+    """
+    Raised when the provided origin parameter is not a valid hostname (plus optional port).
+    """
+    pass
+
+
 class Verifier(object):
     """
     Verifies signed json blobs from Matrix Homeservers by finding the
@@ -197,6 +205,9 @@ def strip_quotes(value):
         if not json_request["signatures"]:
             raise NoAuthenticationError("Missing X-Matrix Authorization header")
 
+        if not is_valid_hostname(json_request["origin"]):
+            raise InvalidServerName("X-Matrix header's origin parameter must be a valid hostname")
+
         yield self.verifyServerSignedJson(json_request, [origin])
 
         logger.info("Verified request from HS %s", origin)
diff --git a/sydent/http/servlets/threepidunbindservlet.py b/sydent/http/servlets/threepidunbindservlet.py
index ddc7db48..87fc6161 100644
--- a/sydent/http/servlets/threepidunbindservlet.py
+++ b/sydent/http/servlets/threepidunbindservlet.py
@@ -19,7 +19,7 @@
 import json
 import logging
 
-from sydent.hs_federation.verifier import NoAuthenticationError
+from sydent.hs_federation.verifier import NoAuthenticationError, InvalidServerName
 from signedjson.sign import SignatureVerifyException
 
 from sydent.http.servlets import dict_to_json_bytes
@@ -81,7 +81,7 @@ def _async_render_POST(self, request):
             # and "client_secret" fields, they are trying to prove that they
             # were the original author of the bind. We then check that what
             # they supply matches and if it does, allow the unbind.
-            # 
+            #
             # However if these fields are not supplied, we instead check
             # whether the request originated from a homeserver, and if so the
             # same homeserver that originally created the bind. We do this by
@@ -121,7 +121,7 @@ def _async_render_POST(self, request):
                         'error': "This validation session has not yet been completed"
                     }))
                     return
-                
+
                 if s.medium != threepid['medium'] or s.address != threepid['address']:
                     request.setResponseCode(403)
                     request.write(dict_to_json_bytes({
@@ -143,7 +143,12 @@ def _async_render_POST(self, request):
                     request.write(dict_to_json_bytes({'errcode': 'M_FORBIDDEN', 'error': str(ex)}))
                     request.finish()
                     return
-                except:
+                except InvalidServerName as ex:
+                    request.setResponseCode(400)
+                    request.write(dict_to_json_bytes({'errcode': 'M_INVALID_PARAM', 'error': str(ex)}))
+                    request.finish()
+                    return
+                except Exception:
                     logger.exception("Exception whilst authenticating unbind request")
                     request.setResponseCode(500)
                     request.write(dict_to_json_bytes({'errcode': 'M_UNKNOWN', 'error': 'Internal Server Error'}))
diff --git a/sydent/threepid/bind.py b/sydent/threepid/bind.py
index a3317465..84160bfe 100644
--- a/sydent/threepid/bind.py
+++ b/sydent/threepid/bind.py
@@ -32,6 +32,8 @@
 
 from sydent.threepid import ThreepidAssociation
 
+from sydent.util.stringutils import is_valid_hostname
+
 from twisted.internet import defer
 
 logger = logging.getLogger(__name__)
@@ -131,6 +133,7 @@ def _notify(self, assoc, attempt):
         """
         mxid = assoc["mxid"]
         mxid_parts = mxid.split(":", 1)
+
         if len(mxid_parts) != 2:
             logger.error(
                 "Can't notify on bind for unparseable mxid %s. Not retrying.",
@@ -138,8 +141,17 @@ def _notify(self, assoc, attempt):
             )
             return
 
+        matrix_server = mxid_parts[1]
+
+        if not is_valid_hostname(matrix_server):
+            logger.error(
+                "MXID server part '%s' not a valid hostname. Not retrying.",
+                matrix_server,
+            )
+            return
+
         post_url = "matrix://%s/_matrix/federation/v1/3pid/onbind" % (
-            mxid_parts[1],
+            matrix_server,
         )
 
         logger.info("Making bind callback to: %s", post_url)