Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add sizing limits for custom UI #2199

Merged
merged 5 commits into from
Feb 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/snaps-controllers/coverage.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"branches": 91.01,
"functions": 96.56,
"lines": 97.79,
"statements": 97.45
"lines": 97.8,
"statements": 97.46
}
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,52 @@ describe('SnapInterfaceController', () => {
'foo.bar',
);
});

it('throws if UI content is too large', async () => {
const rootMessenger = getRootSnapInterfaceControllerMessenger();
const controllerMessenger = getRestrictedSnapInterfaceControllerMessenger(
rootMessenger,
false,
);

/* eslint-disable-next-line no-new */
new SnapInterfaceController({
messenger: controllerMessenger,
});

const components = panel([text('[foo](https://foo.bar)'.repeat(100000))]);

await expect(
rootMessenger.call(
'SnapInterfaceController:createInterface',
MOCK_SNAP_ID,
components,
),
).rejects.toThrow('A Snap UI may not be larger than 250 kB.');
});

it('throws if text content is too large', async () => {
const rootMessenger = getRootSnapInterfaceControllerMessenger();
const controllerMessenger = getRestrictedSnapInterfaceControllerMessenger(
rootMessenger,
false,
);

/* eslint-disable-next-line no-new */
new SnapInterfaceController({
messenger: controllerMessenger,
});

const components = panel([text('[foo](https://foo.bar)'.repeat(2500))]);

await expect(
rootMessenger.call(
'SnapInterfaceController:createInterface',
MOCK_SNAP_ID,
components,
),
).rejects.toThrow('The text in a Snap UI may not be larger than 50 kB.');
});
});

describe('getInterface', () => {
Expand Down Expand Up @@ -292,6 +338,72 @@ describe('SnapInterfaceController', () => {
);
});

it('throws if UI content is too large', async () => {
const rootMessenger = getRootSnapInterfaceControllerMessenger();
const controllerMessenger =
getRestrictedSnapInterfaceControllerMessenger(rootMessenger);

/* eslint-disable-next-line no-new */
new SnapInterfaceController({
messenger: controllerMessenger,
});

const components = form({
name: 'foo',
children: [input({ name: 'bar' })],
});

const newContent = panel([text('[foo](https://foo.bar)'.repeat(100000))]);

const id = await rootMessenger.call(
'SnapInterfaceController:createInterface',
MOCK_SNAP_ID,
components,
);

await expect(
rootMessenger.call(
'SnapInterfaceController:updateInterface',
MOCK_SNAP_ID,
id,
newContent,
),
).rejects.toThrow('A Snap UI may not be larger than 250 kB.');
});

it('throws if text content is too large', async () => {
const rootMessenger = getRootSnapInterfaceControllerMessenger();
const controllerMessenger =
getRestrictedSnapInterfaceControllerMessenger(rootMessenger);

/* eslint-disable-next-line no-new */
new SnapInterfaceController({
messenger: controllerMessenger,
});

const components = form({
name: 'foo',
children: [input({ name: 'bar' })],
});

const newContent = panel([text('[foo](https://foo.bar)'.repeat(2500))]);

const id = await rootMessenger.call(
'SnapInterfaceController:createInterface',
MOCK_SNAP_ID,
components,
);

await expect(
rootMessenger.call(
'SnapInterfaceController:updateInterface',
MOCK_SNAP_ID,
id,
newContent,
),
).rejects.toThrow('The text in a Snap UI may not be larger than 50 kB.');
});

it('throws if the interface does not exist', async () => {
const rootMessenger = getRootSnapInterfaceControllerMessenger();
const controllerMessenger =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,18 @@ import type {
TestOrigin,
} from '@metamask/phishing-controller';
import type { Component, InterfaceState, SnapId } from '@metamask/snaps-sdk';
import { validateComponentLinks } from '@metamask/snaps-utils';
import { assert } from '@metamask/utils';
import {
getTotalTextLength,
validateComponentLinks,
} from '@metamask/snaps-utils';
import { assert, getJsonSize } from '@metamask/utils';
import { nanoid } from 'nanoid';

import { constructState } from './utils';

const MAX_UI_CONTENT_SIZE = 250_000; // 250 kb
const MAX_TEXT_LENGTH = 50_000; // 50 kb

const controllerName = 'SnapInterfaceController';

export type CreateInterface = {
Expand Down Expand Up @@ -252,6 +258,22 @@ export class SnapInterfaceController extends BaseController<
* @param content - The components to verify.
*/
async #validateContent(content: Component) {
const size = getJsonSize(content);

assert(
size <= MAX_UI_CONTENT_SIZE,
`A Snap UI may not be larger than ${MAX_UI_CONTENT_SIZE / 1000} kB.`,
);

const textSize = getTotalTextLength(content);

assert(
textSize <= MAX_TEXT_LENGTH,
`The text in a Snap UI may not be larger than ${
MAX_TEXT_LENGTH / 1000
} kB.`,
);

await this.#triggerPhishingListUpdate();

validateComponentLinks(content, this.#checkPhishingList.bind(this));
Expand Down
8 changes: 4 additions & 4 deletions packages/snaps-utils/coverage.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"branches": 96.4,
"functions": 98.62,
"lines": 98.73,
"statements": 94.45
"branches": 96.44,
"functions": 98.63,
"lines": 98.74,
"statements": 94.49
}
32 changes: 30 additions & 2 deletions packages/snaps-utils/src/ui.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { panel, text, row, address } from '@metamask/snaps-sdk';
import { panel, text, row, address, image } from '@metamask/snaps-sdk';

import { validateTextLinks, validateComponentLinks } from './ui';
import {
validateTextLinks,
validateComponentLinks,
getTotalTextLength,
} from './ui';

describe('validateTextLinks', () => {
it('passes for valid links', () => {
Expand Down Expand Up @@ -85,3 +89,27 @@ describe('validateComponentLinks', () => {
).toThrow('Invalid URL: The specified URL is not allowed.');
});
});

describe('getTotalTextLength', () => {
it('calculates total length', () => {
expect(getTotalTextLength(text('foo'))).toBe(3);
});

it('calculates total length for nested text', () => {
expect(
getTotalTextLength(
panel([text('foo'), panel([text('bar'), text('baz')])]),
),
).toBe(9);
});

it('calculates total length for nested text in rows', () => {
expect(
getTotalTextLength(panel([row('1', text('foo')), row('2', text('bar'))])),
).toBe(6);
});

it('ignores non text components', () => {
expect(getTotalTextLength(panel([text('foo'), image('<svg />')]))).toBe(3);
});
});
26 changes: 26 additions & 0 deletions packages/snaps-utils/src/ui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,29 @@ export function validateComponentLinks(
break;
}
}

/**
* Calculate the total length of all text in the component.
*
* @param component - A custom UI component.
* @returns The total length of all text components in the component.
*/
export function getTotalTextLength(component: Component): number {
const { type } = component;

switch (type) {
case NodeType.Panel:
return component.children.reduce<number>(
// This is a bug in TypeScript: https://github.com/microsoft/TypeScript/issues/48313
// eslint-disable-next-line @typescript-eslint/restrict-plus-operands
(sum, node) => sum + getTotalTextLength(node),
0,
);
case NodeType.Row:
return getTotalTextLength(component.value);
case NodeType.Text:
return component.value.length;
default:
return 0;
}
}
Loading