Validate client_secret parameter according to spec (#20)

4 files changed, 34 insertions, 1 deletions

diff --git a/changelog.d/20.bugfix b/changelog.d/20.bugfix
new file mode 100644
index 0000000000..8ba53c28f9
--- /dev/null
+++ b/changelog.d/20.bugfix
@@ -0,0 +1 @@
+Validate `client_secret` parameter against the regex provided by the C-S spec.
\ No newline at end of file
diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py
index 32770a4a95..9bf4afac66 100644
--- a/synapse/rest/client/v2_alpha/account.py
+++ b/synapse/rest/client/v2_alpha/account.py
@@ -34,7 +34,7 @@ from synapse.http.servlet import (
 )
 from synapse.types import UserID
 from synapse.util.msisdn import phone_number_to_msisdn
-from synapse.util.stringutils import random_string
+from synapse.util.stringutils import assert_valid_client_secret, random_string
 from synapse.util.threepids import check_3pid_allowed
 
 from ._base import client_patterns, interactive_auth_handler
@@ -79,6 +79,8 @@ class EmailPasswordRequestTokenRestServlet(RestServlet):
 
         # Extract params from body
         client_secret = body["client_secret"]
+        assert_valid_client_secret(client_secret)
+
         email = body["email"]
         send_attempt = body["send_attempt"]
         next_link = body.get("next_link")  # Optional param
@@ -216,6 +218,8 @@ class MsisdnPasswordRequestTokenRestServlet(RestServlet):
                 Codes.THREEPID_DENIED,
             )
 
+        assert_valid_client_secret(body["client_secret"])
+
         existingUid = yield self.datastore.get_user_id_by_threepid(
             'msisdn', msisdn
         )
@@ -257,6 +261,9 @@ class PasswordResetSubmitTokenServlet(RestServlet):
 
         sid = parse_string(request, "sid")
         client_secret = parse_string(request, "client_secret")
+
+        assert_valid_client_secret(client_secret)
+
         token = parse_string(request, "token")
 
         # Attempt to validate a 3PID sesssion
@@ -330,6 +337,8 @@ class PasswordResetSubmitTokenServlet(RestServlet):
             'sid', 'client_secret', 'token',
         ])
 
+        assert_valid_client_secret(body["client_secret"])
+
         valid, _ = yield self.datastore.validate_threepid_validation_token(
             body['sid'],
             body['client_secret'],
@@ -510,6 +519,8 @@ class EmailThreepidRequestTokenRestServlet(RestServlet):
                 Codes.THREEPID_DENIED,
             )
 
+        assert_valid_client_secret(body["client_secret"])
+
         existingUid = yield self.datastore.get_user_id_by_threepid(
             'email', body['email']
         )
@@ -547,6 +558,8 @@ class MsisdnThreepidRequestTokenRestServlet(RestServlet):
                 Codes.THREEPID_DENIED,
             )
 
+        assert_valid_client_secret(body["client_secret"])
+
         existingUid = yield self.datastore.get_user_id_by_threepid(
             'msisdn', msisdn
         )
diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py
index dade6530f4..3d5a198278 100644
--- a/synapse/rest/client/v2_alpha/register.py
+++ b/synapse/rest/client/v2_alpha/register.py
@@ -43,6 +43,7 @@ from synapse.http.servlet import (
 )
 from synapse.util.msisdn import phone_number_to_msisdn
 from synapse.util.ratelimitutils import FederationRateLimiter
+from synapse.util.stringutils import assert_valid_client_secret
 from synapse.util.threepids import check_3pid_allowed
 
 from ._base import client_patterns, interactive_auth_handler
@@ -88,6 +89,8 @@ class EmailRegisterRequestTokenRestServlet(RestServlet):
                 Codes.THREEPID_DENIED,
             )
 
+        assert_params_in_dict(body["client_secret"])
+
         existingUid = yield self.hs.get_datastore().get_user_id_by_threepid(
             'email', body['email']
         )
@@ -123,6 +126,8 @@ class MsisdnRegisterRequestTokenRestServlet(RestServlet):
 
         msisdn = phone_number_to_msisdn(body['country'], body['phone_number'])
 
+        assert_valid_client_secret(body["client_secret"])
+
         if not (yield check_3pid_allowed(self.hs, "msisdn", msisdn)):
             raise SynapseError(
                 403,
diff --git a/synapse/util/stringutils.py b/synapse/util/stringutils.py
index 69dffd8244..5fb18ee1f8 100644
--- a/synapse/util/stringutils.py
+++ b/synapse/util/stringutils.py
@@ -1,5 +1,6 @@
 # -*- coding: utf-8 -*-
 # Copyright 2014-2016 OpenMarket Ltd
+# Copyright 2020 The Matrix.org Foundation C.I.C.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -14,12 +15,15 @@
 # limitations under the License.
 
 import random
+import re
 import string
 
 import six
 from six import PY2, PY3
 from six.moves import range
 
+from synapse.api.errors import Codes, SynapseError
+
 _string_with_symbols = (
     string.digits + string.ascii_letters + ".,;:^&*-_+=#~@"
 )
@@ -29,6 +33,8 @@ _string_with_symbols = (
 # we get cryptographically-secure randoms.
 rand = random.SystemRandom()
 
+client_secret_regex = re.compile(r"^[0-9a-zA-Z.=_-]+$")
+
 
 def random_string(length):
     return ''.join(rand.choice(string.ascii_letters) for _ in range(length))
@@ -113,3 +119,11 @@ def exception_to_unicode(e):
         return msg.decode('utf-8', errors='replace')
     else:
         return msg
+
+
+def assert_valid_client_secret(client_secret):
+    """Validate that a given string matches the client_secret regex defined by the spec"""
+    if client_secret_regex.match(client_secret) is None:
+        raise SynapseError(
+            400, "Invalid client_secret parameter", errcode=Codes.INVALID_PARAM
+        )