Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/signature-controller/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed

- Allow redelegation signing for internal accounts from external requests ([#7917](https://github.com/MetaMask/core/issues/7917))
- Previously, `validateDelegation` blocked all delegation signing from external origins for internal accounts. Now it only blocks root delegations (where `authority` equals `ROOT_AUTHORITY`), allowing redelegations which are safe since the signer is not the root delegator.

## [39.0.2]

### Changed
Expand Down
94 changes: 86 additions & 8 deletions packages/signature-controller/src/utils/validation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type { Hex } from '@metamask/utils';

import {
PRIMARY_TYPE_DELEGATION,
ROOT_AUTHORITY,
validatePersonalSignatureRequest,
validateTypedSignatureRequest,
} from './validation';
Expand Down Expand Up @@ -409,12 +410,13 @@ describe('Validation Utils', () => {
});

describe('delegation', () => {
it('throws if external origin in request and delegation from internal account', () => {
it('throws if external origin in request and root delegation from internal account', () => {
const data = JSON.parse(DATA_TYPED_MOCK);

data.primaryType = PRIMARY_TYPE_DELEGATION;
data.types.Delegation = [{ name: 'delegator', type: 'address' }];
data.message.delegator = INTERNAL_ACCOUNT_MOCK;
data.message.authority = ROOT_AUTHORITY;

expect(() =>
validateTypedSignatureRequest({
Expand All @@ -428,16 +430,17 @@ describe('Validation Utils', () => {
version,
}),
).toThrow(
'External signature requests cannot sign delegations for internal accounts.',
'External signature requests cannot sign root delegations for internal accounts.',
);
});

it('throws if external origin in message params and delegation from internal account', () => {
it('throws if external origin in message params and root delegation from internal account', () => {
const data = JSON.parse(DATA_TYPED_MOCK);

data.primaryType = PRIMARY_TYPE_DELEGATION;
data.types.Delegation = [{ name: 'delegator', type: 'address' }];
data.message.delegator = INTERNAL_ACCOUNT_MOCK;
data.message.authority = ROOT_AUTHORITY;

expect(() =>
validateTypedSignatureRequest({
Expand All @@ -452,16 +455,17 @@ describe('Validation Utils', () => {
version,
}),
).toThrow(
'External signature requests cannot sign delegations for internal accounts.',
'External signature requests cannot sign root delegations for internal accounts.',
);
});

it('throws if external origin and delegation from internal account with different case', () => {
it('throws if external origin and root delegation from internal account with different case', () => {
const data = JSON.parse(DATA_TYPED_MOCK);

data.primaryType = PRIMARY_TYPE_DELEGATION;
data.types.Delegation = [{ name: 'delegator', type: 'address' }];
data.message.delegator = INTERNAL_ACCOUNT_MOCK;
data.message.authority = ROOT_AUTHORITY;

expect(() =>
validateTypedSignatureRequest({
Expand All @@ -478,16 +482,17 @@ describe('Validation Utils', () => {
version,
}),
).toThrow(
'External signature requests cannot sign delegations for internal accounts.',
'External signature requests cannot sign root delegations for internal accounts.',
);
});

it('does not throw if internal origin and delegation from internal account', () => {
it('does not throw if internal origin and root delegation from internal account', () => {
const data = JSON.parse(DATA_TYPED_MOCK);

data.primaryType = PRIMARY_TYPE_DELEGATION;
data.types.Delegation = [{ name: 'delegator', type: 'address' }];
data.message.delegator = INTERNAL_ACCOUNT_MOCK;
data.message.authority = ROOT_AUTHORITY;

expect(() =>
validateTypedSignatureRequest({
Expand All @@ -503,12 +508,13 @@ describe('Validation Utils', () => {
).not.toThrow();
});

it('does not throw if no origin and delegation from internal account', () => {
it('does not throw if no origin and root delegation from internal account', () => {
const data = JSON.parse(DATA_TYPED_MOCK);

data.primaryType = PRIMARY_TYPE_DELEGATION;
data.types.Delegation = [{ name: 'delegator', type: 'address' }];
data.message.delegator = INTERNAL_ACCOUNT_MOCK;
data.message.authority = ROOT_AUTHORITY;

expect(() =>
validateTypedSignatureRequest({
Expand All @@ -523,6 +529,78 @@ describe('Validation Utils', () => {
}),
).not.toThrow();
});

it('does not throw if external origin and redelegation (non-root authority) from internal account', () => {
const data = JSON.parse(DATA_TYPED_MOCK);
const NON_ROOT_AUTHORITY =
'0x1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef';

data.primaryType = PRIMARY_TYPE_DELEGATION;
data.types.Delegation = [{ name: 'delegator', type: 'address' }];
data.message.delegator = INTERNAL_ACCOUNT_MOCK;
data.message.authority = NON_ROOT_AUTHORITY;

expect(() =>
validateTypedSignatureRequest({
currentChainId: CHAIN_ID_MOCK,
internalAccounts: ['0x1234', INTERNAL_ACCOUNT_MOCK],
messageData: {
data,
from: '0x3244e191f1b4903970224322180f1fbbc415696b',
},
request: { origin: ORIGIN_MOCK } as OriginalRequest,
version,
}),
).not.toThrow();
});

it('throws if external origin and delegation with missing authority from internal account', () => {
const data = JSON.parse(DATA_TYPED_MOCK);

data.primaryType = PRIMARY_TYPE_DELEGATION;
data.types.Delegation = [{ name: 'delegator', type: 'address' }];
data.message.delegator = INTERNAL_ACCOUNT_MOCK;
// authority field intentionally omitted

expect(() =>
validateTypedSignatureRequest({
currentChainId: CHAIN_ID_MOCK,
internalAccounts: ['0x1234', INTERNAL_ACCOUNT_MOCK],
messageData: {
data,
from: '0x3244e191f1b4903970224322180f1fbbc415696b',
},
request: { origin: ORIGIN_MOCK } as OriginalRequest,
version,
}),
).toThrow(
'External signature requests cannot sign root delegations for internal accounts.',
);
});

it('does not throw if external origin and redelegation from internal account with different case authority', () => {
const data = JSON.parse(DATA_TYPED_MOCK);
const NON_ROOT_AUTHORITY =
'0xABCDEF1234567890ABCDEF1234567890ABCDEF1234567890ABCDEF1234567890';

data.primaryType = PRIMARY_TYPE_DELEGATION;
data.types.Delegation = [{ name: 'delegator', type: 'address' }];
data.message.delegator = INTERNAL_ACCOUNT_MOCK;
data.message.authority = NON_ROOT_AUTHORITY;

expect(() =>
validateTypedSignatureRequest({
currentChainId: CHAIN_ID_MOCK,
internalAccounts: ['0x1234', INTERNAL_ACCOUNT_MOCK],
messageData: {
data,
from: '0x3244e191f1b4903970224322180f1fbbc415696b',
},
request: { origin: ORIGIN_MOCK } as OriginalRequest,
version,
}),
).not.toThrow();
});
});
},
);
Expand Down
18 changes: 16 additions & 2 deletions packages/signature-controller/src/utils/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ import type {

export const PRIMARY_TYPE_DELEGATION = 'Delegation';
export const DELEGATOR_FIELD = 'delegator';
export const AUTHORITY_FIELD = 'authority';
export const ROOT_AUTHORITY =
'0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff';

/**
* Validate a personal signature request.
Expand Down Expand Up @@ -285,9 +288,20 @@ function validateDelegation({
internalAccount.toLowerCase() === delegatorAddressLowercase,
);

if (isOriginExternal && isSignerInternal) {
const authority = (
(data.message as Record<string, Json>)?.[AUTHORITY_FIELD] as
| string
| undefined
)?.toLowerCase();

// Fail-closed: if authority is missing or is ROOT_AUTHORITY, block the request.
// Only allow signing when authority is explicitly a non-root value (redelegation).
const isRootOrMissingAuthority =
!authority || authority === ROOT_AUTHORITY;

if (isOriginExternal && isSignerInternal && isRootOrMissingAuthority) {
throw new Error(
`External signature requests cannot sign delegations for internal accounts.`,
`External signature requests cannot sign root delegations for internal accounts.`,
);
}
}
Expand Down