Skip to content

Commit a4179bc

Browse files
authored
fix: cp-12.15.2 Allow verifyingContract to be omitted from EIP-712 signatures (#31613)
## **Description** The signature controller has been patched with changes from MetaMask/core#5595 to allow the `verifyingContract` field to be missing from EIP-712 signatures. It is optional in the spec. [![Open in GitHub Codespaces](https://github.com/codespaces/badge.svg)](https://codespaces.new/MetaMask/metamask-extension/pull/31613?quickstart=1) ## **Related issues** Fixes #31607 ## **Manual testing steps** 1. Connect to the test-dapp 2. Enter this URL: ``` https://metamask.github.io/test-dapp/request.html?method=eth_signTypedData_v4&params=[%22YOUR_ADDRESSS_HERE%22,{%22types%22:{%22EIP712Domain%22:[{%22name%22:%22name%22,%22type%22:%22string%22}],%22Mail%22:[{%22name%22:%22from%22,%22type%22:%22string%22}]},%22primaryType%22:%22Mail%22,%22domain%22:{%22name%22:%22foo%22},%22message%22:{%from%22:%22foo%22}}] ``` (replace `YOUR_ADDRESS_HERE` with a connected address) 3. Previous it would fail. On this branch it should correctly prompt a signature confirmation. ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I've followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Extension Coding Standards](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [ ] I’ve included tests if applicable - We can manually test this for now, I will follow-up with an E2E test in a later PR - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-extension/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.
1 parent 8fa1420 commit a4179bc

File tree

3 files changed

+54
-3
lines changed

3 files changed

+54
-3
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
diff --git a/dist/utils/validation.cjs b/dist/utils/validation.cjs
2+
index eb116f75643a6607af10eb1669aaabfb48159826..349937fca246b26deac4a428fb3ad93dc7e18a6e 100644
3+
--- a/dist/utils/validation.cjs
4+
+++ b/dist/utils/validation.cjs
5+
@@ -150,7 +150,9 @@ function validateAddress(address, propertyName) {
6+
function validateVerifyingContract({ data, internalAccounts, origin, }) {
7+
const verifyingContract = data?.domain?.verifyingContract;
8+
const isExternal = origin && origin !== approval_controller_1.ORIGIN_METAMASK;
9+
- if (isExternal &&
10+
+ if (verifyingContract &&
11+
+ typeof verifyingContract === 'string' &&
12+
+ isExternal &&
13+
internalAccounts.some((internalAccount) => internalAccount.toLowerCase() === verifyingContract.toLowerCase())) {
14+
throw new Error(`External signature requests cannot use internal accounts as the verifying contract.`);
15+
}
16+
diff --git a/dist/utils/validation.mjs b/dist/utils/validation.mjs
17+
index 58b55c134527537c0ef1f44876afcf2939e9ed53..d4af6e8a1e8fff08e422ebfa6b9f444dc24c8632 100644
18+
--- a/dist/utils/validation.mjs
19+
+++ b/dist/utils/validation.mjs
20+
@@ -145,7 +145,9 @@ function validateAddress(address, propertyName) {
21+
function validateVerifyingContract({ data, internalAccounts, origin, }) {
22+
const verifyingContract = data?.domain?.verifyingContract;
23+
const isExternal = origin && origin !== ORIGIN_METAMASK;
24+
- if (isExternal &&
25+
+ if (verifyingContract &&
26+
+ typeof verifyingContract === 'string' &&
27+
+ isExternal &&
28+
internalAccounts.some((internalAccount) => internalAccount.toLowerCase() === verifyingContract.toLowerCase())) {
29+
throw new Error(`External signature requests cannot use internal accounts as the verifying contract.`);
30+
}

package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@
314314
"@metamask/safe-event-emitter": "^3.1.1",
315315
"@metamask/scure-bip39": "^2.0.3",
316316
"@metamask/selected-network-controller": "^19.0.0",
317-
"@metamask/signature-controller": "^26.0.0",
317+
"@metamask/signature-controller": "patch:@metamask/signature-controller@npm%3A26.0.0#~/.yarn/patches/@metamask-signature-controller-npm-26.0.0-ecbc8b6cda.patch",
318318
"@metamask/smart-transactions-controller": "^16.3.0",
319319
"@metamask/snaps-controllers": "^11.1.0",
320320
"@metamask/snaps-execution-environments": "^7.1.0",

yarn.lock

+23-2
Original file line numberDiff line numberDiff line change
@@ -6247,7 +6247,7 @@ __metadata:
62476247
languageName: node
62486248
linkType: hard
62496249

6250-
"@metamask/signature-controller@npm:^26.0.0":
6250+
"@metamask/signature-controller@npm:26.0.0":
62516251
version: 26.0.0
62526252
resolution: "@metamask/signature-controller@npm:26.0.0"
62536253
dependencies:
@@ -6268,6 +6268,27 @@ __metadata:
62686268
languageName: node
62696269
linkType: hard
62706270

6271+
"@metamask/signature-controller@patch:@metamask/signature-controller@npm%3A26.0.0#~/.yarn/patches/@metamask-signature-controller-npm-26.0.0-ecbc8b6cda.patch":
6272+
version: 26.0.0
6273+
resolution: "@metamask/signature-controller@patch:@metamask/signature-controller@npm%3A26.0.0#~/.yarn/patches/@metamask-signature-controller-npm-26.0.0-ecbc8b6cda.patch::version=26.0.0&hash=eeb7c7"
6274+
dependencies:
6275+
"@metamask/base-controller": "npm:^8.0.0"
6276+
"@metamask/controller-utils": "npm:^11.6.0"
6277+
"@metamask/eth-sig-util": "npm:^8.2.0"
6278+
"@metamask/utils": "npm:^11.2.0"
6279+
jsonschema: "npm:^1.4.1"
6280+
lodash: "npm:^4.17.21"
6281+
uuid: "npm:^8.3.2"
6282+
peerDependencies:
6283+
"@metamask/accounts-controller": ^26.0.0
6284+
"@metamask/approval-controller": ^7.0.0
6285+
"@metamask/keyring-controller": ^21.0.0
6286+
"@metamask/logging-controller": ^6.0.0
6287+
"@metamask/network-controller": ^22.0.0
6288+
checksum: 10/a2df7d134d1b8e48af9ef218c0011b8a41ca5dcbed23ce75cc089043950b5c95114bea223639230fc0197d81b1c89647bf99abad41b7368082aa829c2ff3aacd
6289+
languageName: node
6290+
linkType: hard
6291+
62716292
"@metamask/slip44@npm:^4.1.0":
62726293
version: 4.1.0
62736294
resolution: "@metamask/slip44@npm:4.1.0"
@@ -27274,7 +27295,7 @@ __metadata:
2727427295
"@metamask/safe-event-emitter": "npm:^3.1.1"
2727527296
"@metamask/scure-bip39": "npm:^2.0.3"
2727627297
"@metamask/selected-network-controller": "npm:^19.0.0"
27277-
"@metamask/signature-controller": "npm:^26.0.0"
27298+
"@metamask/signature-controller": "patch:@metamask/signature-controller@npm%3A26.0.0#~/.yarn/patches/@metamask-signature-controller-npm-26.0.0-ecbc8b6cda.patch"
2727827299
"@metamask/smart-transactions-controller": "npm:^16.3.0"
2727927300
"@metamask/snaps-controllers": "npm:^11.1.0"
2728027301
"@metamask/snaps-execution-environments": "npm:^7.1.0"

0 commit comments

Comments
 (0)