From 8e2a296181b0bebf6ed41f7dae67e6afeaa70cb8 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 16 Jan 2025 15:17:08 +0000 Subject: [PATCH 1/3] Switch to secure random strings Because the js-sdk methods are changing and there's no reason for these not to use the secure versions. The dedicated upper/lower functions were *only* used in this one case, so this should do the exact same thing with the one exported function. Requires https://github.com/matrix-org/matrix-js-sdk/pull/4621 (merge both together) --- src/utils/WidgetUtils.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/utils/WidgetUtils.ts b/src/utils/WidgetUtils.ts index d9de59e4013..7ff1886511c 100644 --- a/src/utils/WidgetUtils.ts +++ b/src/utils/WidgetUtils.ts @@ -14,7 +14,7 @@ import { Room, ClientEvent, MatrixClient, RoomStateEvent, MatrixEvent } from "ma import { KnownMembership } from "matrix-js-sdk/src/types"; import { logger } from "matrix-js-sdk/src/logger"; import { CallType } from "matrix-js-sdk/src/webrtc/call"; -import { randomString, randomLowercaseString, randomUppercaseString } from "matrix-js-sdk/src/randomstring"; +import { LOWERCASE, secureRandomString, secureRandomStringFrom } from "matrix-js-sdk/src/randomstring"; import PlatformPeg from "../PlatformPeg"; import SdkConfig from "../SdkConfig"; @@ -30,6 +30,7 @@ import { parseUrl } from "./UrlUtils"; import { useEventEmitter } from "../hooks/useEventEmitter"; import { WidgetLayoutStore } from "../stores/widgets/WidgetLayoutStore"; import { IWidgetEvent, UserWidget } from "./WidgetUtils-types"; +import { capitalize } from "lodash"; // How long we wait for the state event echo to come back from the server // before waitFor[Room/User]Widget rejects its promise @@ -427,7 +428,10 @@ export default class WidgetUtils { ): Promise { const domain = Jitsi.getInstance().preferredDomain; const auth = (await Jitsi.getInstance().getJitsiAuth()) ?? undefined; - const widgetId = randomString(24); // Must be globally unique + + // Must be globally unique, although predicatablity is not important, the js-sdk has functions to generate + // secure ranom strings, and speed is not important here. + const widgetId = secureRandomString(24); let confId: string; if (auth === "openidtoken-jwt") { @@ -437,8 +441,8 @@ export default class WidgetUtils { // https://github.com/matrix-org/prosody-mod-auth-matrix-user-verification confId = base32.stringify(new TextEncoder().encode(roomId), { pad: false }); } else { - // Create a random conference ID - confId = `Jitsi${randomUppercaseString(1)}${randomLowercaseString(23)}`; + // Create a random conference ID (capitalised so the name looks sensible in Jitsi) + confId = `Jitsi${capitalize(secureRandomStringFrom(24, LOWERCASE))}`; } // TODO: Remove URL hacks when the mobile clients eventually support v2 widgets From 1caf06b34e0fd7252e5e4c4fc459cb79d3f034a7 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 16 Jan 2025 17:03:21 +0000 Subject: [PATCH 2/3] Change remaining instances of randomString which I somehow entirely missed the first time. --- src/components/views/elements/LabelledToggleSwitch.tsx | 4 ++-- src/components/views/elements/SettingsFlag.tsx | 4 ++-- src/components/views/elements/StyledCheckbox.tsx | 4 ++-- src/components/views/messages/MBeaconBody.tsx | 6 +++--- src/components/views/messages/MLocationBody.tsx | 4 ++-- src/models/Call.ts | 4 ++-- src/rageshake/rageshake.ts | 4 ++-- src/utils/oidc/authorize.ts | 4 ++-- src/vector/platform/ElectronPlatform.tsx | 4 ++-- test/setupTests.ts | 5 +++-- test/test-utils/poll.ts | 6 +++--- .../components/views/settings/Notifications-test.tsx | 4 ++-- .../views/spaces/SpaceSettingsVisibilityTab-test.tsx | 4 ++-- test/unit-tests/utils/oidc/authorize-test.ts | 2 +- 14 files changed, 30 insertions(+), 29 deletions(-) diff --git a/src/components/views/elements/LabelledToggleSwitch.tsx b/src/components/views/elements/LabelledToggleSwitch.tsx index bbc1344efd4..1fa2e42b19d 100644 --- a/src/components/views/elements/LabelledToggleSwitch.tsx +++ b/src/components/views/elements/LabelledToggleSwitch.tsx @@ -8,7 +8,7 @@ Please see LICENSE files in the repository root for full details. import React from "react"; import classNames from "classnames"; -import { randomString } from "matrix-js-sdk/src/randomstring"; +import { secureRandomString } from "matrix-js-sdk/src/randomstring"; import ToggleSwitch from "./ToggleSwitch"; import { Caption } from "../typography/Caption"; @@ -36,7 +36,7 @@ interface IProps { } export default class LabelledToggleSwitch extends React.PureComponent { - private readonly id = `mx_LabelledToggleSwitch_${randomString(12)}`; + private readonly id = `mx_LabelledToggleSwitch_${secureRandomString(12)}`; public render(): React.ReactNode { // This is a minimal version of a SettingsFlag diff --git a/src/components/views/elements/SettingsFlag.tsx b/src/components/views/elements/SettingsFlag.tsx index f3472f38893..357fa5c9718 100644 --- a/src/components/views/elements/SettingsFlag.tsx +++ b/src/components/views/elements/SettingsFlag.tsx @@ -8,7 +8,7 @@ Please see LICENSE files in the repository root for full details. */ import React from "react"; -import { randomString } from "matrix-js-sdk/src/randomstring"; +import { secureRandomString } from "matrix-js-sdk/src/randomstring"; import SettingsStore from "../../../settings/SettingsStore"; import { _t } from "../../../languageHandler"; @@ -35,7 +35,7 @@ interface IState { } export default class SettingsFlag extends React.Component { - private readonly id = `mx_SettingsFlag_${randomString(12)}`; + private readonly id = `mx_SettingsFlag_${secureRandomString(12)}`; public constructor(props: IProps) { super(props); diff --git a/src/components/views/elements/StyledCheckbox.tsx b/src/components/views/elements/StyledCheckbox.tsx index 49d173cd7c5..8e1905924a1 100644 --- a/src/components/views/elements/StyledCheckbox.tsx +++ b/src/components/views/elements/StyledCheckbox.tsx @@ -7,7 +7,7 @@ Please see LICENSE files in the repository root for full details. */ import React, { Ref } from "react"; -import { randomString } from "matrix-js-sdk/src/randomstring"; +import { secureRandomString } from "matrix-js-sdk/src/randomstring"; import classnames from "classnames"; export enum CheckboxStyle { @@ -33,7 +33,7 @@ export default class StyledCheckbox extends React.PureComponent public constructor(props: IProps) { super(props); // 56^10 so unlikely chance of collision. - this.id = this.props.id || "checkbox_" + randomString(10); + this.id = this.props.id || "checkbox_" + secureRandomString(10); } public render(): React.ReactNode { diff --git a/src/components/views/messages/MBeaconBody.tsx b/src/components/views/messages/MBeaconBody.tsx index 43f7bbd7d34..0737f78c306 100644 --- a/src/components/views/messages/MBeaconBody.tsx +++ b/src/components/views/messages/MBeaconBody.tsx @@ -18,7 +18,7 @@ import { ContentHelpers, M_BEACON, } from "matrix-js-sdk/src/matrix"; -import { randomString } from "matrix-js-sdk/src/randomstring"; +import { secureRandomString } from "matrix-js-sdk/src/randomstring"; import classNames from "classnames"; import MatrixClientContext from "../../../contexts/MatrixClientContext"; @@ -81,10 +81,10 @@ const useBeaconState = ( // eg thread and main timeline, reply // maplibregl needs a unique id to attach the map instance to const useUniqueId = (eventId: string): string => { - const [id, setId] = useState(`${eventId}_${randomString(8)}`); + const [id, setId] = useState(`${eventId}_${secureRandomString(8)}`); useEffect(() => { - setId(`${eventId}_${randomString(8)}`); + setId(`${eventId}_${secureRandomString(8)}`); }, [eventId]); return id; diff --git a/src/components/views/messages/MLocationBody.tsx b/src/components/views/messages/MLocationBody.tsx index 006b35bb9b5..805201e4811 100644 --- a/src/components/views/messages/MLocationBody.tsx +++ b/src/components/views/messages/MLocationBody.tsx @@ -8,7 +8,7 @@ Please see LICENSE files in the repository root for full details. import React from "react"; import { MatrixEvent, ClientEvent, ClientEventHandlerMap } from "matrix-js-sdk/src/matrix"; -import { randomString } from "matrix-js-sdk/src/randomstring"; +import { secureRandomString } from "matrix-js-sdk/src/randomstring"; import { Tooltip } from "@vector-im/compound-web"; import { _t } from "../../../languageHandler"; @@ -41,7 +41,7 @@ export default class MLocationBody extends React.Component { // multiple instances of same map might be in document // eg thread and main timeline, reply - const idSuffix = `${props.mxEvent.getId()}_${randomString(8)}`; + const idSuffix = `${props.mxEvent.getId()}_${secureRandomString(8)}`; this.mapId = `mx_MLocationBody_${idSuffix}`; this.reconnectedListener = createReconnectedListener(this.clearError); diff --git a/src/models/Call.ts b/src/models/Call.ts index 03645ed47ee..b5b0d5a6ec2 100644 --- a/src/models/Call.ts +++ b/src/models/Call.ts @@ -18,7 +18,7 @@ import { } from "matrix-js-sdk/src/matrix"; import { KnownMembership, Membership } from "matrix-js-sdk/src/types"; import { logger } from "matrix-js-sdk/src/logger"; -import { randomString } from "matrix-js-sdk/src/randomstring"; +import { secureRandomString } from "matrix-js-sdk/src/randomstring"; import { CallType } from "matrix-js-sdk/src/webrtc/call"; import { NamespacedValue } from "matrix-js-sdk/src/NamespacedValue"; import { IWidgetApiRequest } from "matrix-widget-api"; @@ -743,7 +743,7 @@ export class ElementCall extends Call { const url = ElementCall.generateWidgetUrl(client, roomId); return WidgetStore.instance.addVirtualWidget( { - id: randomString(24), // So that it's globally unique + id: secureRandomString(24), // So that it's globally unique creatorUserId: client.getUserId()!, name: "Element Call", type: WidgetType.CALL.preferred, diff --git a/src/rageshake/rageshake.ts b/src/rageshake/rageshake.ts index 2d8ec572d5a..fbfedf0e4c1 100644 --- a/src/rageshake/rageshake.ts +++ b/src/rageshake/rageshake.ts @@ -31,7 +31,7 @@ Please see LICENSE files in the repository root for full details. // the frequency with which we flush to indexeddb import { logger } from "matrix-js-sdk/src/logger"; -import { randomString } from "matrix-js-sdk/src/randomstring"; +import { secureRandomString } from "matrix-js-sdk/src/randomstring"; import { getCircularReplacer } from "../utils/JSON"; @@ -135,7 +135,7 @@ export class IndexedDBLogStore { private indexedDB: IDBFactory, private logger: ConsoleLogger, ) { - this.id = "instance-" + randomString(16); + this.id = "instance-" + secureRandomString(16); } /** diff --git a/src/utils/oidc/authorize.ts b/src/utils/oidc/authorize.ts index 50c9e07228b..3f39683cdfe 100644 --- a/src/utils/oidc/authorize.ts +++ b/src/utils/oidc/authorize.ts @@ -9,7 +9,7 @@ Please see LICENSE files in the repository root for full details. import { completeAuthorizationCodeGrant, generateOidcAuthorizationUrl } from "matrix-js-sdk/src/oidc/authorize"; import { QueryDict } from "matrix-js-sdk/src/utils"; import { OidcClientConfig } from "matrix-js-sdk/src/matrix"; -import { randomString } from "matrix-js-sdk/src/randomstring"; +import { secureRandomString } from "matrix-js-sdk/src/randomstring"; import { IdTokenClaims } from "oidc-client-ts"; import { OidcClientError } from "./error"; @@ -34,7 +34,7 @@ export const startOidcLogin = async ( ): Promise => { const redirectUri = PlatformPeg.get()!.getOidcCallbackUrl().href; - const nonce = randomString(10); + const nonce = secureRandomString(10); const prompt = isRegistration ? "create" : undefined; diff --git a/src/vector/platform/ElectronPlatform.tsx b/src/vector/platform/ElectronPlatform.tsx index 6a6409ea42f..252db399c99 100644 --- a/src/vector/platform/ElectronPlatform.tsx +++ b/src/vector/platform/ElectronPlatform.tsx @@ -12,7 +12,7 @@ Please see LICENSE files in the repository root for full details. import { MatrixClient, Room, MatrixEvent, OidcRegistrationClientMetadata } from "matrix-js-sdk/src/matrix"; import React from "react"; -import { randomString } from "matrix-js-sdk/src/randomstring"; +import { secureRandomString } from "matrix-js-sdk/src/randomstring"; import { logger } from "matrix-js-sdk/src/logger"; import BasePlatform, { UpdateCheckStatus, UpdateStatus } from "../../BasePlatform"; @@ -93,7 +93,7 @@ export default class ElectronPlatform extends BasePlatform { private readonly ipc = new IPCManager("ipcCall", "ipcReply"); private readonly eventIndexManager: BaseEventIndexManager = new SeshatIndexManager(); // this is the opaque token we pass to the HS which when we get it in our callback we can resolve to a profile - private readonly ssoID: string = randomString(32); + private readonly ssoID: string = secureRandomString(32); public constructor() { super(); diff --git a/test/setupTests.ts b/test/setupTests.ts index 5d1740f5f49..144f27e15af 100644 --- a/test/setupTests.ts +++ b/test/setupTests.ts @@ -8,7 +8,7 @@ Please see LICENSE files in the repository root for full details. import "@testing-library/jest-dom"; import "blob-polyfill"; -import { randomString } from "matrix-js-sdk/src/randomstring"; +import { secureRandomString } from "matrix-js-sdk/src/randomstring"; import { mocked } from "jest-mock"; import { PredictableRandom } from "./test-utils/predictableRandom"; // https://github.com/jsdom/jsdom/issues/2555 @@ -25,7 +25,8 @@ jest.mock("matrix-js-sdk/src/randomstring"); beforeEach(() => { const chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"; const mockRandom = new PredictableRandom(); - mocked(randomString).mockImplementation((len) => { + // needless to say, the mock is not cryptographically secure + mocked(secureRandomString).mockImplementation((len) => { let ret = ""; for (let i = 0; i < len; ++i) { const v = mockRandom.get() * chars.length; diff --git a/test/test-utils/poll.ts b/test/test-utils/poll.ts index b7fea582599..13003ea5288 100644 --- a/test/test-utils/poll.ts +++ b/test/test-utils/poll.ts @@ -18,7 +18,7 @@ import { M_POLL_RESPONSE, M_TEXT, } from "matrix-js-sdk/src/matrix"; -import { randomString } from "matrix-js-sdk/src/randomstring"; +import { secureRandomString } from "matrix-js-sdk/src/randomstring"; import { flushPromises } from "./utilities"; @@ -67,7 +67,7 @@ export const makePollEndEvent = ( id?: string, ): MatrixEvent => { return new MatrixEvent({ - event_id: id || randomString(16), + event_id: id || secureRandomString(16), room_id: roomId, origin_server_ts: ts, type: M_POLL_END.name, @@ -91,7 +91,7 @@ export const makePollResponseEvent = ( ts = 0, ): MatrixEvent => new MatrixEvent({ - event_id: randomString(16), + event_id: secureRandomString(16), room_id: roomId, origin_server_ts: ts, type: M_POLL_RESPONSE.name, diff --git a/test/unit-tests/components/views/settings/Notifications-test.tsx b/test/unit-tests/components/views/settings/Notifications-test.tsx index c4b893920df..d004fa5efdd 100644 --- a/test/unit-tests/components/views/settings/Notifications-test.tsx +++ b/test/unit-tests/components/views/settings/Notifications-test.tsx @@ -23,7 +23,7 @@ import { IThreepid, ThreepidMedium, } from "matrix-js-sdk/src/matrix"; -import { randomString } from "matrix-js-sdk/src/randomstring"; +import { secureRandomString } from "matrix-js-sdk/src/randomstring"; import { act, fireEvent, @@ -287,7 +287,7 @@ describe("", () => { beforeEach(async () => { let i = 0; - mocked(randomString).mockImplementation(() => { + mocked(secureRandomString).mockImplementation(() => { return "testid_" + i++; }); diff --git a/test/unit-tests/components/views/spaces/SpaceSettingsVisibilityTab-test.tsx b/test/unit-tests/components/views/spaces/SpaceSettingsVisibilityTab-test.tsx index f16651abd93..3891e1eda57 100644 --- a/test/unit-tests/components/views/spaces/SpaceSettingsVisibilityTab-test.tsx +++ b/test/unit-tests/components/views/spaces/SpaceSettingsVisibilityTab-test.tsx @@ -8,7 +8,7 @@ Please see LICENSE files in the repository root for full details. import React from "react"; import { mocked } from "jest-mock"; -import { randomString } from "matrix-js-sdk/src/randomstring"; +import { secureRandomString } from "matrix-js-sdk/src/randomstring"; import { act, fireEvent, render, RenderResult } from "jest-matrix-react"; import { EventType, MatrixClient, Room, GuestAccess, HistoryVisibility, JoinRule } from "matrix-js-sdk/src/matrix"; @@ -92,7 +92,7 @@ describe("", () => { beforeEach(() => { let i = 0; - mocked(randomString).mockImplementation(() => { + mocked(secureRandomString).mockImplementation(() => { return "testid_" + i++; }); diff --git a/test/unit-tests/utils/oidc/authorize-test.ts b/test/unit-tests/utils/oidc/authorize-test.ts index 72c42fab370..bea9724fce3 100644 --- a/test/unit-tests/utils/oidc/authorize-test.ts +++ b/test/unit-tests/utils/oidc/authorize-test.ts @@ -49,7 +49,7 @@ describe("OIDC authorization", () => { origin: baseUrl, }; - jest.spyOn(randomStringUtils, "randomString").mockRestore(); + jest.spyOn(randomStringUtils, "secureRandomString").mockRestore(); mockPlatformPeg(); Object.defineProperty(window, "crypto", { value: { From eeb98aa9ac779b66a2b15583ab0e3a57d5dea457 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 16 Jan 2025 17:11:41 +0000 Subject: [PATCH 3/3] Fix import order --- src/utils/WidgetUtils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/WidgetUtils.ts b/src/utils/WidgetUtils.ts index 7ff1886511c..617ee351013 100644 --- a/src/utils/WidgetUtils.ts +++ b/src/utils/WidgetUtils.ts @@ -9,6 +9,7 @@ Please see LICENSE files in the repository root for full details. import { useCallback, useEffect, useState } from "react"; import { base32 } from "rfc4648"; +import { capitalize } from "lodash"; import { IWidget, IWidgetData } from "matrix-widget-api"; import { Room, ClientEvent, MatrixClient, RoomStateEvent, MatrixEvent } from "matrix-js-sdk/src/matrix"; import { KnownMembership } from "matrix-js-sdk/src/types"; @@ -30,7 +31,6 @@ import { parseUrl } from "./UrlUtils"; import { useEventEmitter } from "../hooks/useEventEmitter"; import { WidgetLayoutStore } from "../stores/widgets/WidgetLayoutStore"; import { IWidgetEvent, UserWidget } from "./WidgetUtils-types"; -import { capitalize } from "lodash"; // How long we wait for the state event echo to come back from the server // before waitFor[Room/User]Widget rejects its promise