diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index d71994b69697..a9467f26c69f 100644 --- a/app/_locales/en/messages.json +++ b/app/_locales/en/messages.json @@ -3971,6 +3971,12 @@ "message": "To: $1", "description": "$1 is the address to include in the To label. It is typically shortened first using shortenAddress" }, + "toggleEthSignDescriptionField": { + "message": "Turn this on to let dapps request your signature using eth_sign requests. eth_sign is an open-ended signing method that lets you sign an arbitrary hash, making it a dangerous phishing risk. Only sign eth_sign requests if you can read what you are signing and trust the origin of the request." + }, + "toggleEthSignField": { + "message": "Toggle eth_sign requests" + }, "toggleTestNetworks": { "message": "$1 test networks", "description": "$1 is a clickable link with text defined by the 'showHide' key. The link will open Settings > Advanced where users can enable the display of test networks in the network dropdown." diff --git a/app/scripts/controllers/preferences.js b/app/scripts/controllers/preferences.js index 2d113e599ce3..456f01263420 100644 --- a/app/scripts/controllers/preferences.js +++ b/app/scripts/controllers/preferences.js @@ -30,6 +30,9 @@ export default class PreferencesController { useNonceField: false, usePhishDetect: true, dismissSeedBackUpReminder: false, + disabledRpcMethodPreferences: { + eth_sign: false, + }, useMultiAccountBalanceChecker: true, // set to true means the dynamic list from the API is being used @@ -72,7 +75,7 @@ export default class PreferencesController { this.network = opts.network; this.store = new ObservableStore(initState); - this.store.setMaxListeners(12); + this.store.setMaxListeners(13); this.openPopup = opts.openPopup; this.tokenListController = opts.tokenListController; @@ -544,6 +547,29 @@ export default class PreferencesController { }); } + /** + * A setter for the user preference to enable/disable rpc methods + * + * @param {string} methodName - The RPC method name to change the setting of + * @param {bool} isEnabled - true to enable the rpc method + */ + async setDisabledRpcMethodPreference(methodName, isEnabled) { + const currentRpcMethodPreferences = + this.store.getState().disabledRpcMethodPreferences; + const updatedRpcMethodPreferences = { + ...currentRpcMethodPreferences, + [methodName]: isEnabled, + }; + + this.store.updateState({ + disabledRpcMethodPreferences: updatedRpcMethodPreferences, + }); + } + + getRpcMethodPreferences() { + return this.store.getState().disabledRpcMethodPreferences; + } + // // PRIVATE METHODS // diff --git a/app/scripts/metamask-controller.js b/app/scripts/metamask-controller.js index 8318714eed1c..6947f6eae17e 100644 --- a/app/scripts/metamask-controller.js +++ b/app/scripts/metamask-controller.js @@ -1824,6 +1824,14 @@ export default class MetamaskController extends EventEmitter { preferencesController.setDismissSeedBackUpReminder.bind( preferencesController, ), + setDisabledRpcMethodPreference: + preferencesController.setDisabledRpcMethodPreference.bind( + preferencesController, + ), + getRpcMethodPreferences: + preferencesController.getRpcMethodPreferences.bind( + preferencesController, + ), setAdvancedGasFee: preferencesController.setAdvancedGasFee.bind( preferencesController, ), @@ -3046,8 +3054,19 @@ export default class MetamaskController extends EventEmitter { * @param {object} [req] - The original request, containing the origin. */ async newUnsignedMessage(msgParams, req) { + const { disabledRpcMethodPreferences } = + this.preferencesController.store.getState(); + const { eth_sign } = disabledRpcMethodPreferences; // eslint-disable-line camelcase const data = normalizeMsgData(msgParams.data); let promise; + + // eslint-disable-next-line camelcase + if (!eth_sign) { + throw ethErrors.rpc.methodNotFound( + 'eth_sign has been disabled. You must enable it in the advanced settings', + ); + } + // 64 hex + "0x" at the beginning // This is needed because Ethereum's EcSign works only on 32 byte numbers // For 67 length see: https://github.com/MetaMask/metamask-extension/pull/12679/files#r749479607 @@ -3591,6 +3610,13 @@ export default class MetamaskController extends EventEmitter { hostname, phishingTestResponse, ); + this.metaMetricsController.trackEvent({ + event: EVENT_NAMES.PHISHING_PAGE_DISPLAYED, + category: EVENT.CATEGORIES.PHISHING, + properties: { + url: hostname, + }, + }); return; } } diff --git a/app/scripts/metamask-controller.test.js b/app/scripts/metamask-controller.test.js index 698b6496b53e..9a4c49684626 100644 --- a/app/scripts/metamask-controller.test.js +++ b/app/scripts/metamask-controller.test.js @@ -872,6 +872,10 @@ describe('MetaMaskController', function () { data, }; + metamaskController.preferencesController.setDisabledRpcMethodPreference( + 'eth_sign', + true, + ); const promise = metamaskController.newUnsignedMessage(msgParams); // handle the promise so it doesn't throw an unhandledRejection promise.then(noop).catch(noop); diff --git a/shared/constants/metametrics.js b/shared/constants/metametrics.js index 5e3f0e2f3df6..8f4cf4b5b3d0 100644 --- a/shared/constants/metametrics.js +++ b/shared/constants/metametrics.js @@ -333,6 +333,7 @@ export const EVENT_NAMES = { PERMISSIONS_APPROVED: 'Permissions Approved', PERMISSIONS_REJECTED: 'Permissions Rejected', PERMISSIONS_REQUESTED: 'Permissions Requested', + PHISHING_PAGE_DISPLAYED: 'Phishing Page Displayed', PORTFOLIO_LINK_CLICKED: 'Portfolio Link Clicked', PUBLIC_ADDRESS_COPIED: 'Public Address Copied', PROVIDER_METHOD_CALLED: 'Provider Method Called', @@ -393,6 +394,7 @@ export const EVENT = { NAVIGATION: 'Navigation', NETWORK: 'Network', ONBOARDING: 'Onboarding', + PHISHING: 'Phishing', RETENTION: 'Retention', SETTINGS: 'Settings', SNAPS: 'Snaps', diff --git a/test/data/mock-send-state.json b/test/data/mock-send-state.json index 7dd00632a360..6e460ef8ef54 100644 --- a/test/data/mock-send-state.json +++ b/test/data/mock-send-state.json @@ -45,6 +45,9 @@ "metamask": { "ipfsGateway": "", "dismissSeedBackUpReminder": false, + "disabledRpcMethodPreferences": { + "eth_sign": false + }, "usePhishDetect": true, "participateInMetaMetrics": false, "gasEstimateType": "fee-market", diff --git a/test/e2e/tests/eth-sign.spec.js b/test/e2e/tests/eth-sign.spec.js index 87cab742bceb..51ef2168c217 100644 --- a/test/e2e/tests/eth-sign.spec.js +++ b/test/e2e/tests/eth-sign.spec.js @@ -2,17 +2,45 @@ const { strict: assert } = require('assert'); const { convertToHexValue, withFixtures } = require('../helpers'); const FixtureBuilder = require('../fixture-builder'); +const ganacheOptions = { + accounts: [ + { + secretKey: + '0x7C9529A67102755B7E6102D6D950AC5D5863C98713805CEC576B945B15B71EAC', + balance: convertToHexValue(25000000000000000000), + }, + ], +}; + describe('Eth sign', function () { + it('will detect if eth_sign is disabled', async function () { + await withFixtures( + { + dapp: true, + fixtures: new FixtureBuilder() + .withPermissionControllerConnectedToTestDapp() + .build(), + ganacheOptions, + title: this.test.title, + }, + async ({ driver }) => { + await driver.navigate(); + await driver.fill('#password', 'correct horse battery staple'); + await driver.press('#password', driver.Key.ENTER); + + await driver.openNewPage('http://127.0.0.1:8080/'); + await driver.clickElement('#ethSign'); + + const ethSignButton = await driver.findElement('#ethSign'); + const exceptionString = + 'ERROR: ETH_SIGN HAS BEEN DISABLED. YOU MUST ENABLE IT IN THE ADVANCED SETTINGS'; + + assert.equal(await ethSignButton.getText(), exceptionString); + }, + ); + }); + it('can initiate and confirm a eth sign', async function () { - const ganacheOptions = { - accounts: [ - { - secretKey: - '0x7C9529A67102755B7E6102D6D950AC5D5863C98713805CEC576B945B15B71EAC', - balance: convertToHexValue(25000000000000000000), - }, - ], - }; const expectedPersonalMessage = '0x879a053d4800c6354e76c7985a865d2922c82fb5b3f4577b2fe08b998954f2e0'; const expectedEthSignResult = @@ -21,6 +49,11 @@ describe('Eth sign', function () { { dapp: true, fixtures: new FixtureBuilder() + .withPreferencesController({ + disabledRpcMethodPreferences: { + eth_sign: true, + }, + }) .withPermissionControllerConnectedToTestDapp() .build(), ganacheOptions, @@ -34,6 +67,7 @@ describe('Eth sign', function () { await driver.openNewPage('http://127.0.0.1:8080/'); await driver.clickElement('#ethSign'); + // Wait for Signature request popup await driver.waitUntilXWindowHandles(3); let windowHandles = await driver.getAllWindowHandles(); await driver.switchToWindowWithTitle( diff --git a/ui/helpers/constants/settings.js b/ui/helpers/constants/settings.js index 0c7979557db4..7c92aebe078b 100644 --- a/ui/helpers/constants/settings.js +++ b/ui/helpers/constants/settings.js @@ -132,6 +132,13 @@ export const SETTINGS_CONSTANTS = [ route: `${ADVANCED_ROUTE}#dimiss-secretrecovery`, icon: 'fas fa-sliders-h', }, + { + tabMessage: (t) => t('advanced'), + sectionMessage: (t) => t('toggleEthSignField'), + descriptionMessage: (t) => t('toggleEthSignDescriptionField'), + route: `${ADVANCED_ROUTE}#toggle-ethsign`, + icon: 'fas fa-sliders-h', + }, { tabMessage: (t) => t('contacts'), sectionMessage: (t) => t('contacts'), diff --git a/ui/helpers/utils/settings-search.test.js b/ui/helpers/utils/settings-search.test.js index daaadae9d775..74c2c08b4eef 100644 --- a/ui/helpers/utils/settings-search.test.js +++ b/ui/helpers/utils/settings-search.test.js @@ -159,7 +159,7 @@ describe('Settings Search Utils', () => { }); it('should get good advanced section number', () => { - expect(getNumberOfSettingsInSection(t, t('advanced'))).toStrictEqual(13); + expect(getNumberOfSettingsInSection(t, t('advanced'))).toStrictEqual(14); }); it('should get good contact section number', () => { diff --git a/ui/pages/settings/advanced-tab/advanced-tab.component.js b/ui/pages/settings/advanced-tab/advanced-tab.component.js index 3a57a9ad20e2..0072b0580c2e 100644 --- a/ui/pages/settings/advanced-tab/advanced-tab.component.js +++ b/ui/pages/settings/advanced-tab/advanced-tab.component.js @@ -56,6 +56,10 @@ export default class AdvancedTab extends PureComponent { userHasALedgerAccount: PropTypes.bool.isRequired, backupUserData: PropTypes.func.isRequired, restoreUserData: PropTypes.func.isRequired, + setDisabledRpcMethodPreference: PropTypes.func.isRequired, + disabledRpcMethodPreferences: PropTypes.shape({ + eth_sign: PropTypes.bool.isRequired, + }), }; state = { @@ -580,6 +584,39 @@ export default class AdvancedTab extends PureComponent { ); } + renderToggleEthSignControl() { + const { t } = this.context; + const { disabledRpcMethodPreferences, setDisabledRpcMethodPreference } = + this.props; + + return ( +
+
+ {t('toggleEthSignField')} +
+ {t('toggleEthSignDescriptionField')} +
+
+
+
+ + setDisabledRpcMethodPreference('eth_sign', !value) + } + offLabel={t('off')} + onLabel={t('on')} + /> +
+
+
+ ); + } + handleLockChange(time) { const { t } = this.context; const autoLockTimeLimit = Math.max(Number(time), 0); @@ -711,6 +748,7 @@ export default class AdvancedTab extends PureComponent { {this.renderRestoreUserData()} {notUsingFirefox ? this.renderLedgerLiveControl() : null} {this.renderDismissSeedBackupReminderControl()} + {this.renderToggleEthSignControl()} ); } diff --git a/ui/pages/settings/advanced-tab/advanced-tab.container.js b/ui/pages/settings/advanced-tab/advanced-tab.container.js index 9826811f6389..f30c954b64a8 100644 --- a/ui/pages/settings/advanced-tab/advanced-tab.container.js +++ b/ui/pages/settings/advanced-tab/advanced-tab.container.js @@ -11,6 +11,7 @@ import { setUseNonceField, setLedgerTransportPreference, setDismissSeedBackUpReminder, + setDisabledRpcMethodPreference, backupUserData, restoreUserData, } from '../../../store/actions'; @@ -25,6 +26,7 @@ export const mapStateToProps = (state) => { } = state; const { featureFlags: { sendHexData, advancedInlineGas } = {}, + disabledRpcMethodPreferences, useNonceField, ledgerTransportType, dismissSeedBackUpReminder, @@ -48,6 +50,7 @@ export const mapStateToProps = (state) => { ledgerTransportType, dismissSeedBackUpReminder, userHasALedgerAccount, + disabledRpcMethodPreferences, }; }; @@ -78,6 +81,9 @@ export const mapDispatchToProps = (dispatch) => { setDismissSeedBackUpReminder: (value) => { return dispatch(setDismissSeedBackUpReminder(value)); }, + setDisabledRpcMethodPreference: (methodName, isEnabled) => { + return dispatch(setDisabledRpcMethodPreference(methodName, isEnabled)); + }, }; }; diff --git a/ui/store/actions.ts b/ui/store/actions.ts index b6acf0828202..de453b6b1932 100644 --- a/ui/store/actions.ts +++ b/ui/store/actions.ts @@ -3830,6 +3830,33 @@ export function setDismissSeedBackUpReminder( }; } +export function setDisabledRpcMethodPreference( + methodName: string, + value: number, +): ThunkAction { + return async (dispatch) => { + dispatch(showLoadingIndication()); + await submitRequestToBackground('setDisabledRpcMethodPreference', [ + methodName, + value, + ]); + dispatch(hideLoadingIndication()); + }; +} + +export function getRpcMethodPreferences(): ThunkAction< + void, + MetaMaskReduxState, + unknown, + AnyAction +> { + return async (dispatch) => { + dispatch(showLoadingIndication()); + await submitRequestToBackground('getRpcMethodPreferences', []); + dispatch(hideLoadingIndication()); + }; +} + export function setConnectedStatusPopoverHasBeenShown(): ThunkAction< void, MetaMaskReduxState,