From 1e1d66924f05113778c4a5539491656fb8342981 Mon Sep 17 00:00:00 2001
From: ElementRobot <releases@riot.im>
Date: Tue, 21 Jan 2025 16:15:44 +0100
Subject: [PATCH] Switch to secure random strings (#29013) (#29035)

* 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)

* Change remaining instances of randomString

which I somehow entirely missed the first time.

* Fix import order

(cherry picked from commit 56eafc908e84d5bb015a06a26f006243bd584014)

Co-authored-by: David Baker <dbkr@users.noreply.github.com>
---
 .../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/WidgetUtils.ts                             | 12 ++++++++----
 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 +-
 15 files changed, 38 insertions(+), 33 deletions(-)

diff --git a/src/components/views/elements/LabelledToggleSwitch.tsx b/src/components/views/elements/LabelledToggleSwitch.tsx
index bbc1344efd..1fa2e42b19 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<IProps> {
-    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 f3472f3889..357fa5c971 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<IProps, IState> {
-    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 49d173cd7c..8e1905924a 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<IProps, IState>
     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 43f7bbd7d3..0737f78c30 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 006b35bb9b..805201e481 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<IBodyProps, IState> {
 
         // 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 03645ed47e..b5b0d5a6ec 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 2d8ec572d5..fbfedf0e4c 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/WidgetUtils.ts b/src/utils/WidgetUtils.ts
index d9de59e401..617ee35101 100644
--- a/src/utils/WidgetUtils.ts
+++ b/src/utils/WidgetUtils.ts
@@ -9,12 +9,13 @@ 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";
 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";
@@ -427,7 +428,10 @@ export default class WidgetUtils {
     ): Promise<void> {
         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
diff --git a/src/utils/oidc/authorize.ts b/src/utils/oidc/authorize.ts
index 50c9e07228..3f39683cdf 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<void> => {
     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 6a6409ea42..252db399c9 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 5d1740f5f4..144f27e15a 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 b7fea58259..13003ea528 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 c4b893920d..d004fa5efd 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("<Notifications />", () => {
 
     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 f16651abd9..3891e1eda5 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("<SpaceSettingsVisibilityTab />", () => {
 
     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 72c42fab37..bea9724fce 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: {
-- 
GitLab