Stricter mxc/mxid validation #105

l1s5 · 2026-11-03T07:53:23Z

l1s5 commented

2026-11-03 07:53:23 +00:00

Here are some centralized and well tested regexes for mxc and mxid parsing :)

l1s5 added 2 commits

2026-11-03 07:53:24 +00:00

matrix: stricter mxc validation d087586bbd

This commit refactors various instances of mxc matching so that there
is a single well-tested regex that can be reused everywhere. The new
regex is made to match the specification word for word, and should
hopefully rule out most over- and undermatching.

matrix: stricter mxid validation e7af19de80

This commit refactors various instances of mxid matching so that there
is a single well-tested regex that can be reused everywhere. The new
regex is made to match the specification word for word, and should
hopefully rule out most over- and undermatching.

By chance, this also fixes a bug in `src/discord/interactions/ping.js`
where the autocomplete tried matching an mxid against a raw sql query
rather than the provided input.

cadence reviewed

2026-11-03 09:53:52 +00:00

cadence left a comment

yay

src/matrix/api.js Outdated

					
				@ -11,2 +11,4 @@

				/** @type {import("./txnid")} */

				const makeTxnId = sync.require("./txnid")

				/** @type {import("./utils")} */

				const mUtils = sync.require("./utils")

I call it mxUtils more often, pls rename

I call it `mxUtils` more often, pls rename

l1s5 marked this conversation as resolved

src/matrix/utils.js Outdated

					
				@ -244,16 +239,165 @@ function getPublicUrlForMxc(mxc) {

					return `${reg.ooye.bridge_origin}/download/matrix/${serverAndMediaID}`

				}

				const _serverNameIpv4Address = String.raw`[0-9]{1,3}(?:\.[0-9]{1,3}){3}`

Since no patterns or regexps are being exported now and the only access is through functions, you can remove the leading _, which should make the regexp construction less dense on symbols.

This suggestion is optional.

Since no patterns or regexps are being exported now and the only access is through functions, you can remove the leading `_`, which should make the regexp construction less dense on symbols. This suggestion is optional.

l1s5 marked this conversation as resolved

src/matrix/utils.js Outdated

					
				@ -247,0 +291,4 @@

				/**

				 * @param {string} mxc

				 * @param {{[key: string]: string}} matchGroups

				 * @returns {{mxc: string, serverName: string, mediaId: string} | undefined} undefined if it isn't a valid mxc URI

I would really like to return null rather than undefined for all of these

I would really like to return `null` rather than `undefined` for all of these

l1s5 marked this conversation as resolved

src/matrix/utils.js Outdated

					
				@ -247,0 +296,4 @@

				function _mxcFromMatchGroups(mxc, matchGroups) {

					// "." and ".." are *technically* valid server names,

					// but would become dot-segments (path traversal) in URL paths.

					if (matchGroups.serverName === "." || matchGroups.serverName === "..") return undefined

I think we should check that server names match [^.][.][^.], i.e. a name on each side of a period. Although a name like example. is technically valid, it's not practically valid. This could prevent a class of typos where people don't finish typing their URL, and it takes out . and .. too.

Move this check to isServerName and call it?

I think we should check that server names match `[^.][.][^.]`, i.e. a name on each side of a period. Although a name like `example.` is technically valid, it's not _practically_ valid. This could prevent a class of typos where people don't finish typing their URL, and it takes out `.` and `..` too. Move this check to `isServerName` and call it?

I've rewritten the . check as serverName.startsWith(".") || serverName.endsWith("."), but I ended up copy pasting it in 3 locations in order not to run the same input through the same regex twice.

I've rewritten the `.` check as `serverName.startsWith(".") || serverName.endsWith(".")`, but I ended up copy pasting it in 3 locations in order not to run the same input through the same regex twice.

src/matrix/utils.js Outdated

					
				@ -247,0 +337,4 @@

				 * @see https://spec.matrix.org/latest/appendices/#user-identifiers specification

				 * @see https://spec.matrix.org/latest/appendices/#historical-user-ids historical user IDs

				 */

				const _mxidLocalpartPattern = String.raw`[\x21-\x39\x3B-\x3F\x41-\x7E]+`

* to match zero characters or no?

`*` to match zero characters or no?

l1s5 marked this conversation as resolved

src/matrix/utils.test.js Outdated

					
				@ -420,0 +472,4 @@

				];

				for (const mxc of INVALID_MXCS) {

					test(`parse mxc: ${String(mxc)} is invalid`, t => {

String() does nothing here, remove unless you need it to keep the LSP happy

l1s5 marked this conversation as resolved

src/web/routes/download-matrix.js Outdated

					
				@ -22,2 +22,4 @@

				/** @type {import("../../m2d/actions/sticker")} */

				const sticker = sync.require("../../m2d/actions/sticker")

				/** @type {import("../../matrix/utils")} */

				const mUtils = sync.require("../../matrix/utils")

I call it mxUtils more often, pls rename

I call it `mxUtils` more often, pls rename

l1s5 marked this conversation as resolved

src/web/routes/log-in-with-matrix.test.js Outdated

					
				@ -29,3 +29,3 @@

					const [error] = await tryToCatch(() => router.test("post", "/api/log-in-with-matrix", {

						body: {

							mxid: "@cadence:cadence."

							mxid: "@cadence:cadence_moe"