From 38bed30b411d8e438d430eae2670482eb2778628 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Fri, 26 Aug 2022 09:56:41 +0100 Subject: [PATCH 1/2] Restrict username length properly (#2676) This fixes #2674 by fixing the username check to comply with the [spec appendices](https://spec.matrix.org/v1.3/appendices/#user-identifiers): > The length of a user ID, including the @ sigil and the domain, MUST NOT exceed 255 characters. --- clientapi/routing/register.go | 30 +++++++++++++++--------------- clientapi/routing/routing.go | 2 +- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/clientapi/routing/register.go b/clientapi/routing/register.go index af0329a48..0bda1e488 100644 --- a/clientapi/routing/register.go +++ b/clientapi/routing/register.go @@ -276,19 +276,19 @@ type recaptchaResponse struct { } // validateUsername returns an error response if the username is invalid -func validateUsername(username string) *util.JSONResponse { +func validateUsername(localpart string, domain gomatrixserverlib.ServerName) *util.JSONResponse { // https://github.com/matrix-org/synapse/blob/v0.20.0/synapse/rest/client/v2_alpha/register.py#L161 - if len(username) > maxUsernameLength { + if id := fmt.Sprintf("@%s:%s", localpart, domain); len(id) > maxUsernameLength { return &util.JSONResponse{ Code: http.StatusBadRequest, - JSON: jsonerror.BadJSON(fmt.Sprintf("'username' >%d characters", maxUsernameLength)), + JSON: jsonerror.BadJSON(fmt.Sprintf("%q exceeds the maximum length of %d characters", id, maxUsernameLength)), } - } else if !validUsernameRegex.MatchString(username) { + } else if !validUsernameRegex.MatchString(localpart) { return &util.JSONResponse{ Code: http.StatusBadRequest, JSON: jsonerror.InvalidUsername("Username can only contain characters a-z, 0-9, or '_-./='"), } - } else if username[0] == '_' { // Regex checks its not a zero length string + } else if localpart[0] == '_' { // Regex checks its not a zero length string return &util.JSONResponse{ Code: http.StatusBadRequest, JSON: jsonerror.InvalidUsername("Username cannot start with a '_'"), @@ -298,13 +298,13 @@ func validateUsername(username string) *util.JSONResponse { } // validateApplicationServiceUsername returns an error response if the username is invalid for an application service -func validateApplicationServiceUsername(username string) *util.JSONResponse { - if len(username) > maxUsernameLength { +func validateApplicationServiceUsername(localpart string, domain gomatrixserverlib.ServerName) *util.JSONResponse { + if id := fmt.Sprintf("@%s:%s", localpart, domain); len(id) > maxUsernameLength { return &util.JSONResponse{ Code: http.StatusBadRequest, - JSON: jsonerror.BadJSON(fmt.Sprintf("'username' >%d characters", maxUsernameLength)), + JSON: jsonerror.BadJSON(fmt.Sprintf("%q exceeds the maximum length of %d characters", id, maxUsernameLength)), } - } else if !validUsernameRegex.MatchString(username) { + } else if !validUsernameRegex.MatchString(localpart) { return &util.JSONResponse{ Code: http.StatusBadRequest, JSON: jsonerror.InvalidUsername("Username can only contain characters a-z, 0-9, or '_-./='"), @@ -523,7 +523,7 @@ func validateApplicationService( } // Check username application service is trying to register is valid - if err := validateApplicationServiceUsername(username); err != nil { + if err := validateApplicationServiceUsername(username, cfg.Matrix.ServerName); err != nil { return "", err } @@ -604,7 +604,7 @@ func Register( case r.Type == authtypes.LoginTypeApplicationService && accessTokenErr == nil: // Spec-compliant case (the access_token is specified and the login type // is correctly set, so it's an appservice registration) - if resErr := validateApplicationServiceUsername(r.Username); resErr != nil { + if resErr := validateApplicationServiceUsername(r.Username, cfg.Matrix.ServerName); resErr != nil { return *resErr } case accessTokenErr == nil: @@ -617,7 +617,7 @@ func Register( default: // Spec-compliant case (neither the access_token nor the login type are // specified, so it's a normal user registration) - if resErr := validateUsername(r.Username); resErr != nil { + if resErr := validateUsername(r.Username, cfg.Matrix.ServerName); resErr != nil { return *resErr } } @@ -1018,7 +1018,7 @@ func RegisterAvailable( // Squash username to all lowercase letters username = strings.ToLower(username) - if err := validateUsername(username); err != nil { + if err := validateUsername(username, cfg.Matrix.ServerName); err != nil { return *err } @@ -1059,7 +1059,7 @@ func RegisterAvailable( } } -func handleSharedSecretRegistration(userAPI userapi.ClientUserAPI, sr *SharedSecretRegistration, req *http.Request) util.JSONResponse { +func handleSharedSecretRegistration(cfg *config.ClientAPI, userAPI userapi.ClientUserAPI, sr *SharedSecretRegistration, req *http.Request) util.JSONResponse { ssrr, err := NewSharedSecretRegistrationRequest(req.Body) if err != nil { return util.JSONResponse{ @@ -1080,7 +1080,7 @@ func handleSharedSecretRegistration(userAPI userapi.ClientUserAPI, sr *SharedSec // downcase capitals ssrr.User = strings.ToLower(ssrr.User) - if resErr := validateUsername(ssrr.User); resErr != nil { + if resErr := validateUsername(ssrr.User, cfg.Matrix.ServerName); resErr != nil { return *resErr } if resErr := validatePassword(ssrr.Password); resErr != nil { diff --git a/clientapi/routing/routing.go b/clientapi/routing/routing.go index 6904a2b34..d7a48d228 100644 --- a/clientapi/routing/routing.go +++ b/clientapi/routing/routing.go @@ -133,7 +133,7 @@ func Setup( } } if req.Method == http.MethodPost { - return handleSharedSecretRegistration(userAPI, sr, req) + return handleSharedSecretRegistration(cfg, userAPI, sr, req) } return util.JSONResponse{ Code: http.StatusMethodNotAllowed, From a3eb4e5e9806745f3e6fd36f43bfa07333650cb4 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Sun, 28 Aug 2022 09:52:03 +0100 Subject: [PATCH 2/2] Enforce `type`, `sender`, `state_key` and `room_id` lengths using codepoints rather than bytes (update to matrix-org/gomatrixserverlib@5f66df0, matrix-org/matrix-spec#1001) --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 0833f2908..1f55ee5b8 100644 --- a/go.mod +++ b/go.mod @@ -21,7 +21,7 @@ require ( github.com/matrix-org/dugong v0.0.0-20210921133753-66e6b1c67e2e github.com/matrix-org/go-sqlite3-js v0.0.0-20220419092513-28aa791a1c91 github.com/matrix-org/gomatrix v0.0.0-20210324163249-be2af5ef2e16 - github.com/matrix-org/gomatrixserverlib v0.0.0-20220824082345-662dca17bf94 + github.com/matrix-org/gomatrixserverlib v0.0.0-20220828085018-5f66df08dc66 github.com/matrix-org/pinecone v0.0.0-20220803093810-b7a830c08fb9 github.com/matrix-org/util v0.0.0-20200807132607-55161520e1d4 github.com/mattn/go-sqlite3 v1.14.13 diff --git a/go.sum b/go.sum index fcbdb7e41..8e98e0bcf 100644 --- a/go.sum +++ b/go.sum @@ -343,8 +343,8 @@ github.com/matrix-org/go-sqlite3-js v0.0.0-20220419092513-28aa791a1c91/go.mod h1 github.com/matrix-org/gomatrix v0.0.0-20190528120928-7df988a63f26/go.mod h1:3fxX6gUjWyI/2Bt7J1OLhpCzOfO/bB3AiX0cJtEKud0= github.com/matrix-org/gomatrix v0.0.0-20210324163249-be2af5ef2e16 h1:ZtO5uywdd5dLDCud4r0r55eP4j9FuUNpl60Gmntcop4= github.com/matrix-org/gomatrix v0.0.0-20210324163249-be2af5ef2e16/go.mod h1:/gBX06Kw0exX1HrwmoBibFA98yBk/jxKpGVeyQbff+s= -github.com/matrix-org/gomatrixserverlib v0.0.0-20220824082345-662dca17bf94 h1:zoTv/qxg7C/O995JBPvp+Z8KMR69HhB+M+P22A8Hmm0= -github.com/matrix-org/gomatrixserverlib v0.0.0-20220824082345-662dca17bf94/go.mod h1:jX38yp3SSLJNftBg3PXU1ayd0PCLIiDHQ4xAc9DIixk= +github.com/matrix-org/gomatrixserverlib v0.0.0-20220828085018-5f66df08dc66 h1:4+ycQUZ4Do5LeEuiesz6gD7LkjRNUtk8oETFqjalZxQ= +github.com/matrix-org/gomatrixserverlib v0.0.0-20220828085018-5f66df08dc66/go.mod h1:jX38yp3SSLJNftBg3PXU1ayd0PCLIiDHQ4xAc9DIixk= github.com/matrix-org/pinecone v0.0.0-20220803093810-b7a830c08fb9 h1:ed8yvWhTLk7+sNeK/eOZRTvESFTOHDRevoRoyeqPtvY= github.com/matrix-org/pinecone v0.0.0-20220803093810-b7a830c08fb9/go.mod h1:P4MqPf+u83OPulPJ+XTbSDbbWrdFYNY4LZ/B1PIduFE= github.com/matrix-org/util v0.0.0-20190711121626-527ce5ddefc7/go.mod h1:vVQlW/emklohkZnOPwD3LrZUBqdfsbiyO3p1lNV8F6U=