Skip to content

Commit

Permalink
Switch to secure random strings (#29013)
Browse files Browse the repository at this point in the history
* 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 matrix-org/matrix-js-sdk#4621 (merge both together)

* Change remaining instances of randomString

which I somehow entirely missed the first time.

* Fix import order
  • Loading branch information
dbkr authored Jan 21, 2025
1 parent 1644169 commit 56eafc9
Show file tree
Hide file tree
Showing 15 changed files with 38 additions and 33 deletions.
4 changes: 2 additions & 2 deletions src/components/views/elements/LabelledToggleSwitch.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/components/views/elements/SettingsFlag.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/components/views/elements/StyledCheckbox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions src/components/views/messages/MBeaconBody.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/components/views/messages/MLocationBody.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/models/Call.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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,
Expand Down
4 changes: 2 additions & 2 deletions src/rageshake/rageshake.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -135,7 +135,7 @@ export class IndexedDBLogStore {
private indexedDB: IDBFactory,
private logger: ConsoleLogger,
) {
this.id = "instance-" + randomString(16);
this.id = "instance-" + secureRandomString(16);
}

/**
Expand Down
12 changes: 8 additions & 4 deletions src/utils/WidgetUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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") {
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/utils/oidc/authorize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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;

Expand Down
4 changes: 2 additions & 2 deletions src/vector/platform/ElectronPlatform.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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();
Expand Down
5 changes: 3 additions & 2 deletions test/setupTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand Down
6 changes: 3 additions & 3 deletions test/test-utils/poll.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -287,7 +287,7 @@ describe("<Notifications />", () => {

beforeEach(async () => {
let i = 0;
mocked(randomString).mockImplementation(() => {
mocked(secureRandomString).mockImplementation(() => {
return "testid_" + i++;
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -92,7 +92,7 @@ describe("<SpaceSettingsVisibilityTab />", () => {

beforeEach(() => {
let i = 0;
mocked(randomString).mockImplementation(() => {
mocked(secureRandomString).mockImplementation(() => {
return "testid_" + i++;
});

Expand Down
2 changes: 1 addition & 1 deletion test/unit-tests/utils/oidc/authorize-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down

0 comments on commit 56eafc9

Please sign in to comment.