diff --git a/app/_locales/en/messages.json b/app/_locales/en/messages.json index d71994b69..a9467f26c 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 2d113e599..456f01263 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 8318714ee..37761cb3c 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 diff --git a/app/scripts/metamask-controller.test.js b/app/scripts/metamask-controller.test.js index 698b6496b..9a4c49684 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/test/data/mock-send-state.json b/test/data/mock-send-state.json index 7dd00632a..6e460ef8e 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 87cab742b..51ef2168c 100644 --- a/test/e2e/tests/eth-sign.spec.js +++ b/test/e2e/tests/eth-sign.spec.js @@ -2,21 +2,18 @@ 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('can initiate and confirm a eth sign', async function () { - const ganacheOptions = { - accounts: [ - { - secretKey: - '0x7C9529A67102755B7E6102D6D950AC5D5863C98713805CEC576B945B15B71EAC', - balance: convertToHexValue(25000000000000000000), - }, - ], - }; - const expectedPersonalMessage = - '0x879a053d4800c6354e76c7985a865d2922c82fb5b3f4577b2fe08b998954f2e0'; - const expectedEthSignResult = - '"0x816ab6c5d5356548cc4e004ef35a37fdfab916742a2bbeda756cd064c3d3789a6557d41d49549be1de249e1937a8d048996dfcc70d0552111605dc7cc471e8531b"'; + it('will detect if eth_sign is disabled', async function () { await withFixtures( { dapp: true, @@ -34,6 +31,43 @@ describe('Eth sign', function () { 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 expectedPersonalMessage = + '0x879a053d4800c6354e76c7985a865d2922c82fb5b3f4577b2fe08b998954f2e0'; + const expectedEthSignResult = + '"0x816ab6c5d5356548cc4e004ef35a37fdfab916742a2bbeda756cd064c3d3789a6557d41d49549be1de249e1937a8d048996dfcc70d0552111605dc7cc471e8531b"'; + await withFixtures( + { + dapp: true, + fixtures: new FixtureBuilder() + .withPreferencesController({ + disabledRpcMethodPreferences: { + eth_sign: true, + }, + }) + .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'); + + // 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 0c7979557..7c92aebe0 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 daaadae9d..74c2c08b4 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 3a57a9ad2..0072b0580 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 ( +