diff --git a/CHANGELOG.md b/CHANGELOG.md index cc16d3f96b..e11ba1a5dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +Changes in [34.11.0](https://github.com/matrix-org/matrix-js-sdk/releases/tag/v34.11.0) (2024-11-12) +==================================================================================================== +# Security +- Fixes for [CVE-2024-50336](https://nvd.nist.gov/vuln/detail/CVE-2024-50336) / [GHSA-xvg8-m4x3-w6xr](https://github.com/matrix-org/matrix-js-sdk/security/advisories/GHSA-xvg8-m4x3-w6xr). + Changes in [34.10.0](https://github.com/matrix-org/matrix-js-sdk/releases/tag/v34.10.0) (2024-11-05) ==================================================================================================== ## 🦖 Deprecations diff --git a/spec/unit/content-repo.spec.ts b/spec/unit/content-repo.spec.ts index 23f504c1e4..8d24107fe2 100644 --- a/spec/unit/content-repo.spec.ts +++ b/spec/unit/content-repo.spec.ts @@ -63,20 +63,6 @@ describe("ContentRepo", function () { ); }); - it("should put fragments from mxc:// URIs after any query parameters", function () { - const mxcUri = "mxc://server.name/resourceid#automade"; - expect(getHttpUriForMxc(baseUrl, mxcUri, 32)).toEqual( - baseUrl + "/_matrix/media/v3/thumbnail/server.name/resourceid" + "?width=32#automade", - ); - }); - - it("should put fragments from mxc:// URIs at the end of the HTTP URI", function () { - const mxcUri = "mxc://server.name/resourceid#automade"; - expect(getHttpUriForMxc(baseUrl, mxcUri)).toEqual( - baseUrl + "/_matrix/media/v3/download/server.name/resourceid#automade", - ); - }); - it("should return an authenticated URL when requested", function () { const mxcUri = "mxc://server.name/resourceid"; expect(getHttpUriForMxc(baseUrl, mxcUri, undefined, undefined, undefined, undefined, true, true)).toEqual( @@ -98,5 +84,30 @@ describe("ContentRepo", function () { "/_matrix/client/v1/media/thumbnail/server.name/resourceid?width=64&height=64&method=scale&allow_redirect=true", ); }); + + it("should drop mxc urls with invalid server_name", () => { + const mxcUri = "mxc://server.name:test/foobar"; + expect(getHttpUriForMxc(baseUrl, mxcUri)).toEqual(""); + }); + + it("should drop mxc urls with invalid media_id", () => { + const mxcUri = "mxc://server.name/foobar:test"; + expect(getHttpUriForMxc(baseUrl, mxcUri)).toEqual(""); + }); + + it("should drop mxc urls attempting a path traversal attack", () => { + const mxcUri = "mxc://../../../../foo"; + expect(getHttpUriForMxc(baseUrl, mxcUri)).toEqual(""); + }); + + it("should drop mxc urls attempting to pass query parameters", () => { + const mxcUri = "mxc://server.name/foobar?bar=baz"; + expect(getHttpUriForMxc(baseUrl, mxcUri)).toEqual(""); + }); + + it("should drop mxc urls with too many parts", () => { + const mxcUri = "mxc://server.name/foo//bar"; + expect(getHttpUriForMxc(baseUrl, mxcUri)).toEqual(""); + }); }); }); diff --git a/src/content-repo.ts b/src/content-repo.ts index b6174b6d8d..b0b02a0569 100644 --- a/src/content-repo.ts +++ b/src/content-repo.ts @@ -1,5 +1,5 @@ /* -Copyright 2015 - 2021 The Matrix.org Foundation C.I.C. +Copyright 2015 - 2024 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,7 +14,22 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { encodeParams } from "./utils.ts"; +// Validation based on https://spec.matrix.org/v1.12/appendices/#server-name +// We do not use the validation described in https://spec.matrix.org/v1.12/client-server-api/#security-considerations-5 +// as it'd wrongly make all MXCs invalid due to not allowing `[].:` in server names. +const serverNameRegex = + /^(?:(?:\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})|(?:\[[\dA-Fa-f:.]{2,45}])|(?:[A-Za-z\d\-.]{1,255}))(?::\d{1,5})?$/; +function validateServerName(serverName: string): boolean { + const matches = serverNameRegex.exec(serverName); + return matches?.[0] === serverName; +} + +// Validation based on https://spec.matrix.org/v1.12/client-server-api/#security-considerations-5 +const mediaIdRegex = /^[\w-]+$/; +function validateMediaId(mediaId: string): boolean { + const matches = mediaIdRegex.exec(mediaId); + return matches?.[0] === mediaId; +} /** * Get the HTTP URL for an MXC URI. @@ -36,7 +51,7 @@ import { encodeParams } from "./utils.ts"; * for authenticated media will *not* be checked - it is the caller's responsibility * to do so before calling this function. Note also that `useAuthentication` * implies `allowRedirects`. Defaults to false (unauthenticated endpoints). - * @returns The complete URL to the content. + * @returns The complete URL to the content, may be an empty string if the provided mxc is not valid. */ export function getHttpUriForMxc( baseUrl: string, @@ -51,7 +66,7 @@ export function getHttpUriForMxc( if (typeof mxc !== "string" || !mxc) { return ""; } - if (mxc.indexOf("mxc://") !== 0) { + if (!mxc.startsWith("mxc://")) { if (allowDirectLinks) { return mxc; } else { @@ -59,6 +74,11 @@ export function getHttpUriForMxc( } } + const [serverName, mediaId, ...rest] = mxc.slice(6).split("/"); + if (rest.length > 0 || !validateServerName(serverName) || !validateMediaId(mediaId)) { + return ""; + } + if (useAuthentication) { allowRedirects = true; // per docs (MSC3916 always expects redirects) @@ -67,46 +87,31 @@ export function getHttpUriForMxc( // callers, hopefully. } - let serverAndMediaId = mxc.slice(6); // strips mxc:// let prefix: string; + const isThumbnailRequest = !!width || !!height || !!resizeMethod; + const verb = isThumbnailRequest ? "thumbnail" : "download"; if (useAuthentication) { - prefix = "/_matrix/client/v1/media/download/"; + prefix = `/_matrix/client/v1/media/${verb}`; } else { - prefix = "/_matrix/media/v3/download/"; + prefix = `/_matrix/media/v3/${verb}`; } - const params: Record = {}; + + const url = new URL(`${prefix}/${serverName}/${mediaId}`, baseUrl); if (width) { - params["width"] = Math.round(width).toString(); + url.searchParams.set("width", Math.round(width).toString()); } if (height) { - params["height"] = Math.round(height).toString(); + url.searchParams.set("height", Math.round(height).toString()); } if (resizeMethod) { - params["method"] = resizeMethod; - } - if (Object.keys(params).length > 0) { - // these are thumbnailing params so they probably want the - // thumbnailing API... - if (useAuthentication) { - prefix = "/_matrix/client/v1/media/thumbnail/"; - } else { - prefix = "/_matrix/media/v3/thumbnail/"; - } + url.searchParams.set("method", resizeMethod); } if (typeof allowRedirects === "boolean") { // We add this after, so we don't convert everything to a thumbnail request. - params["allow_redirect"] = JSON.stringify(allowRedirects); - } - - const fragmentOffset = serverAndMediaId.indexOf("#"); - let fragment = ""; - if (fragmentOffset >= 0) { - fragment = serverAndMediaId.slice(fragmentOffset); - serverAndMediaId = serverAndMediaId.slice(0, fragmentOffset); + url.searchParams.set("allow_redirect", JSON.stringify(allowRedirects)); } - const urlParams = Object.keys(params).length === 0 ? "" : "?" + encodeParams(params); - return baseUrl + prefix + serverAndMediaId + urlParams + fragment; + return url.href; }