Skip to content

Commit

Permalink
Fix snap_getBip32PublicKey manifest validation (#970)
Browse files Browse the repository at this point in the history 
* Fix snap_getBip32PublicKey manifest validation

* Fix more issues

* Add test

* Fix coverage
  • Loading branch information
@Mrtenz
Mrtenz committed Nov 16, 2022
1 parent b0512e6 commit f06d1dd
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 152 deletions.
8 changes: 4 additions & 4 deletions packages/rpc-methods/jest.config.js
View file
Expand Up @@ -5,10 +5,10 @@ module.exports = deepmerge(baseConfig, {
coveragePathIgnorePatterns: ['./src/index.ts'],
coverageThreshold: {
global: {
branches: 88.05,
functions: 89.65,
lines: 84.11,
statements: 84.11,
branches: 87.41,
functions: 87.27,
lines: 83.36,
statements: 83.36,
},
},
testTimeout: 2500,
Expand Down
20 changes: 20 additions & 0 deletions packages/rpc-methods/src/restricted/getBip32Entropy.test.ts
View file
Expand Up @@ -165,6 +165,26 @@ describe('getBip32EntropyCaveatSpecifications', () => {
).toBe('foo');
});

it('ignores unknown fields', async () => {
const fn = jest.fn().mockImplementation(() => 'foo');

expect(
await getBip32EntropyCaveatSpecifications[
SnapCaveatType.PermittedDerivationPaths
].decorator(fn, {
type: SnapCaveatType.PermittedDerivationPaths,
value: [params],
// @ts-expect-error Missing other required properties.
})({
params: {
path: ['m', "44'", "60'", "0'", '0', '1'],
curve: 'secp256k1',
compressed: true,
},
}),
).toBe('foo');
});

it('throws if the path is invalid', async () => {
const fn = jest.fn().mockImplementation(() => 'foo');

Expand Down
69 changes: 0 additions & 69 deletions packages/rpc-methods/src/restricted/getBip32PublicKey.test.ts
View file
@@ -1,7 +1,5 @@
import { SnapCaveatType } from '@metamask/snaps-utils';
import {
getBip32PublicKeyBuilder,
getBip32PublicKeyCaveatSpecifications,
getBip32PublicKeyImplementation,
} from './getBip32PublicKey';

Expand Down Expand Up @@ -45,73 +43,6 @@ describe('specificationBuilder', () => {
});
});

describe('getBip32PublicKeyCaveatSpecifications', () => {
describe('decorator', () => {
const params = { path: ['m', "44'", "60'"], curve: 'secp256k1' };

it('returns the result of the method implementation', async () => {
const fn = jest.fn().mockImplementation(() => 'foo');

expect(
await getBip32PublicKeyCaveatSpecifications[
SnapCaveatType.PermittedDerivationPaths
].decorator(fn, {
type: SnapCaveatType.PermittedDerivationPaths,
value: [params],
// @ts-expect-error Missing other required properties.
})({ params }),
).toBe('foo');
});

it('throws if the path is invalid', async () => {
const fn = jest.fn().mockImplementation(() => 'foo');

await expect(
getBip32PublicKeyCaveatSpecifications[
SnapCaveatType.PermittedDerivationPaths
].decorator(fn, {
type: SnapCaveatType.PermittedDerivationPaths,
value: [params],
// @ts-expect-error Missing other required properties.
})({ params: { ...params, path: [] } }),
).rejects.toThrow(
'Invalid BIP-32 public key path definition: At path: path -- Path must be a non-empty BIP-32 derivation path array.',
);
});

it('throws if the path is not specified in the caveats', async () => {
const fn = jest.fn().mockImplementation(() => 'foo');

await expect(
getBip32PublicKeyCaveatSpecifications[
SnapCaveatType.PermittedDerivationPaths
].decorator(fn, {
type: SnapCaveatType.PermittedDerivationPaths,
value: [params],
// @ts-expect-error Missing other required properties.
})({ params: { ...params, path: ['m', "44'", "0'"] } }),
).rejects.toThrow(
'The requested path is not permitted. Allowed paths must be specified in the snap manifest.',
);
});
});

describe('validator', () => {
it('throws if the caveat values are invalid', () => {
expect(() =>
getBip32PublicKeyCaveatSpecifications[
SnapCaveatType.PermittedDerivationPaths
].validator?.({
type: SnapCaveatType.PermittedDerivationPaths,
value: [{ path: ['foo'], curve: 'secp256k1' }],
}),
).toThrow(
'Invalid BIP-32 public key caveat: At path: value.0.path -- Path must start with "m".',
);
});
});
});

describe('getBip32PublicKeyImplementation', () => {
describe('getBip32PublicKey', () => {
it('derives the public key from the path', async () => {
Expand Down
86 changes: 22 additions & 64 deletions packages/rpc-methods/src/restricted/getBip32PublicKey.ts
View file
Expand Up @@ -3,20 +3,20 @@ import {
PermissionSpecificationBuilder,
PermissionType,
PermissionValidatorConstraint,
RestrictedMethodCaveatSpecificationConstraint,
RestrictedMethodOptions,
ValidPermissionSpecification,
} from '@metamask/controllers';
import { BIP32Node, SLIP10Node } from '@metamask/key-tree';
import {
Bip32PublicKey,
Bip32PublicKeyStruct,
Bip32Entropy,
bip32entropy,
Bip32PathStruct,
SnapCaveatType,
SnapGetBip32EntropyPermissionsStruct,
} from '@metamask/snaps-utils';
import { NonEmptyArray, assertStruct } from '@metamask/utils';
import { ethErrors } from 'eth-rpc-errors';
import { array, size, type } from 'superstruct';
import { isEqual } from '../utils';
import { boolean, enums, object, optional, type } from 'superstruct';

const targetKey = 'snap_getBip32PublicKey';

Expand Down Expand Up @@ -52,23 +52,13 @@ type GetBip32PublicKeyParameters = {
compressed?: boolean;
};

/**
* Validate a caveat path object. The object must consist of a `path` array and
* a `curve` string. Paths must start with `m`, and must contain at
* least two indices. If `ed25519` is used, this checks if all the path indices
* are hardened.
*
* @param value - The value to validate.
* @throws If the value is invalid.
*/
function validatePath(value: unknown): asserts value is Bip32PublicKey {
assertStruct(
value,
Bip32PublicKeyStruct,
'Invalid BIP-32 public key path definition',
ethErrors.rpc.invalidParams,
);
}
export const Bip32PublicKeyArgsStruct = bip32entropy(
object({
path: Bip32PathStruct,
curve: enums(['ed225519', 'secp256k1']),
compressed: optional(boolean()),
}),
);

/**
* Validate the path values associated with a caveat. This validates that the
Expand All @@ -79,10 +69,10 @@ function validatePath(value: unknown): asserts value is Bip32PublicKey {
*/
export function validateCaveatPaths(
caveat: Caveat<string, any>,
): asserts caveat is Caveat<string, Bip32PublicKey[]> {
): asserts caveat is Caveat<string, Bip32Entropy[]> {
assertStruct(
caveat,
type({ value: size(array(Bip32PublicKeyStruct), 1, Infinity) }),
type({ value: SnapGetBip32EntropyPermissionsStruct }),
'Invalid BIP-32 public key caveat',
ethErrors.rpc.internal,
);
Expand Down Expand Up @@ -129,43 +119,6 @@ export const getBip32PublicKeyBuilder = Object.freeze({
},
} as const);

export const getBip32PublicKeyCaveatSpecifications: Record<
SnapCaveatType.PermittedDerivationPaths,
RestrictedMethodCaveatSpecificationConstraint
> = {
[SnapCaveatType.PermittedDerivationPaths]: Object.freeze({
type: SnapCaveatType.PermittedDerivationPaths,
decorator: (
method,
caveat: Caveat<
SnapCaveatType.PermittedDerivationPaths,
GetBip32PublicKeyParameters[]
>,
) => {
return async (args) => {
const { params } = args;
validatePath(params);

const path = caveat.value.find(
(caveatPath) =>
isEqual(params.path, caveatPath.path) &&
caveatPath.curve === params.curve,
);

if (!path) {
throw ethErrors.provider.unauthorized({
message:
'The requested path is not permitted. Allowed paths must be specified in the snap manifest.',
});
}

return await method(args);
};
},
validator: (caveat) => validateCaveatPaths(caveat),
}),
};

/**
* Builds the method implementation for `snap_getBip32PublicKey`.
*
Expand All @@ -185,9 +138,14 @@ export function getBip32PublicKeyImplementation({
): Promise<string> {
await getUnlockPromise(true);

// `args.params` is validated by the decorator, so it's safe to assert here.
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const params = args.params!;
assertStruct(
args.params,
Bip32PublicKeyArgsStruct,
'Invalid BIP-32 public key params',
ethErrors.rpc.invalidParams,
);

const { params } = args;

const node = await SLIP10Node.fromDerivationPath({
curve: params.curve,
Expand Down
24 changes: 9 additions & 15 deletions packages/snaps-utils/src/manifest/validation.ts
View file
Expand Up @@ -112,7 +112,7 @@ export const Bip32PathStruct = refine(
},
);

const bip32entropy = <T extends { path: string[]; curve: string }, S>(
export const bip32entropy = <T extends { path: string[]; curve: string }, S>(
struct: Struct<T, S>,
) =>
refine(struct, 'BIP-32 entropy', (value) => {
Expand All @@ -128,25 +128,21 @@ const bip32entropy = <T extends { path: string[]; curve: string }, S>(

// Used outside @metamask/snap-utils
export const Bip32EntropyStruct = bip32entropy(
object({
type({
path: Bip32PathStruct,
curve: enums(['ed25519', 'secp256k1']),
}),
);

export type Bip32Entropy = Infer<typeof Bip32EntropyStruct>;

export const Bip32PublicKeyStruct = bip32entropy(
object({
path: Bip32PathStruct,
curve: enums(['ed225519', 'secp256k1']),
compressed: optional(boolean()),
}),
export const SnapGetBip32EntropyPermissionsStruct = size(
array(Bip32EntropyStruct),
1,
Infinity,
);

export type Bip32PublicKey = Infer<typeof Bip32PublicKeyStruct>;

const PermissionsStruct = type({
export const PermissionsStruct = type({
'endowment:long-running': optional(object({})),
'endowment:network-access': optional(object({})),
'endowment:transaction-insight': optional(
Expand All @@ -160,10 +156,8 @@ const PermissionsStruct = type({
snap_confirm: optional(object({})),
snap_manageState: optional(object({})),
snap_notify: optional(object({})),
snap_getBip32Entropy: optional(size(array(Bip32EntropyStruct), 1, Infinity)),
snap_getBip32PublicKey: optional(
size(array(Bip32PublicKeyStruct), 1, Infinity),
),
snap_getBip32Entropy: optional(SnapGetBip32EntropyPermissionsStruct),
snap_getBip32PublicKey: optional(SnapGetBip32EntropyPermissionsStruct),
snap_getBip44Entropy: optional(
size(
array(object({ coinType: size(integer(), 0, 2 ** 32 - 1) })),
Expand Down

0 comments on commit f06d1dd

Please sign in to comment.