diff --git a/src/SnapKeyring.test.ts b/src/SnapKeyring.test.ts index febd48fc..a2c1a5f0 100644 --- a/src/SnapKeyring.test.ts +++ b/src/SnapKeyring.test.ts @@ -316,6 +316,19 @@ describe('SnapKeyring', () => { ])('returns a redirect %s', async (redirect) => { const spy = jest.spyOn(console, 'log').mockImplementation(); + const snapObject = { + id: snapId, + manifest: { + initialPermissions: { + 'endowment:keyring': { + allowedOrigins: ['https://example.com'], + }, + }, + }, + enabled: true, + }; + mockSnapController.get.mockReturnValue(snapObject); + mockSnapController.handleRequest.mockResolvedValue({ pending: true, redirect, @@ -353,6 +366,76 @@ describe('SnapKeyring', () => { spy.mockRestore(); }); + describe('async request redirect url', () => { + const isNotAllowedOrigin = async ( + allowedOrigins: string[], + redirectUrl: string, + ) => { + const { origin } = new URL(redirectUrl); + const snapObject = { + id: snapId, + manifest: { + initialPermissions: + allowedOrigins.length > 0 + ? { 'endowment:keyring': { allowedOrigins } } + : {}, + }, + enabled: true, + }; + mockSnapController.get.mockReturnValue(snapObject); + mockSnapController.handleRequest.mockResolvedValue({ + pending: true, + redirect: { + message: 'Go to dapp to continue.', + url: redirectUrl, + }, + }); + const requestPromise = keyring.signPersonalMessage( + accounts[0].address, + 'hello', + ); + + await expect(requestPromise).rejects.toThrow( + `Redirect URL domain '${origin}' is not an allowed origin by snap '${snapId}'`, + ); + }; + + it('throws an error if async request redirect url is not an allowed origin', async () => { + expect.hasAssertions(); + await isNotAllowedOrigin( + ['https://allowed.com'], + 'https://notallowed.com/sign?tx=1234', + ); + }); + + it('throws an error if no allowed origins', async () => { + expect.hasAssertions(); + await isNotAllowedOrigin([], 'https://example.com/sign?tx=1234'); + }); + + it('throws an error if the snap is undefined', async () => { + const redirect = { + message: 'Go to dapp to continue.', + url: 'https://example.com/sign?tx=1234', + }; + + mockSnapController.get.mockReturnValue(undefined); + + mockSnapController.handleRequest.mockResolvedValue({ + pending: true, + redirect, + }); + const requestPromise = keyring.signPersonalMessage( + accounts[0].address, + 'hello', + ); + + await expect(requestPromise).rejects.toThrow( + `Snap '${snapId}' not found.`, + ); + }); + }); + it('rejects an async request', async () => { mockSnapController.handleRequest.mockResolvedValue({ pending: true, diff --git a/src/SnapKeyring.ts b/src/SnapKeyring.ts index 5e72c60d..233d3143 100644 --- a/src/SnapKeyring.ts +++ b/src/SnapKeyring.ts @@ -24,6 +24,7 @@ import { } from '@metamask/keyring-api'; import type { SnapController } from '@metamask/snaps-controllers'; import type { SnapId } from '@metamask/snaps-sdk'; +import type { Snap } from '@metamask/snaps-utils'; import type { Json } from '@metamask/utils'; import { bigIntToHex } from '@metamask/utils'; import { EventEmitter } from 'events'; @@ -436,6 +437,22 @@ export class SnapKeyring extends EventEmitter { // If the snap answers asynchronously, we will inform the user with a redirect if (response.redirect?.message || response.redirect?.url) { const { message = '', url = '' } = response.redirect; + + // Check redirect url domain is in the snap allowed origins + if (url) { + const { origin } = new URL(url); + const snap = this.#snapClient.getController().get(snapId); + if (!snap) { + throw new Error(`Snap '${snapId}' not found.`); + } + const allowedOrigins = this.#getSnapAllowedOrigins(snap); + if (!allowedOrigins.includes(origin)) { + throw new Error( + `Redirect URL domain '${origin}' is not an allowed origin by snap '${snapId}'`, + ); + } + } + await this.#callbacks.redirectUser(snapId, url, message); } @@ -695,6 +712,19 @@ export class SnapKeyring extends EventEmitter { : undefined; } + /** + * Get the allowed origins of a snap. + * + * @param snap - Snap. + * @returns The allowed origins of the snap. + */ + #getSnapAllowedOrigins(snap: Snap): string[] { + return ( + snap.manifest.initialPermissions['endowment:keyring']?.allowedOrigins ?? + [] + ); + } + /** * Return an internal account object for a given address. *