Skip to content

Commit c947237

Browse files
authored
Allow Advanced Permissions metadata in signtypeddata payload (MetaMask#8603)
## Explanation MetaMask#8526 adds tighter validation to signtypeddata v4 payloads, to ensure that no extraneous properties are added. This additional validation disallows Advanced Permissions `metadata` which is used to communicate the origin and justification of the permission. This change loosens the validation just enough to allow `metadata: { justification: string; origin: string }` as a property on the payload that is not used within the message encoding. ## References MetaMask#8526 MetaMask/metamask-extension#42181 <!-- Are there any issues that this pull request is tied to? Are there other links that reviewers should consult to understand these changes better? Are there client or consumer pull requests to adopt any breaking changes? For example: * Fixes #12345 * Related to #67890 --> ## Checklist - [ ] I've updated the test suite for new or updated code as appropriate - [ ] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [ ] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md) - [ ] I've introduced [breaking changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md) in this PR and have prepared draft pull requests for clients and consumer packages to resolve them <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches security-adjacent request validation for typed-data signing; while the new `metadata` allowance is tightly constrained, any loosening here could affect input filtering behavior. > > **Overview** > Relaxes `signTypedData` (V4) payload validation to permit an additional top-level `metadata` field used by Advanced Permissions. > > `validateTypedMessageKeys` now explicitly allows `metadata` and enforces it is exactly `{ justification: string, origin: string }` (rejecting non-objects, missing/typed fields, or extra keys), with new unit tests covering the allowed and rejected cases; changelog updated accordingly. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 17f9432. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 12ba53a commit c947237

3 files changed

Lines changed: 148 additions & 1 deletion

File tree

packages/eth-json-rpc-middleware/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- Allow Advanced Permissions `metadata` in signTypedData V4 requests ([#8603](https://github.com/MetaMask/core/pull/8603))
13+
1014
## [23.1.2]
1115

1216
### Changed

packages/eth-json-rpc-middleware/src/utils/validation.test.ts

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,5 +160,122 @@ describe('Validation Utils', () => {
160160

161161
expect(() => validateTypedMessageKeys(data)).toThrow('Invalid input.');
162162
});
163+
164+
describe('metadata', () => {
165+
const baseTypedData = {
166+
types: {
167+
EIP712Domain: [{ name: 'name', type: 'string' }],
168+
},
169+
primaryType: 'EIP712Domain',
170+
domain: {},
171+
message: {},
172+
};
173+
174+
it('does not throw when metadata has exactly justification and origin as strings', () => {
175+
const data = JSON.stringify({
176+
...baseTypedData,
177+
metadata: {
178+
justification: 'Permission to spend tokens',
179+
origin: 'https://example.com',
180+
},
181+
});
182+
183+
expect(() => validateTypedMessageKeys(data)).not.toThrow();
184+
});
185+
186+
it('does not throw when metadata is the only top-level key', () => {
187+
const data = JSON.stringify({
188+
metadata: {
189+
justification: 'Permission to spend tokens',
190+
origin: 'https://example.com',
191+
},
192+
});
193+
194+
expect(() => validateTypedMessageKeys(data)).not.toThrow();
195+
});
196+
197+
it.each([
198+
['null', null],
199+
['a string', 'not-an-object'],
200+
['a number', 42],
201+
['a boolean', true],
202+
['an array', ['justification', 'origin']],
203+
])('throws when metadata is %s', (_label, value) => {
204+
const data = JSON.stringify({
205+
...baseTypedData,
206+
metadata: value,
207+
});
208+
209+
expect(() => validateTypedMessageKeys(data)).toThrow('Invalid input.');
210+
});
211+
212+
it('throws when metadata is missing justification', () => {
213+
const data = JSON.stringify({
214+
...baseTypedData,
215+
metadata: {
216+
origin: 'https://example.com',
217+
},
218+
});
219+
220+
expect(() => validateTypedMessageKeys(data)).toThrow('Invalid input.');
221+
});
222+
223+
it('throws when metadata is missing origin', () => {
224+
const data = JSON.stringify({
225+
...baseTypedData,
226+
metadata: {
227+
justification: 'Permission to spend tokens',
228+
},
229+
});
230+
231+
expect(() => validateTypedMessageKeys(data)).toThrow('Invalid input.');
232+
});
233+
234+
it('throws when metadata.justification is not a string', () => {
235+
const data = JSON.stringify({
236+
...baseTypedData,
237+
metadata: {
238+
justification: 123,
239+
origin: 'https://example.com',
240+
},
241+
});
242+
243+
expect(() => validateTypedMessageKeys(data)).toThrow('Invalid input.');
244+
});
245+
246+
it('throws when metadata.origin is not a string', () => {
247+
const data = JSON.stringify({
248+
...baseTypedData,
249+
metadata: {
250+
justification: 'Permission to spend tokens',
251+
origin: 123,
252+
},
253+
});
254+
255+
expect(() => validateTypedMessageKeys(data)).toThrow('Invalid input.');
256+
});
257+
258+
it('throws when metadata has an extraneous third key', () => {
259+
const data = JSON.stringify({
260+
...baseTypedData,
261+
metadata: {
262+
justification: 'Permission to spend tokens',
263+
origin: 'https://example.com',
264+
extra: 'unexpected',
265+
},
266+
});
267+
268+
expect(() => validateTypedMessageKeys(data)).toThrow('Invalid input.');
269+
});
270+
271+
it('throws when metadata is an empty object', () => {
272+
const data = JSON.stringify({
273+
...baseTypedData,
274+
metadata: {},
275+
});
276+
277+
expect(() => validateTypedMessageKeys(data)).toThrow('Invalid input.');
278+
});
279+
});
163280
});
164281
});

packages/eth-json-rpc-middleware/src/utils/validation.ts

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,12 +199,38 @@ export function validateTypedDataForPrototypePollution(data: string): void {
199199
*/
200200
export function validateTypedMessageKeys(data: string): void {
201201
const parsedData = parseTypedMessage(data);
202-
const allowedKeys = new Set(Object.keys(TYPED_MESSAGE_SCHEMA.properties));
202+
const allowedKeys = new Set([
203+
...Object.keys(TYPED_MESSAGE_SCHEMA.properties),
204+
'metadata',
205+
]);
203206
const hasExtraneousKey = Object.keys(parsedData).some(
204207
(key) => !allowedKeys.has(key),
205208
);
206209

207210
if (hasExtraneousKey) {
208211
throw rpcErrors.invalidInput();
209212
}
213+
214+
// Advanced Permissions adds `metadata: { justification: string, origin: string }` to eth_signTypedData requests.
215+
// see GatorPermissionsController.decodePermissionFromPermissionContextForOrigin for more details.
216+
const { metadata } = parsedData as { metadata?: unknown };
217+
if (metadata !== undefined) {
218+
if (typeof metadata !== 'object' || metadata === null) {
219+
throw rpcErrors.invalidInput();
220+
}
221+
222+
const { justification, origin } = metadata as {
223+
justification?: unknown;
224+
origin?: unknown;
225+
};
226+
227+
if (typeof justification !== 'string' || typeof origin !== 'string') {
228+
throw rpcErrors.invalidInput();
229+
}
230+
231+
// we only need to check the keys length, because we already checked the known keys (justification and origin).
232+
if (Object.keys(metadata).length !== 2) {
233+
throw rpcErrors.invalidInput();
234+
}
235+
}
210236
}

0 commit comments

Comments
 (0)