From 29b75385a381a198790c94be9e9f37b31b9a8ebf Mon Sep 17 00:00:00 2001 From: David Langley Date: Thu, 7 Nov 2024 09:12:49 +0000 Subject: [PATCH 1/8] handle rendering of invalid date errors --- .../views/messages/DateSeparator.tsx | 36 ++++++++++--------- src/i18n/strings/en_EN.json | 1 + .../views/messages/DateSeparator-test.tsx | 6 ++++ .../__snapshots__/DateSeparator-test.tsx.snap | 27 ++++++++++++++ 4 files changed, 54 insertions(+), 16 deletions(-) diff --git a/src/components/views/messages/DateSeparator.tsx b/src/components/views/messages/DateSeparator.tsx index 10ec7ddad9..e6c7af43ab 100644 --- a/src/components/views/messages/DateSeparator.tsx +++ b/src/components/views/messages/DateSeparator.tsx @@ -96,25 +96,29 @@ export default class DateSeparator extends React.Component { } private getLabel(): string { - const date = new Date(this.props.ts); - const disableRelativeTimestamps = !SettingsStore.getValue(UIFeature.TimelineEnableRelativeDates); + try { + const date = new Date(this.props.ts); + const disableRelativeTimestamps = !SettingsStore.getValue(UIFeature.TimelineEnableRelativeDates); - // During the time the archive is being viewed, a specific day might not make sense, so we return the full date - if (this.props.forExport || disableRelativeTimestamps) return formatFullDateNoTime(date); + // During the time the archive is being viewed, a specific day might not make sense, so we return the full date + if (this.props.forExport || disableRelativeTimestamps) return formatFullDateNoTime(date); - const today = new Date(); - const yesterday = new Date(); - const days = getDaysArray("long"); - yesterday.setDate(today.getDate() - 1); + const today = new Date(); + const yesterday = new Date(); + const days = getDaysArray("long"); + yesterday.setDate(today.getDate() - 1); - if (date.toDateString() === today.toDateString()) { - return this.relativeTimeFormat.format(0, "day"); // Today - } else if (date.toDateString() === yesterday.toDateString()) { - return this.relativeTimeFormat.format(-1, "day"); // Yesterday - } else if (today.getTime() - date.getTime() < 6 * 24 * 60 * 60 * 1000) { - return days[date.getDay()]; // Sunday-Saturday - } else { - return formatFullDateNoTime(date); + if (date.toDateString() === today.toDateString()) { + return this.relativeTimeFormat.format(0, "day"); // Today + } else if (date.toDateString() === yesterday.toDateString()) { + return this.relativeTimeFormat.format(-1, "day"); // Yesterday + } else if (today.getTime() - date.getTime() < 6 * 24 * 60 * 60 * 1000) { + return days[date.getDay()]; // Sunday-Saturday + } else { + return formatFullDateNoTime(date); + } + } catch (e) { + return _t("common|message_timestamp_invalid"); } } diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 41c4396ff3..2a62cc2ac6 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -506,6 +506,7 @@ "matrix": "Matrix", "message": "Message", "message_layout": "Message layout", + "message_timestamp_invalid": "Invalid timestamp", "microphone": "Microphone", "model": "Model", "modern": "Modern", diff --git a/test/unit-tests/components/views/messages/DateSeparator-test.tsx b/test/unit-tests/components/views/messages/DateSeparator-test.tsx index 4bdd66b227..93687be0e7 100644 --- a/test/unit-tests/components/views/messages/DateSeparator-test.tsx +++ b/test/unit-tests/components/views/messages/DateSeparator-test.tsx @@ -91,6 +91,12 @@ describe("DateSeparator", () => { expect(getComponent({ ts, forExport: false }).container.textContent).toEqual(result); }); + it("renders invalid date separator correctly", () => { + let ts = new Date(-8640000000000004).getTime(); + const { asFragment } = getComponent({ ts }); + expect(asFragment()).toMatchSnapshot(); + }); + describe("when forExport is true", () => { it.each(testCases)("formats date in full when current time is %s", (_d, ts) => { expect(getComponent({ ts, forExport: true }).container.textContent).toEqual( diff --git a/test/unit-tests/components/views/messages/__snapshots__/DateSeparator-test.tsx.snap b/test/unit-tests/components/views/messages/__snapshots__/DateSeparator-test.tsx.snap index 2294a40a99..69de39e31d 100644 --- a/test/unit-tests/components/views/messages/__snapshots__/DateSeparator-test.tsx.snap +++ b/test/unit-tests/components/views/messages/__snapshots__/DateSeparator-test.tsx.snap @@ -1,5 +1,32 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`DateSeparator renders invalid date separator correctly 1`] = ` + + + +`; + exports[`DateSeparator renders the date separator correctly 1`] = `
Date: Thu, 7 Nov 2024 09:37:40 +0000 Subject: [PATCH 2/8] lint --- src/components/views/messages/DateSeparator.tsx | 2 +- .../unit-tests/components/views/messages/DateSeparator-test.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/views/messages/DateSeparator.tsx b/src/components/views/messages/DateSeparator.tsx index e6c7af43ab..272afc1823 100644 --- a/src/components/views/messages/DateSeparator.tsx +++ b/src/components/views/messages/DateSeparator.tsx @@ -117,7 +117,7 @@ export default class DateSeparator extends React.Component { } else { return formatFullDateNoTime(date); } - } catch (e) { + } catch (_) { return _t("common|message_timestamp_invalid"); } } diff --git a/test/unit-tests/components/views/messages/DateSeparator-test.tsx b/test/unit-tests/components/views/messages/DateSeparator-test.tsx index 93687be0e7..ebe3ac42d4 100644 --- a/test/unit-tests/components/views/messages/DateSeparator-test.tsx +++ b/test/unit-tests/components/views/messages/DateSeparator-test.tsx @@ -92,7 +92,7 @@ describe("DateSeparator", () => { }); it("renders invalid date separator correctly", () => { - let ts = new Date(-8640000000000004).getTime(); + const ts = new Date(-8640000000000004).getTime(); const { asFragment } = getComponent({ ts }); expect(asFragment()).toMatchSnapshot(); }); From 3f701052048d2db5e411a2031d13e2d4faae9d1f Mon Sep 17 00:00:00 2001 From: David Langley Date: Thu, 7 Nov 2024 10:21:10 +0000 Subject: [PATCH 3/8] lint --- src/components/views/messages/DateSeparator.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/messages/DateSeparator.tsx b/src/components/views/messages/DateSeparator.tsx index 272afc1823..f36c02a16a 100644 --- a/src/components/views/messages/DateSeparator.tsx +++ b/src/components/views/messages/DateSeparator.tsx @@ -117,7 +117,7 @@ export default class DateSeparator extends React.Component { } else { return formatFullDateNoTime(date); } - } catch (_) { + } catch { return _t("common|message_timestamp_invalid"); } } From 6134cfd9c4fed35780b2944f2d7d585730882dcf Mon Sep 17 00:00:00 2001 From: David Langley Date: Wed, 6 Nov 2024 23:14:38 +0000 Subject: [PATCH 4/8] Add mimetype checks Add checks to validate the advertised mimetype and file extension of stickers, videos and images are coherent. --- jest.config.ts | 2 +- package.json | 1 + playwright/e2e/widgets/stickers.spec.ts | 165 +++++++++++------- .../views/messages/MessageEvent.tsx | 110 +++++++++++- .../views/messages/MessageEvent-test.tsx | 54 +++++- yarn.lock | 5 + 6 files changed, 265 insertions(+), 72 deletions(-) diff --git a/jest.config.ts b/jest.config.ts index 4f75eb04db..04f1a91e77 100644 --- a/jest.config.ts +++ b/jest.config.ts @@ -38,7 +38,7 @@ const config: Config = { "recorderWorkletFactory": "/__mocks__/empty.js", "^fetch-mock$": "/node_modules/fetch-mock", }, - transformIgnorePatterns: ["/node_modules/(?!matrix-js-sdk).+$"], + transformIgnorePatterns: ["/node_modules/(?!(mime|matrix-js-sdk)).+$"], collectCoverageFrom: [ "/src/**/*.{js,ts,tsx}", // getSessionLock is piped into a different JS context via stringification, and the coverage functionality is diff --git a/package.json b/package.json index 65ee24b6f0..6c41ae4b0d 100644 --- a/package.json +++ b/package.json @@ -129,6 +129,7 @@ "matrix-js-sdk": "34.10.0", "matrix-widget-api": "^1.9.0", "memoize-one": "^6.0.0", + "mime": "^4.0.4", "oidc-client-ts": "^3.0.1", "opus-recorder": "^8.0.3", "pako": "^2.0.3", diff --git a/playwright/e2e/widgets/stickers.spec.ts b/playwright/e2e/widgets/stickers.spec.ts index ff5526a6e7..318f712961 100644 --- a/playwright/e2e/widgets/stickers.spec.ts +++ b/playwright/e2e/widgets/stickers.spec.ts @@ -6,32 +6,48 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only Please see LICENSE files in the repository root for full details. */ +import * as fs from "node:fs"; + import type { Page } from "@playwright/test"; import { test, expect } from "../../element-web-test"; import { ElementAppPage } from "../../pages/ElementAppPage"; +import { Credentials } from "../../plugins/homeserver"; const STICKER_PICKER_WIDGET_ID = "fake-sticker-picker"; const STICKER_PICKER_WIDGET_NAME = "Fake Stickers"; const STICKER_NAME = "Test Sticker"; const ROOM_NAME_1 = "Sticker Test"; const ROOM_NAME_2 = "Sticker Test Two"; -const STICKER_MESSAGE = JSON.stringify({ - action: "m.sticker", - api: "fromWidget", - data: { - name: "teststicker", - description: STICKER_NAME, - file: "test.png", - content: { - body: STICKER_NAME, - msgtype: "m.sticker", - url: "mxc://localhost/somewhere", +const STICKER_IMAGE = fs.readFileSync("playwright/sample-files/riot.png"); + +function getStickerMessage(contentUri: string, mimetype: string): string { + return JSON.stringify({ + action: "m.sticker", + api: "fromWidget", + data: { + name: "teststicker", + description: STICKER_NAME, + file: "test.png", + content: { + body: STICKER_NAME, + info: { + h: 480, + mimetype: mimetype, + size: 13818, + w: 480, + }, + msgtype: "m.sticker", + url: contentUri, + }, }, - }, - requestId: "1", - widgetId: STICKER_PICKER_WIDGET_ID, -}); -const WIDGET_HTML = ` + requestId: "1", + widgetId: STICKER_PICKER_WIDGET_ID, + }); +} + +function getWidgetHtml(contentUri: string, mimetype: string) { + const stickerMessage = getStickerMessage(contentUri, mimetype); + return ` Fake Sticker Picker @@ -51,13 +67,13 @@ const WIDGET_HTML = ` `; - +} async function openStickerPicker(app: ElementAppPage) { const options = await app.openMessageComposerOptions(); await options.getByRole("menuitem", { name: "Sticker" }).click(); @@ -71,7 +87,8 @@ async function sendStickerFromPicker(page: Page) { await expect(page.locator(".mx_AppTileFullWidth#stickers")).not.toBeVisible(); } -async function expectTimelineSticker(page: Page, roomId: string) { +async function expectTimelineSticker(page: Page, roomId: string, contentUri: string) { + const contentId = contentUri.split("/").slice(-1)[0]; // Make sure it's in the right room await expect(page.locator(".mx_EventTile_sticker > a")).toHaveAttribute("href", new RegExp(`/${roomId}/`)); @@ -80,13 +97,43 @@ async function expectTimelineSticker(page: Page, roomId: string) { // download URL. await expect(page.locator(`img[alt="${STICKER_NAME}"]`)).toHaveAttribute( "src", - new RegExp("/download/localhost/somewhere"), + new RegExp(`/localhost/${contentId}`), ); } +async function expectFileTile(page: Page, roomId: string, contentUri: string) { + await expect(page.locator(".mx_MFileBody_info_filename")).toContainText(STICKER_NAME); +} + +async function setWidgetAccountData( + app: ElementAppPage, + user: Credentials, + stickerPickerUrl: string, + provideCreatorUserId: boolean = true, +) { + await app.client.setAccountData("m.widgets", { + [STICKER_PICKER_WIDGET_ID]: { + content: { + type: "m.stickerpicker", + name: STICKER_PICKER_WIDGET_NAME, + url: stickerPickerUrl, + creatorUserId: provideCreatorUserId ? user.userId : undefined, + }, + sender: user.userId, + state_key: STICKER_PICKER_WIDGET_ID, + type: "m.widget", + id: STICKER_PICKER_WIDGET_ID, + }, + }); +} + test.describe("Stickers", () => { test.use({ displayName: "Sally", + room: async ({ app }, use) => { + const roomId = await app.client.createRoom({ name: ROOM_NAME_1 }); + await use({ roomId }); + }, }); // We spin up a web server for the sticker picker so that we're not testing to see if @@ -96,34 +143,19 @@ test.describe("Stickers", () => { // // See sendStickerFromPicker() for more detail on iframe comms. let stickerPickerUrl: string; - test.beforeEach(async ({ webserver }) => { - stickerPickerUrl = webserver.start(WIDGET_HTML); - }); - test("should send a sticker to multiple rooms", async ({ page, app, user }) => { - const roomId1 = await app.client.createRoom({ name: ROOM_NAME_1 }); + test("should send a sticker to multiple rooms", async ({ webserver, page, app, user, room }) => { const roomId2 = await app.client.createRoom({ name: ROOM_NAME_2 }); - - await app.client.setAccountData("m.widgets", { - [STICKER_PICKER_WIDGET_ID]: { - content: { - type: "m.stickerpicker", - name: STICKER_PICKER_WIDGET_NAME, - url: stickerPickerUrl, - creatorUserId: user.userId, - }, - sender: user.userId, - state_key: STICKER_PICKER_WIDGET_ID, - type: "m.widget", - id: STICKER_PICKER_WIDGET_ID, - }, - }); + const { content_uri: contentUri } = await app.client.uploadContent(STICKER_IMAGE, { type: "image/png" }); + const widgetHtml = getWidgetHtml(contentUri, "image/png"); + stickerPickerUrl = webserver.start(widgetHtml); + setWidgetAccountData(app, user, stickerPickerUrl); await app.viewRoomByName(ROOM_NAME_1); - await expect(page).toHaveURL(`/#/room/${roomId1}`); + await expect(page).toHaveURL(`/#/room/${room.roomId}`); await openStickerPicker(app); await sendStickerFromPicker(page); - await expectTimelineSticker(page, roomId1); + await expectTimelineSticker(page, room.roomId, contentUri); // Ensure that when we switch to a different room that the sticker // goes to the right place @@ -131,31 +163,40 @@ test.describe("Stickers", () => { await expect(page).toHaveURL(`/#/room/${roomId2}`); await openStickerPicker(app); await sendStickerFromPicker(page); - await expectTimelineSticker(page, roomId2); + await expectTimelineSticker(page, roomId2, contentUri); }); - test("should handle a sticker picker widget missing creatorUserId", async ({ page, app, user }) => { - const roomId1 = await app.client.createRoom({ name: ROOM_NAME_1 }); - - await app.client.setAccountData("m.widgets", { - [STICKER_PICKER_WIDGET_ID]: { - content: { - type: "m.stickerpicker", - name: STICKER_PICKER_WIDGET_NAME, - url: stickerPickerUrl, - // No creatorUserId - }, - sender: user.userId, - state_key: STICKER_PICKER_WIDGET_ID, - type: "m.widget", - id: STICKER_PICKER_WIDGET_ID, - }, - }); + test("should handle a sticker picker widget missing creatorUserId", async ({ + webserver, + page, + app, + user, + room, + }) => { + const { content_uri: contentUri } = await app.client.uploadContent(STICKER_IMAGE, { type: "image/png" }); + const widgetHtml = getWidgetHtml(contentUri, "image/png"); + stickerPickerUrl = webserver.start(widgetHtml); + setWidgetAccountData(app, user, stickerPickerUrl, false); await app.viewRoomByName(ROOM_NAME_1); - await expect(page).toHaveURL(`/#/room/${roomId1}`); + await expect(page).toHaveURL(`/#/room/${room.roomId}`); await openStickerPicker(app); await sendStickerFromPicker(page); - await expectTimelineSticker(page, roomId1); + await expectTimelineSticker(page, room.roomId, contentUri); + }); + + test("should render invalid mimetype as a file", async ({ webserver, page, app, user, room }) => { + const { content_uri: contentUri } = await app.client.uploadContent(STICKER_IMAGE, { + type: "application/octet-stream", + }); + const widgetHtml = getWidgetHtml(contentUri, "application/octet-stream"); + stickerPickerUrl = webserver.start(widgetHtml); + setWidgetAccountData(app, user, stickerPickerUrl); + + await app.viewRoomByName(ROOM_NAME_1); + await expect(page).toHaveURL(`/#/room/${room.roomId}`); + await openStickerPicker(app); + await sendStickerFromPicker(page); + await expectFileTile(page, room.roomId, contentUri); }); }); diff --git a/src/components/views/messages/MessageEvent.tsx b/src/components/views/messages/MessageEvent.tsx index e4f3cb83ec..481d5918c7 100644 --- a/src/components/views/messages/MessageEvent.tsx +++ b/src/components/views/messages/MessageEvent.tsx @@ -6,7 +6,9 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only Please see LICENSE files in the repository root for full details. */ +import mime from "mime"; import React, { createRef } from "react"; +import { logger } from "matrix-js-sdk/src/logger"; import { EventType, MsgType, @@ -15,6 +17,7 @@ import { M_LOCATION, M_POLL_END, M_POLL_START, + IContent, } from "matrix-js-sdk/src/matrix"; import SettingsStore from "../../../settings/SettingsStore"; @@ -144,6 +147,98 @@ export default class MessageEvent extends React.Component implements IMe this.forceUpdate(); }; + /** + * Validates that the filename extension and advertised mimetype + * of the supplied image/file message content are not null, match and are actuallly video/image content. + * For image/video messages with a thumbnail it also validates the mimetype is an image. + * @param content The mxEvent content of the message + * @returns + */ + private validateImageOrVideoMimetype = (content: IContent): boolean => { + // As per the spec if filename is not present the body represents the filename + const filename = content.filename ?? content.body; + if (!filename) { + logger.log("Failed to validate image/video content, filename null"); + return false; + } + // Validate mimetype of the thumbnail is valid + const thumbnailResult = this.validateThumbnailMimeType(content); + if (!thumbnailResult) { + logger.log("Failed to validate file/image thumbnail"); + return false; + } + const typeFromExtension = mime.getType(filename); + const majorContentTypeFromExtension = typeFromExtension?.split("/")[0]; + const allowedMajorContentTypes = ["image", "video"]; + + // Validate mimetype of the extension is valid + const result = + !!majorContentTypeFromExtension && allowedMajorContentTypes.includes(majorContentTypeFromExtension); + if (!result) { + logger.log("Failed to validate image/video content, invalid or missing extension"); + } + + // Validate content mimetype is valid if it is set + const contentMimetype = content.info?.mimetype; + if (contentMimetype) { + const majorContentTypeFromContent = contentMimetype?.split("/")[0]; + const result = + !!majorContentTypeFromContent && + allowedMajorContentTypes.includes(majorContentTypeFromContent) && + majorContentTypeFromExtension == majorContentTypeFromContent; + if (!result) { + logger.log("Failed to validate image/video content, invalid or missing mimetype"); + return false; + } + } + return true; + }; + + /** + * Validates that the advertised mimetype of the supplied sticker content + * is not null and is an image. + * For stickers with a thumbnail it also validates the mimetype is an image. + * @param content The mxEvent content of the message + * @returns + */ + private validateStickerMimetype = (content: IContent): boolean => { + // Validate mimetype of the thumbnail is valid + const thumbnailResult = this.validateThumbnailMimeType(content); + if (!thumbnailResult) { + logger.log("Failed to validate sticker thumbnail"); + return false; + } + const contentMimetype = content.info?.mimetype; + if (contentMimetype) { + // Validate mimetype of the content is valid + const majorContentTypeFromContent = contentMimetype?.split("/")[0]; + const result = majorContentTypeFromContent === "image"; + if (!result) { + logger.log("Failed to validate image/video content, invalid or missing mimetype/extensions"); + return false; + } + } + return true; + }; + + /** + * Validates the thumbnail assocaited with an image/video message or sticker + * is has an image mimetype. + * @param content The mxEvent content of the message + * @returns + */ + private validateThumbnailMimeType = (content: IContent): boolean => { + const thumbnailInfo = content.info?.thumbnail_info; + if (thumbnailInfo) { + const majorContentTypeFromThumbnail = thumbnailInfo.mimetype?.split("/")[0]; + if (!majorContentTypeFromThumbnail || majorContentTypeFromThumbnail !== "image") { + logger.log("Failed to validate image/video content, thumbnail mimetype is not an image"); + return false; + } + } + return true; + }; + public render(): React.ReactNode { const content = this.props.mxEvent.getContent(); const type = this.props.mxEvent.getType(); @@ -154,9 +249,20 @@ export default class MessageEvent extends React.Component implements IMe if (this.props.mxEvent.isDecryptionFailure()) { BodyType = DecryptionFailureBody; } else if (type && this.evTypes.has(type)) { - BodyType = this.evTypes.get(type)!; + if (type == EventType.Sticker && !this.validateStickerMimetype(content)) { + BodyType = this.bodyTypes.get(MsgType.File)!; + } else { + BodyType = this.evTypes.get(type)!; + } } else if (msgtype && this.bodyTypes.has(msgtype)) { - BodyType = this.bodyTypes.get(msgtype)!; + if ( + (msgtype == MsgType.Image || msgtype == MsgType.Video) && + !this.validateImageOrVideoMimetype(content) + ) { + BodyType = this.bodyTypes.get(MsgType.File)!; + } else { + BodyType = this.bodyTypes.get(msgtype)!; + } } else if (content.url) { // Fallback to MFileBody if there's a content URL BodyType = this.bodyTypes.get(MsgType.File)!; diff --git a/test/unit-tests/components/views/messages/MessageEvent-test.tsx b/test/unit-tests/components/views/messages/MessageEvent-test.tsx index 2195e8de0e..9ee323f175 100644 --- a/test/unit-tests/components/views/messages/MessageEvent-test.tsx +++ b/test/unit-tests/components/views/messages/MessageEvent-test.tsx @@ -33,6 +33,16 @@ jest.mock("../../../../../src/components/views/messages/MImageBody", () => ({ default: () =>
, })); +jest.mock("../../../../../src/components/views/messages/MVideoBody", () => ({ + __esModule: true, + default: () =>
, +})); + +jest.mock("../../../../../src/components/views/messages/MFileBody", () => ({ + __esModule: true, + default: () =>
, +})); + jest.mock("../../../../../src/components/views/messages/MImageReplyBody", () => ({ __esModule: true, default: () =>
, @@ -95,8 +105,8 @@ describe("MessageEvent", () => { describe("when an image with a caption is sent", () => { let result: RenderResult; - beforeEach(() => { - event = mkEvent({ + function createEvent(mimetype: string, filename: string, msgtype: string) { + return mkEvent({ event: true, type: EventType.RoomMessage, user: client.getUserId()!, @@ -105,19 +115,19 @@ describe("MessageEvent", () => { body: "caption for a test image", format: "org.matrix.custom.html", formatted_body: "caption for a test image", - msgtype: MsgType.Image, - filename: "image.webp", + msgtype: msgtype, + filename: filename, info: { w: 40, h: 50, + mimetype: mimetype, }, url: "mxc://server/image", }, }); - result = renderMessageEvent(); - }); + } - it("should render a TextualBody and an ImageBody", () => { + function mockMedia() { fetchMock.getOnce( "https://server/_matrix/media/v3/download/server/image", { @@ -125,8 +135,38 @@ describe("MessageEvent", () => { }, { sendAsJson: false }, ); + } + + it("should render a TextualBody and an ImageBody", () => { + event = createEvent("image/webp", "image.webp", MsgType.Image); + result = renderMessageEvent(); + mockMedia(); result.getByTestId("image-body"); result.getByTestId("textual-body"); }); + + it("should render a TextualBody and a FileBody for mismatched extension", () => { + event = createEvent("image/webp", "image.exe", MsgType.Image); + result = renderMessageEvent(); + mockMedia(); + result.getByTestId("file-body"); + result.getByTestId("textual-body"); + }); + + it("should render a TextualBody and an VideoBody", () => { + event = createEvent("video/mp4", "video.mp4", MsgType.Video); + result = renderMessageEvent(); + mockMedia(); + result.getByTestId("video-body"); + result.getByTestId("textual-body"); + }); + + it("should render a TextualBody and a FileBody for non-video mimetype", () => { + event = createEvent("application/octet-stream", "video.mp4", MsgType.Video); + result = renderMessageEvent(); + mockMedia(); + result.getByTestId("file-body"); + result.getByTestId("textual-body"); + }); }); }); diff --git a/yarn.lock b/yarn.lock index 3e1c409a24..f246eaf135 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8411,6 +8411,11 @@ mime@1.6.0: resolved "https://registry.yarnpkg.com/mime/-/mime-1.6.0.tgz#32cd9e5c64553bd58d19a568af452acff04981b1" integrity sha512-x0Vn8spI+wuJ1O6S7gnbaQg8Pxh4NNHb7KSINmEWKiPE4RKOplvijn+NkmYmmRgP68mc70j2EbeTFRsrswaQeg== +mime@^4.0.4: + version "4.0.4" + resolved "https://registry.yarnpkg.com/mime/-/mime-4.0.4.tgz#9f851b0fc3c289d063b20a7a8055b3014b25664b" + integrity sha512-v8yqInVjhXyqP6+Kw4fV3ZzeMRqEW6FotRsKXjRS5VMTNIuXsdRoAvklpoRgSqXm6o9VNH4/C0mgedko9DdLsQ== + mimic-fn@^2.1.0: version "2.1.0" resolved "https://registry.yarnpkg.com/mimic-fn/-/mimic-fn-2.1.0.tgz#7ed2c2ccccaf84d3ffcb7a69b57711fc2083401b" From c0a313abae83c9b25a4cdd4ee098f6dd40cadb62 Mon Sep 17 00:00:00 2001 From: David Langley Date: Mon, 11 Nov 2024 19:38:58 +0000 Subject: [PATCH 5/8] Make logic more DRY, simplify logic, improve naming. --- .../views/messages/MessageEvent.tsx | 113 +++++++++--------- 1 file changed, 57 insertions(+), 56 deletions(-) diff --git a/src/components/views/messages/MessageEvent.tsx b/src/components/views/messages/MessageEvent.tsx index 481d5918c7..d047be1fdf 100644 --- a/src/components/views/messages/MessageEvent.tsx +++ b/src/components/views/messages/MessageEvent.tsx @@ -149,10 +149,10 @@ export default class MessageEvent extends React.Component implements IMe /** * Validates that the filename extension and advertised mimetype - * of the supplied image/file message content are not null, match and are actuallly video/image content. + * of the supplied image/file message content match and are actuallly video/image content. * For image/video messages with a thumbnail it also validates the mimetype is an image. * @param content The mxEvent content of the message - * @returns + * @returns A boolean indicating whether the validation passed */ private validateImageOrVideoMimetype = (content: IContent): boolean => { // As per the spec if filename is not present the body represents the filename @@ -161,32 +161,28 @@ export default class MessageEvent extends React.Component implements IMe logger.log("Failed to validate image/video content, filename null"); return false; } - // Validate mimetype of the thumbnail is valid - const thumbnailResult = this.validateThumbnailMimeType(content); - if (!thumbnailResult) { + // Check mimetype of the thumbnail + if (!this.validateThumbnailMimeType(content)) { logger.log("Failed to validate file/image thumbnail"); return false; } - const typeFromExtension = mime.getType(filename); - const majorContentTypeFromExtension = typeFromExtension?.split("/")[0]; - const allowedMajorContentTypes = ["image", "video"]; - // Validate mimetype of the extension is valid - const result = - !!majorContentTypeFromExtension && allowedMajorContentTypes.includes(majorContentTypeFromExtension); - if (!result) { + // if there is no mimetype from the extesion or the mimetype is not image/video validation fails. + const typeFromExtension = mime.getType(filename) ?? undefined; + const extensionMajorMimeType = this.parseMajorMimeType(typeFromExtension); + if (!typeFromExtension || !this.validateAllowedMimetype(typeFromExtension, ["image", "video"])) { logger.log("Failed to validate image/video content, invalid or missing extension"); + return false; } - // Validate content mimetype is valid if it is set + // if the content mimetype is set check it is an image/video and that it matches the extesion mimetype otherwise validation fails const contentMimetype = content.info?.mimetype; if (contentMimetype) { - const majorContentTypeFromContent = contentMimetype?.split("/")[0]; - const result = - !!majorContentTypeFromContent && - allowedMajorContentTypes.includes(majorContentTypeFromContent) && - majorContentTypeFromExtension == majorContentTypeFromContent; - if (!result) { + const contentMajorMimetype = this.parseMajorMimeType(contentMimetype); + if ( + !this.validateAllowedMimetype(contentMimetype, ["image", "video"]) || + extensionMajorMimeType !== contentMajorMimetype + ) { logger.log("Failed to validate image/video content, invalid or missing mimetype"); return false; } @@ -195,50 +191,59 @@ export default class MessageEvent extends React.Component implements IMe }; /** - * Validates that the advertised mimetype of the supplied sticker content - * is not null and is an image. + * Validates that the advertised mimetype of the sticker content + * is an image. * For stickers with a thumbnail it also validates the mimetype is an image. * @param content The mxEvent content of the message - * @returns + * @returns A boolean indicating whether the validation passed */ private validateStickerMimetype = (content: IContent): boolean => { - // Validate mimetype of the thumbnail is valid + // Validate mimetype of the thumbnail const thumbnailResult = this.validateThumbnailMimeType(content); if (!thumbnailResult) { logger.log("Failed to validate sticker thumbnail"); return false; } + // Validate mimetype of the content info is valid if it is set const contentMimetype = content.info?.mimetype; - if (contentMimetype) { - // Validate mimetype of the content is valid - const majorContentTypeFromContent = contentMimetype?.split("/")[0]; - const result = majorContentTypeFromContent === "image"; - if (!result) { - logger.log("Failed to validate image/video content, invalid or missing mimetype/extensions"); - return false; - } + if (contentMimetype && !this.validateAllowedMimetype(contentMimetype, ["image"])) { + logger.log("Failed to validate image/video content, invalid or missing mimetype/extensions"); + return false; } return true; }; /** - * Validates the thumbnail assocaited with an image/video message or sticker - * is has an image mimetype. + * For image/video messages or stickers that have a thumnail mimetype specified, + * validates that the major mimetime is image. * @param content The mxEvent content of the message - * @returns + * @returns A boolean indicating whether the validation passed */ private validateThumbnailMimeType = (content: IContent): boolean => { - const thumbnailInfo = content.info?.thumbnail_info; - if (thumbnailInfo) { - const majorContentTypeFromThumbnail = thumbnailInfo.mimetype?.split("/")[0]; - if (!majorContentTypeFromThumbnail || majorContentTypeFromThumbnail !== "image") { - logger.log("Failed to validate image/video content, thumbnail mimetype is not an image"); - return false; - } - } - return true; + const thumbnailMimetype = content.info?.thumbnail_info?.mimetype; + return !thumbnailMimetype || this.validateAllowedMimetype(thumbnailMimetype, ["image"]); }; + /** + * Validates that the major part of a mimetime from an allowed list. + * @param mimetype The mimetype to validate + * @param allowedMajorMimeTypes The list of allowed major mimetimes + * @returns A boolean indicating whether the validation passed + */ + private validateAllowedMimetype = (mimetype: string, allowedMajorMimeTypes: string[]): boolean => { + const majorMimetype = this.parseMajorMimeType(mimetype); + return !!majorMimetype && allowedMajorMimeTypes.includes(majorMimetype); + }; + + /** + * Parses and returns the the major part of a mimetype(before the "/"). + * @param mimetype As optional mimetype string to parse + * @returns The major part of the mimetype string or undefined + */ + private parseMajorMimeType(mimetype?: string): string | undefined { + return mimetype?.split("/")[0]; + } + public render(): React.ReactNode { const content = this.props.mxEvent.getContent(); const type = this.props.mxEvent.getType(); @@ -249,20 +254,9 @@ export default class MessageEvent extends React.Component implements IMe if (this.props.mxEvent.isDecryptionFailure()) { BodyType = DecryptionFailureBody; } else if (type && this.evTypes.has(type)) { - if (type == EventType.Sticker && !this.validateStickerMimetype(content)) { - BodyType = this.bodyTypes.get(MsgType.File)!; - } else { - BodyType = this.evTypes.get(type)!; - } + BodyType = this.evTypes.get(type)!; } else if (msgtype && this.bodyTypes.has(msgtype)) { - if ( - (msgtype == MsgType.Image || msgtype == MsgType.Video) && - !this.validateImageOrVideoMimetype(content) - ) { - BodyType = this.bodyTypes.get(MsgType.File)!; - } else { - BodyType = this.bodyTypes.get(msgtype)!; - } + BodyType = this.bodyTypes.get(msgtype)!; } else if (content.url) { // Fallback to MFileBody if there's a content URL BodyType = this.bodyTypes.get(MsgType.File)!; @@ -271,6 +265,13 @@ export default class MessageEvent extends React.Component implements IMe BodyType = UnknownBody; } + if ( + ((BodyType === MImageBody || BodyType == MVideoBody) && !this.validateImageOrVideoMimetype(content)) || + (BodyType === MStickerBody && !this.validateStickerMimetype(content)) + ) { + BodyType = this.bodyTypes.get(MsgType.File)!; + } + // TODO: move to eventTypes when location sharing spec stabilises if (M_LOCATION.matches(type) || (type === EventType.RoomMessage && msgtype === MsgType.Location)) { BodyType = MLocationBody; From bff17ff470d5efe399d4019721a77f36bb5786ac Mon Sep 17 00:00:00 2001 From: David Langley Date: Mon, 11 Nov 2024 19:54:05 +0000 Subject: [PATCH 6/8] Make case consistent --- src/components/views/messages/MessageEvent.tsx | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/components/views/messages/MessageEvent.tsx b/src/components/views/messages/MessageEvent.tsx index d047be1fdf..60fcce6493 100644 --- a/src/components/views/messages/MessageEvent.tsx +++ b/src/components/views/messages/MessageEvent.tsx @@ -162,14 +162,14 @@ export default class MessageEvent extends React.Component implements IMe return false; } // Check mimetype of the thumbnail - if (!this.validateThumbnailMimeType(content)) { + if (!this.validateThumbnailMimetype(content)) { logger.log("Failed to validate file/image thumbnail"); return false; } - // if there is no mimetype from the extesion or the mimetype is not image/video validation fails. + // if there is no mimetype from the extesion or the mimetype is not image/video validation fails const typeFromExtension = mime.getType(filename) ?? undefined; - const extensionMajorMimeType = this.parseMajorMimeType(typeFromExtension); + const extensionMajorMimetype = this.parseMajorMimetype(typeFromExtension); if (!typeFromExtension || !this.validateAllowedMimetype(typeFromExtension, ["image", "video"])) { logger.log("Failed to validate image/video content, invalid or missing extension"); return false; @@ -178,10 +178,10 @@ export default class MessageEvent extends React.Component implements IMe // if the content mimetype is set check it is an image/video and that it matches the extesion mimetype otherwise validation fails const contentMimetype = content.info?.mimetype; if (contentMimetype) { - const contentMajorMimetype = this.parseMajorMimeType(contentMimetype); + const contentMajorMimetype = this.parseMajorMimetype(contentMimetype); if ( !this.validateAllowedMimetype(contentMimetype, ["image", "video"]) || - extensionMajorMimeType !== contentMajorMimetype + extensionMajorMimetype !== contentMajorMimetype ) { logger.log("Failed to validate image/video content, invalid or missing mimetype"); return false; @@ -199,7 +199,7 @@ export default class MessageEvent extends React.Component implements IMe */ private validateStickerMimetype = (content: IContent): boolean => { // Validate mimetype of the thumbnail - const thumbnailResult = this.validateThumbnailMimeType(content); + const thumbnailResult = this.validateThumbnailMimetype(content); if (!thumbnailResult) { logger.log("Failed to validate sticker thumbnail"); return false; @@ -219,7 +219,7 @@ export default class MessageEvent extends React.Component implements IMe * @param content The mxEvent content of the message * @returns A boolean indicating whether the validation passed */ - private validateThumbnailMimeType = (content: IContent): boolean => { + private validateThumbnailMimetype = (content: IContent): boolean => { const thumbnailMimetype = content.info?.thumbnail_info?.mimetype; return !thumbnailMimetype || this.validateAllowedMimetype(thumbnailMimetype, ["image"]); }; @@ -231,7 +231,7 @@ export default class MessageEvent extends React.Component implements IMe * @returns A boolean indicating whether the validation passed */ private validateAllowedMimetype = (mimetype: string, allowedMajorMimeTypes: string[]): boolean => { - const majorMimetype = this.parseMajorMimeType(mimetype); + const majorMimetype = this.parseMajorMimetype(mimetype); return !!majorMimetype && allowedMajorMimeTypes.includes(majorMimetype); }; @@ -240,7 +240,7 @@ export default class MessageEvent extends React.Component implements IMe * @param mimetype As optional mimetype string to parse * @returns The major part of the mimetype string or undefined */ - private parseMajorMimeType(mimetype?: string): string | undefined { + private parseMajorMimetype(mimetype?: string): string | undefined { return mimetype?.split("/")[0]; } From bebf44d9eeaa5c22dcd3a1e8a85ef14bcb123d51 Mon Sep 17 00:00:00 2001 From: RiotRobot Date: Tue, 12 Nov 2024 09:31:49 +0000 Subject: [PATCH 7/8] Upgrade dependency to matrix-js-sdk@34.11.1 --- package.json | 2 +- yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index 6c41ae4b0d..1752e8c63c 100644 --- a/package.json +++ b/package.json @@ -126,7 +126,7 @@ "maplibre-gl": "^2.0.0", "matrix-encrypt-attachment": "^1.0.3", "matrix-events-sdk": "0.0.1", - "matrix-js-sdk": "34.10.0", + "matrix-js-sdk": "34.11.1", "matrix-widget-api": "^1.9.0", "memoize-one": "^6.0.0", "mime": "^4.0.4", diff --git a/yarn.lock b/yarn.lock index f246eaf135..b6d20ac40f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8250,10 +8250,10 @@ matrix-events-sdk@0.0.1: resolved "https://registry.yarnpkg.com/matrix-events-sdk/-/matrix-events-sdk-0.0.1.tgz#c8c38911e2cb29023b0bbac8d6f32e0de2c957dd" integrity sha512-1QEOsXO+bhyCroIe2/A5OwaxHvBm7EsSQ46DEDn8RBIfQwN5HWBpFvyWWR4QY0KHPPnnJdI99wgRiAl7Ad5qaA== -matrix-js-sdk@34.10.0: - version "34.10.0" - resolved "https://registry.yarnpkg.com/matrix-js-sdk/-/matrix-js-sdk-34.10.0.tgz#39d2b4a10ee9a05dc5b40553e317b65b2b5a5efd" - integrity sha512-+X9euFC1BeDIX06ckPeYD4Bs1yIGzETTSqVsktjZG6FOx6kr8CGIenmcCe4majq0Kj9tpv+p/NFenh59aPdh8Q== +matrix-js-sdk@34.11.1: + version "34.11.1" + resolved "https://registry.yarnpkg.com/matrix-js-sdk/-/matrix-js-sdk-34.11.1.tgz#b9a6d8212d8417682265769b3088c4717458089c" + integrity sha512-rDbIUIqEsN/pbHb6haBQmjxxgeb9G3Df2IhPPOotUbX6R1KseA8yJ6TAY0YySM2zVaBV3yZ6dnKWexF/uWvZfA== dependencies: "@babel/runtime" "^7.12.5" "@matrix-org/matrix-sdk-crypto-wasm" "^9.0.0" From bbe474ae578d1f2152f57093d744321a24811279 Mon Sep 17 00:00:00 2001 From: RiotRobot Date: Tue, 12 Nov 2024 09:43:10 +0000 Subject: [PATCH 8/8] v1.11.85 --- CHANGELOG.md | 8 ++++++++ package.json | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1274048c8c..6260a72f99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +Changes in [1.11.85](https://github.com/element-hq/element-web/releases/tag/v1.11.85) (2024-11-12) +================================================================================================== +# Security +- Fixes for [CVE-2024-51750](https://www.cve.org/CVERecord?id=CVE-2024-51750) / [GHSA-w36j-v56h-q9pc](https://github.com/element-hq/element-web/security/advisories/GHSA-w36j-v56h-q9pc) +- Fixes for [CVE-2024-51749](https://www.cve.org/CVERecord?id=CVE-2024-51749) / [GHSA-5486-384g-mcx2](https://github.com/element-hq/element-web/security/advisories/GHSA-5486-384g-mcx2) +- Update JS SDK with the fixes for [CVE-2024-50336](https://www.cve.org/CVERecord?id=CVE-2024-50336) / [GHSA-xvg8-m4x3-w6xr](https://github.com/matrix-org/matrix-js-sdk/security/advisories/GHSA-xvg8-m4x3-w6xr) + + Changes in [1.11.84](https://github.com/element-hq/element-web/releases/tag/v1.11.84) (2024-11-05) ================================================================================================== ## ✨ Features diff --git a/package.json b/package.json index 1752e8c63c..c6fa234cae 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "element-web", - "version": "1.11.84", + "version": "1.11.85", "description": "A feature-rich client for Matrix.org", "author": "New Vector Ltd.", "repository": {