[barodeur]

The validation of client_secret params accepts invalid values: any value that includes a char in [0-9a-zA-Z.=_-] is accepted, for example "café" is accepted.

Instead synapse should only accept if all chars are within [0-9a-zA-Z.=_-], not just one.

This is the current validator:

ClientSecretStr = Annotated[
    str,
    StringConstraints(
        pattern="[0-9a-zA-Z.=_-]",
        min_length=1,
        max_length=255,
        strict=True,
    ),
]

Unfortunately, Pydantic only defines the pattern argument as:

pattern

A regex pattern that the string must match.

Which is extremely imprecise.

Here is a little script to verify the behavior:

from typing import Annotated
from pydantic import BaseModel, StringConstraints, ValidationError


def check(pattern: str) -> None:
    ClientSecretStr = Annotated[
        str,
        StringConstraints(pattern=pattern, min_length=1, max_length=255, strict=True),
    ]

    class Body(BaseModel):
        client_secret: ClientSecretStr

    try:
        Body.model_validate({"client_secret": "café"})
        print(f"pattern = {pattern!r}: 'café' ACCEPTED  <-- should have been rejected")
    except ValidationError:
        print(f"pattern = {pattern!r}: 'café' rejected")


check("[0-9a-zA-Z.=_-]")  # before the fix (unanchored)
check("^[0-9a-zA-Z.=_-]+$")  # after the fix

which would output

pattern = '[0-9a-zA-Z.=_-]': 'café' ACCEPTED  <-- should have been rejected
pattern = '^[0-9a-zA-Z.=_-]+$': 'café' rejected

History

This is a regression of a previously reported and fixed bug:

• Synapse does not correctly enforce the spec's regex for client_secret matrix-org/synapse#6766 (2020) reported that Synapse did not enforce the spec's client_secret regex at all — with real-world fallout: FluffyChat had started sending secrets containing : because nothing rejected them. Fixed by introducing assert_valid_client_secret (Validate client_secret parameter matrix-org/synapse#6767).
• Use Pydantic to systematically validate a first batch of endpoints in synapse.rest.client.account. matrix-org/synapse#13188 (Synapse 1.66.0) ported the account endpoints to Pydantic and transcribed the regex without anchors/quantifier; Pydantic v1's re.match semantics meant only the first character was validated.
• Pydantic v2 #19071 (Synapse 1.142.0) migrated to Pydantic v2, whose search semantics weakened it further to "any one character anywhere".

---

Pull Request Checklist

• Pull request is based on the develop branch
• Pull request includes a changelog file. The entry should:
  • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
  • Use markdown where necessary, mostly for code blocks.
  • End with either a period (.) or an exclamation mark (!).
  • Start with a capital letter.
  • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
• Code style is correct (run the linters)