Skip to content

Commit a210744

Browse files
committed
fix: replace Math.random usage by uuid for better security
1 parent 8bbcb66 commit a210744

File tree

32 files changed

+821
-42
lines changed

32 files changed

+821
-42
lines changed

.changeset/mighty-pigs-perform.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
---
2+
"@ledgerhq/coin-solana": minor
3+
"ledger-live-desktop": minor
4+
"live-mobile": minor
5+
"@ledgerhq/live-common": minor
6+
"@ledgerhq/ethereum-provider": minor
7+
"@ledgerhq/web-tools": minor
8+
"ledger-live-mobile-e2e-tests": minor
9+
---
10+
11+
Replace usages of Math.random() for id generation by uuid()
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
import { ipcRenderer } from "electron";
2+
import { DeviceModelId } from "@ledgerhq/types-devices";
3+
import IPCTransport from "./IPCTransport";
4+
5+
jest.mock("electron", () => ({
6+
ipcRenderer: {
7+
invoke: jest.fn(),
8+
},
9+
}));
10+
11+
jest.mock("@ledgerhq/logs", () => {
12+
const mockInstance = {
13+
trace: jest.fn(),
14+
withContext: jest.fn().mockReturnThis(),
15+
withType: jest.fn().mockReturnThis(),
16+
};
17+
return {
18+
log: jest.fn(),
19+
trace: jest.fn(),
20+
LocalTracer: jest.fn().mockImplementation(() => mockInstance),
21+
};
22+
});
23+
24+
jest.mock("@ledgerhq/devices", () => ({
25+
getDeviceModel: jest.fn(() => ({ id: DeviceModelId.nanoS })),
26+
}));
27+
28+
const mockInvoke = jest.mocked(ipcRenderer.invoke);
29+
30+
describe("IPCTransport", () => {
31+
beforeEach(() => {
32+
mockInvoke.mockReset();
33+
});
34+
35+
describe("isSupported", () => {
36+
it("should resolve to true when ipcRenderer is an object", async () => {
37+
await expect(IPCTransport.isSupported()).resolves.toBe(true);
38+
});
39+
});
40+
41+
describe("listen", () => {
42+
it("should call transport:listen with a uuid requestId and emit add descriptor on success", () => {
43+
const observer = { next: jest.fn(), error: jest.fn(), complete: jest.fn() };
44+
mockInvoke.mockImplementation((channel: string) => {
45+
if (channel === "transport:listen") {
46+
return Promise.resolve({ type: "listen-success" });
47+
}
48+
return Promise.resolve(undefined);
49+
});
50+
51+
const subscription = IPCTransport.listen(observer);
52+
53+
expect(mockInvoke).toHaveBeenCalledWith(
54+
"transport:listen",
55+
expect.objectContaining({
56+
requestId: expect.stringMatching(
57+
/^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i,
58+
),
59+
}),
60+
);
61+
62+
return new Promise<void>(resolve => {
63+
setImmediate(() => {
64+
expect(observer.next).toHaveBeenCalledWith(
65+
expect.objectContaining({
66+
type: "add",
67+
descriptor: "http-proxy",
68+
}),
69+
);
70+
subscription.unsubscribe();
71+
resolve();
72+
});
73+
});
74+
});
75+
});
76+
77+
describe("open", () => {
78+
it("should call transport:open with descriptor and uuid requestId and return IPCTransport instance", async () => {
79+
const descriptor = "http-proxy";
80+
mockInvoke.mockImplementation((channel: string) => {
81+
if (channel === "transport:open") return Promise.resolve({ type: "open-success" });
82+
return Promise.resolve(undefined);
83+
});
84+
85+
const transport = await IPCTransport.open(descriptor);
86+
87+
expect(transport).toBeInstanceOf(IPCTransport);
88+
const openCall = mockInvoke.mock.calls.find(c => c[0] === "transport:open");
89+
expect(openCall).toBeDefined();
90+
expect(openCall![1]).toMatchObject({
91+
descriptor,
92+
});
93+
expect(openCall![1].requestId).toMatch(
94+
/^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i,
95+
);
96+
});
97+
});
98+
});

apps/ledger-live-desktop/src/renderer/IPCTransport.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { log, trace, TraceContext } from "@ledgerhq/logs";
88
import { DescriptorEvent, DeviceModelId } from "@ledgerhq/types-devices";
99
import { Observer } from "rxjs";
1010
import { getDeviceModel } from "@ledgerhq/devices";
11+
import { v4 as uuid } from "uuid";
1112
// No longer need transport channels - using direct invoke
1213

1314
const LOG_TYPE = "hid-ipc";
@@ -25,7 +26,7 @@ export default class IPCTransport extends Transport {
2526

2627
static listen = (observer: Observer<DescriptorEvent<string>>) => {
2728
let unsubscribed = false;
28-
const requestId = String(Math.random());
29+
const requestId = uuid();
2930

3031
const unsubscribe = () => {
3132
if (unsubscribed) return;
@@ -68,7 +69,7 @@ export default class IPCTransport extends Transport {
6869
): Promise<IPCTransport> {
6970
log(LOG_TYPE, "open", { descriptor, timeout });
7071

71-
const requestId = String(Math.random());
72+
const requestId = uuid();
7273

7374
try {
7475
const result = await ipcRenderer.invoke("transport:open", {

apps/ledger-live-mobile/e2e/bridge/server.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
SettingsSetOverriddenFeatureFlagPlayload,
1616
SettingsSetOverriddenFeatureFlagsPlayload,
1717
} from "~/actions/types";
18+
import { v4 as uuid } from "uuid";
1819

1920
let clientResponse: (data: string) => void;
2021
const RESPONSE_TIMEOUT = 10000;
@@ -45,9 +46,7 @@ export async function findFreePort(): Promise<number> {
4546
}
4647

4748
function uniqueId(): string {
48-
const timestamp = Date.now().toString(36); // Convert timestamp to base36 string
49-
const randomString = Math.random().toString(36).slice(2, 7); // Generate random string
50-
return timestamp + randomString; // Concatenate timestamp and random string
49+
return uuid();
5150
}
5251

5352
function isFeatureId(

apps/ledger-live-mobile/src/mvvm/components/ProviderInterstitial/index.tsx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { Icon, Text, Flex } from "@ledgerhq/native-ui";
1616
import { useShowProviderLoadingTransition } from "@ledgerhq/live-common/hooks/useShowProviderLoadingTransition";
1717
import { InterstitialType } from "~/components/WebPTXPlayer";
1818
import { DdRum, RumActionType } from "@datadog/mobile-react-native";
19+
import { v4 as uuid } from "uuid";
1920

2021
export const Loader = styled(Flex)`
2122
gap: 28px;
@@ -197,7 +198,5 @@ function useProviderConnectRum({
197198
}
198199

199200
function uniqueId(): string {
200-
const timestamp = Date.now().toString(36);
201-
const randomString = Math.random().toString(36).slice(2, 7);
202-
return timestamp + randomString;
201+
return uuid();
203202
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import { generateCallbackId } from "../callbackIdGenerator";
2+
3+
describe("callbackIdGenerator", () => {
4+
it("returns a string prefixed with callback_", () => {
5+
const id = generateCallbackId();
6+
expect(id).toMatch(/^callback_.*$/);
7+
expect(typeof id).toBe("string");
8+
});
9+
10+
it("returns unique ids on each call", () => {
11+
const id1 = generateCallbackId();
12+
const id2 = generateCallbackId();
13+
expect(id1).not.toBe(id2);
14+
});
15+
});
Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
1+
import { v4 as uuid } from "uuid";
2+
13
/**
24
* Generates a unique callback ID for the callback registry.
35
* This ensures each callback has a unique identifier to avoid conflicts.
46
*/
57
export const generateCallbackId = (): string => {
6-
return `callback_${Date.now()}_${Math.random().toString(36).substring(2, 9)}`;
8+
return `callback_${uuid()}`;
79
};

apps/ledger-live-mobile/src/screens/Settings/Debug/Generators/GenerateMockAccounts.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import React from "react";
22
import sample from "lodash/sample";
33
import { Alert } from "react-native";
44
import { IconsLegacy } from "@ledgerhq/native-ui";
5+
import { v4 as uuid } from "uuid";
56
import { genAccount } from "@ledgerhq/live-common/mock/account";
67
import { listSupportedCurrencies } from "@ledgerhq/live-common/currencies/index";
78
import SettingsRow from "~/components/SettingsRow";
@@ -13,7 +14,7 @@ const generateMockAccounts = (count: number) =>
1314
Array(count)
1415
.fill(null)
1516
.map(() => {
16-
return genAccount(String(Math.random()), {
17+
return genAccount(uuid(), {
1718
currency: sample(listSupportedCurrencies()),
1819
});
1920
});

apps/ledger-live-mobile/src/screens/Settings/Debug/Generators/GenerateMockAccountsSelect.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import React, { useCallback, useState } from "react";
2+
import { v4 as uuid } from "uuid";
23
import { genAccount } from "@ledgerhq/live-common/mock/account";
34
import { listSupportedCurrencies } from "@ledgerhq/live-common/currencies/index";
45
import { useNavigation } from "@react-navigation/native";
@@ -29,7 +30,7 @@ const generateMockAccounts = (currencies: CryptoCurrency[], tokens: string): Acc
2930
const tokenIds = tokens.split(",").map(t => t.toLowerCase().trim());
3031

3132
return currencies.map(currency =>
32-
genAccount(String(Math.random()), {
33+
genAccount(uuid(), {
3334
currency,
3435
tokenIds,
3536
}),
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import React from "react";
2+
import { Alert } from "react-native";
3+
import { render, screen } from "@tests/test-renderer";
4+
import GenerateMockAccountsButton from "../GenerateMockAccounts";
5+
6+
jest.mock("uuid", () => ({
7+
v4: jest.fn(() => "mock-uuid-for-accounts"),
8+
}));
9+
10+
const mockGenAccount = jest.fn(() => ({ id: "account-1" }));
11+
jest.mock("@ledgerhq/live-common/mock/account", () => ({
12+
genAccount: (...args: unknown[]) => Reflect.apply(mockGenAccount, null, args),
13+
}));
14+
15+
const mockDispatch = jest.fn();
16+
jest.mock("~/context/hooks", () => ({
17+
...jest.requireActual("~/context/hooks"),
18+
useDispatch: () => mockDispatch,
19+
}));
20+
21+
jest.mock("~/actions/accounts", () => ({
22+
replaceAccounts: jest.fn((accounts: unknown) => ({ type: "REPLACE_ACCOUNTS", accounts })),
23+
}));
24+
25+
jest.mock("~/actions/appstate", () => ({
26+
reboot: jest.fn(() => ({ type: "REBOOT" })),
27+
}));
28+
29+
describe("GenerateMockAccountsButton", () => {
30+
let alertSpy: jest.SpyInstance;
31+
32+
beforeEach(() => {
33+
jest.clearAllMocks();
34+
alertSpy = jest
35+
.spyOn(Alert, "alert")
36+
.mockImplementation(
37+
(_title: string, _message?: string, buttons?: Array<{ text?: string; onPress?: () => void }>) => {
38+
const okButton = buttons?.find(b => b.text === "Ok");
39+
if (okButton?.onPress) okButton.onPress();
40+
},
41+
);
42+
});
43+
44+
afterEach(() => {
45+
alertSpy.mockRestore();
46+
});
47+
48+
it("should call genAccount with uuid when generating mock accounts", async () => {
49+
const { user } = render(
50+
<GenerateMockAccountsButton count={2} title="Generate" desc="Description" />,
51+
);
52+
53+
const row = await screen.findByText("Generate");
54+
await user.press(row);
55+
56+
expect(mockGenAccount).toHaveBeenCalled();
57+
expect(
58+
mockGenAccount.mock.calls.every((call: unknown[]) => call[0] === "mock-uuid-for-accounts"),
59+
).toBe(true);
60+
});
61+
62+
it("should pass a uuid-shaped seed to genAccount for each account", async () => {
63+
const { user } = render(
64+
<GenerateMockAccountsButton count={3} title="Generate" desc="Description" />,
65+
);
66+
67+
const row = await screen.findByText("Generate");
68+
await user.press(row);
69+
70+
const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i;
71+
expect(mockGenAccount).toHaveBeenCalledTimes(3);
72+
mockGenAccount.mock.calls.forEach((call: unknown[]) => {
73+
expect(String(call[0])).toMatch(uuidRegex);
74+
});
75+
});
76+
});

0 commit comments

Comments
 (0)