Skip to content

Commit

Permalink
Merge branch 'develop' into getWeiHexFromDecimalValue-error
Browse files Browse the repository at this point in the history
  • Loading branch information
darkwing authored Oct 12, 2023
2 parents f91683a + 57d3212 commit d5c8a65
Show file tree
Hide file tree
Showing 32 changed files with 1,051 additions and 33 deletions.
12 changes: 12 additions & 0 deletions app/_locales/en/messages.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions app/scripts/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -806,9 +806,8 @@ export function setupController(
///: END:ONLY_INCLUDE_IN
///: BEGIN:ONLY_INCLUDE_IN(keyring-snaps)
case SNAP_MANAGE_ACCOUNTS_CONFIRMATION_TYPES.confirmAccountCreation:
controller.approvalController.accept(id, false);
break;
case SNAP_MANAGE_ACCOUNTS_CONFIRMATION_TYPES.confirmAccountRemoval:
case SNAP_MANAGE_ACCOUNTS_CONFIRMATION_TYPES.showSnapAccountRedirect:
controller.approvalController.accept(id, false);
break;
///: END:ONLY_INCLUDE_IN
Expand Down
25 changes: 24 additions & 1 deletion app/scripts/lib/keyring-snaps-permissions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ import {
SubjectType,
} from '@metamask/permission-controller';
import { KeyringRpcMethod } from '@metamask/keyring-api';
import { keyringSnapPermissionsBuilder } from './keyring-snaps-permissions';
import {
isProtocolAllowed,
keyringSnapPermissionsBuilder,
} from './keyring-snaps-permissions';

describe('keyringSnapPermissionsBuilder', () => {
const mockController = new SubjectMetadataController({
Expand Down Expand Up @@ -73,3 +76,23 @@ describe('keyringSnapPermissionsBuilder', () => {
expect(permissions(origin as any)).toStrictEqual([]);
});
});

describe('isProtocolAllowed', () => {
it.each([
['http://some-dapp.com', true],
['https://some-dapp.com', true],
['sftp://some-dapp.com', false],
['', false],
['null', false],
['0', false],
[undefined, false],
[null, false],
[true, false],
[false, false],
[1, false],
[0, false],
[-1, false],
])('"%s" cannot call any methods', (origin: any, expected: boolean) => {
expect(isProtocolAllowed(origin)).toBe(expected);
});
});
10 changes: 7 additions & 3 deletions app/scripts/lib/keyring-snaps-permissions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,13 @@ const ALLOWED_PROTOCOLS: string[] = [
* @param origin - The origin to check.
* @returns `true` if the protocol of the origin is allowed, `false` otherwise.
*/
function isProtocolAllowed(origin: string): boolean {
const url = new URL(origin);
return ALLOWED_PROTOCOLS.includes(url.protocol);
export function isProtocolAllowed(origin: string): boolean {
try {
const url = new URL(origin);
return ALLOWED_PROTOCOLS.includes(url.protocol);
} catch (error) {
return false;
}
}

/**
Expand Down
28 changes: 28 additions & 0 deletions app/scripts/lib/snap-keyring/snap-keyring.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@ import type {
ResultComponent,
} from '@metamask/approval-controller';
import type { KeyringController } from '@metamask/keyring-controller';
import { PhishingController } from '@metamask/phishing-controller';
import browser from 'webextension-polyfill';
import { SNAP_MANAGE_ACCOUNTS_CONFIRMATION_TYPES } from '../../../../shared/constants/app';
import { t } from '../../translate';
import MetamaskController from '../../metamask-controller';
import PreferencesController from '../../controllers/preferences';
import { isBlockedUrl } from './utils/isBlockedUrl';

/**
* Get the addresses of the accounts managed by a given Snap.
Expand All @@ -32,6 +35,7 @@ export const getAccountsBySnapId = async (
* @param getApprovalController - A function that retrieves the Approval Controller instance.
* @param getKeyringController - A function that retrieves the Keyring Controller instance.
* @param getPreferencesController - A function that retrieves the Preferences Controller instance.
* @param getPhishingController - A function that retrieves the Phishing Controller instance
* @param removeAccountHelper - A function to help remove an account based on its address.
* @returns The constructed SnapKeyring builder instance with the following methods:
* - `saveState`: Persists all keyrings in the keyring controller.
Expand All @@ -43,6 +47,7 @@ export const snapKeyringBuilder = (
getApprovalController: () => ApprovalController,
getKeyringController: () => KeyringController,
getPreferencesController: () => PreferencesController,
getPhishingController: () => PhishingController,
removeAccountHelper: (address: string) => Promise<any>,
) => {
const builder = (() => {
Expand All @@ -51,6 +56,29 @@ export const snapKeyringBuilder = (
const addresses = await getKeyringController().getAccounts();
return addresses.includes(address.toLowerCase());
},
redirectUser: async (snapId: string, url: string, message: string) => {
// Either url or message must be defined
if (url.length > 0 || message.length > 0) {
const isBlocked = await isBlockedUrl(url, getPhishingController());

const confirmationResult: boolean =
(await getApprovalController().addAndShowApprovalRequest({
origin: snapId,
requestData: { url, message, isBlockedUrl: isBlocked },
type: SNAP_MANAGE_ACCOUNTS_CONFIRMATION_TYPES.showSnapAccountRedirect,
})) as boolean;

if (confirmationResult && url.length > 0) {
browser.tabs.create({ url });
} else {
console.log('User refused snap account redirection to:', url);
}
} else {
console.log(
'Error occurred when redirecting snap account. url or message must be defined',
);
}
},
saveState: async () => {
await getKeyringController().persistAllKeyrings();
},
Expand Down
38 changes: 38 additions & 0 deletions app/scripts/lib/snap-keyring/utils/isBlockedUrl.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { ListNames, PhishingController } from '@metamask/phishing-controller';
import { isBlockedUrl } from './isBlockedUrl';

describe('isBlockedUrl', () => {
const phishingController = new PhishingController(
{},
{
phishingLists: [
{
blocklist: ['https://metamask.test'],
allowlist: [],
fuzzylist: [],
tolerance: 0,
version: 1,
lastUpdated: 0,
name: ListNames.MetaMask,
},
],
},
);

it.each([
['http://metamask.io', false],
['https://metamask.io', false],
['https://metamask.test', true],
['sftp://metamask.io', true],
['', true],
['1', true],
[undefined, true],
[null, true],
[1, true],
[0, true],
[-1, true],
])('"%s" is blocked: %s', async (url: any, expected: boolean) => {
const result = await isBlockedUrl(url, phishingController);
expect(result).toEqual(expected);
});
});
32 changes: 32 additions & 0 deletions app/scripts/lib/snap-keyring/utils/isBlockedUrl.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { PhishingController } from '@metamask/phishing-controller';
import { isProtocolAllowed } from '../../keyring-snaps-permissions';

/**
* Checks whether a given URL is blocked due to not using HTTPS or being
* recognized as a phishing URL.
*
* @param url - The URL to check.
* @param phishingController - An instance of PhishingController to verify
* against known phishing URLs.
* @returns Returns a promise which resolves to `true` if the URL is blocked
* either due to using an insecure protocol (not HTTPS) or being recognized as
* a phishing URL. Otherwise, resolves to `false`.
*/
export const isBlockedUrl = async (
url: string,
phishingController: PhishingController,
): Promise<boolean> => {
try {
// check if the URL is HTTPS
if (!isProtocolAllowed(url)) {
return true;
}

// check if the url is in the phishing list
await phishingController.maybeUpdateState();
return phishingController.test(url).result;
} catch (error) {
console.error('Invalid URL passed into snap-keyring:', error);
return false;
}
};
2 changes: 2 additions & 0 deletions app/scripts/metamask-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -877,13 +877,15 @@ export default class MetamaskController extends EventEmitter {
const getApprovalController = () => this.approvalController;
const getKeyringController = () => this.keyringController;
const getPreferencesController = () => this.preferencesController;
const getPhishingController = () => this.phishingController;

additionalKeyrings.push(
snapKeyringBuilder(
getSnapController,
getApprovalController,
getKeyringController,
getPreferencesController,
getPhishingController,
(address) => this.removeAccount(address),
),
);
Expand Down
3 changes: 1 addition & 2 deletions lavamoat/browserify/desktop/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -1227,8 +1227,7 @@
},
"@metamask/eth-snap-keyring": {
"globals": {
"console.error": true,
"console.log": true
"console.error": true
},
"packages": {
"@ethereumjs/tx": true,
Expand Down
3 changes: 1 addition & 2 deletions lavamoat/browserify/flask/policy.json
Original file line number Diff line number Diff line change
Expand Up @@ -1227,8 +1227,7 @@
},
"@metamask/eth-snap-keyring": {
"globals": {
"console.error": true,
"console.log": true
"console.error": true
},
"packages": {
"@ethereumjs/tx": true,
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@
"@metamask/eth-json-rpc-middleware": "^11.0.0",
"@metamask/eth-keyring-controller": "^13.0.1",
"@metamask/eth-ledger-bridge-keyring": "^0.15.0",
"@metamask/eth-snap-keyring": "^1.0.0-rc.1",
"@metamask/eth-snap-keyring": "1.0.0-rc.2",
"@metamask/eth-token-tracker": "^4.0.0",
"@metamask/eth-trezor-keyring": "^1.1.0",
"@metamask/etherscan-link": "^2.2.0",
Expand Down Expand Up @@ -436,6 +436,7 @@
"@types/sinon": "^10.0.13",
"@types/w3c-web-hid": "^1.0.3",
"@types/watchify": "^3.11.1",
"@types/webextension-polyfill": "^0.10.4",
"@types/yargs": "^17.0.8",
"@typescript-eslint/eslint-plugin": "^5.30.7",
"@typescript-eslint/parser": "^5.30.7",
Expand Down
1 change: 1 addition & 0 deletions privacy-snapshot.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"api.lens.dev",
"api.segment.io",
"arbitrum-mainnet.infura.io",
"aus5.mozilla.org",
"bafkreifvhjdf6ve4jfv6qytqtux5nd4nwnelioeiqx5x2ez5yrgrzk7ypi.ipfs.dweb.link",
"bafybeidxfmwycgzcp4v2togflpqh2gnibuexjy4m4qqwxp7nh3jx5zlh4y.ipfs.dweb.link",
"cdnjs.cloudflare.com",
Expand Down
1 change: 1 addition & 0 deletions shared/constants/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export const SNAP_DIALOG_TYPES = {
export const SNAP_MANAGE_ACCOUNTS_CONFIRMATION_TYPES = {
confirmAccountCreation: 'snap_manageAccounts:confirmAccountCreation',
confirmAccountRemoval: 'snap_manageAccounts:confirmAccountRemoval',
showSnapAccountRedirect: 'showSnapAccountRedirect',
};
///: END:ONLY_INCLUDE_IN

Expand Down
Loading

0 comments on commit d5c8a65

Please sign in to comment.