Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

fix: allow redelegation signing for internal accounts from external requests #7918

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
osobot-ai wants to merge 2 commits into MetaMask:main
base: main
Choose a base branch
Loading
from osobot-ai:fix/delegation-redelegation-validation
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
View file Open in desktop
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
View file Open in desktop
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
View file Open in desktop
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

AltStyle によって変換されたページ (->オリジナル) /