Skip to content
Merged
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
6 changes: 4 additions & 2 deletions packages/gator-permissions-snap/src/core/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,11 @@ export type DeepRequired<TParent> = TParent extends (infer U)[]
? DeepRequired<U>[]
: TParent extends object
? {
[P in keyof TParent]-?: DeepRequired<Exclude<TParent[P], undefined>>;
[P in keyof TParent]-?: DeepRequired<
Exclude<TParent[P], undefined | null>
>;
}
: Exclude<TParent, undefined>;
: Exclude<TParent, undefined | null>;

/**
* An enum representing the time periods for which the stream rate can be calculated.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,29 @@ export function validateExpiry(expiry: string): string | undefined {
}
}

/**
* Validates that start time is before expiry.
* @param startTime - The start time string to validate.
* @param expiry - The expiry time string to validate.
* @returns Validation error message or undefined if valid.
*/
export function validateStartTimeVsExpiry(
startTime: string,
expiry: string,
): string | undefined {
try {
const startTimeDate = convertReadableDateToTimestamp(startTime);
const expiryDate = convertReadableDateToTimestamp(expiry);
if (startTimeDate >= expiryDate) {
return 'Start time must be before expiry';
}
return undefined;
} catch (error) {
// If date conversion fails, return undefined to let individual validation handle it
return undefined;
}
}

/**
* Validates that max amount is greater than initial amount.
* @param maxAmount - The maximum amount as BigInt.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
validateStartTime,
validateExpiry,
validatePeriodDuration,
validateStartTimeVsExpiry,
} from '../contextValidation';
import type {
Erc20TokenPeriodicContext,
Expand Down Expand Up @@ -78,6 +79,10 @@ export async function populatePermission({
}): Promise<PopulatedErc20TokenPeriodicPermission> {
return {
...permission,
data: {
...permission.data,
startTime: permission.data.startTime ?? Math.floor(Date.now() / 1000),
},
Comment thread
MoMannn marked this conversation as resolved.
rules: permission.rules ?? {},
};
}
Expand Down Expand Up @@ -157,7 +162,9 @@ export async function buildContext({
periodType = 'Other';
}

const startTime = permissionRequest.permission.data.startTime.toString();
const startTime =
permissionRequest.permission.data.startTime?.toString() ??
Math.floor(Date.now() / 1000).toString();

const balance = bigIntToHex(rawBalance);

Expand Down Expand Up @@ -233,6 +240,17 @@ export async function deriveMetadata({
validationErrors.expiryError = expiryError;
}

// Validate start time vs expiry (only if individual validations passed)
if (!validationErrors.startTimeError && !validationErrors.expiryError) {
const startTimeVsExpiryError = validateStartTimeVsExpiry(
permissionDetails.startTime,
expiry,
);
if (startTimeVsExpiryError) {
validationErrors.startTimeError = startTimeVsExpiryError;
}
}

return {
validationErrors,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {
zHexStr,
zPermission,
zMetaMaskPermissionData,
zAddress,
} from '@metamask/7715-permissions-shared/types';
import { z } from 'zod';

Expand All @@ -12,6 +13,7 @@ import type {
TimePeriod,
BaseMetadata,
} from '../../core/types';
import { validateStartTimeZod } from '../../utils/validate';

export type Erc20TokenPeriodicMetadata = BaseMetadata & {
validationErrors: {
Expand All @@ -38,9 +40,25 @@ export const zErc20TokenPeriodicPermission = zPermission.extend({
zMetaMaskPermissionData,
z.object({
periodAmount: zHexStr,
periodDuration: z.number(),
startTime: z.number(),
tokenAddress: zHexStr,
periodDuration: z.number().int().positive(),
startTime: z
.number()
.int()
.positive()
.nullable()
Comment thread
MoMannn marked this conversation as resolved.
.optional()
.refine(
(value) => {
if (value === undefined || value === null) {
return true;
}
return validateStartTimeZod(value);
},
{
message: 'Start time must be today or later',
},
),
tokenAddress: zAddress,
}),
),
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,15 @@ import { zErc20TokenPeriodicPermission } from './types';
/**
* Validates a permission object data specific to the permission type.
* @param permission - The ERC20 token periodic permission object to validate.
* @param expiry - The expiry time of permission request.
* @returns True if the permission data is valid, throws an error otherwise.
* @throws {Error} If any validation check fails.
*/
function validatePermissionData(
permission: Erc20TokenPeriodicPermission,
expiry: number,
): true {
const { periodAmount, periodDuration, startTime, tokenAddress } =
permission.data;
const { periodAmount, startTime } = permission.data;

validateHexInteger({
name: 'periodAmount',
Expand All @@ -27,26 +28,9 @@ function validatePermissionData(
allowZero: false,
});

if (periodDuration <= 0) {
throw new Error('Invalid periodDuration: must be a positive number');
}

if (periodDuration !== Math.floor(periodDuration)) {
throw new Error('Invalid periodDuration: must be an integer');
}

if (startTime <= 0) {
throw new Error('Invalid startTime: must be a positive number');
}

if (startTime !== Math.floor(startTime)) {
throw new Error('Invalid startTime: must be an integer');
}

if (!tokenAddress || tokenAddress === '0x') {
throw new Error(
'Invalid tokenAddress: must be a valid ERC20 token address',
);
// If startTime is not provided it default to Date.now(), expiry is always in the future so no need to check.
if (startTime && startTime >= expiry) {
throw new Error('Invalid startTime: must be before expiry');
}

return true;
Expand All @@ -71,7 +55,7 @@ export function parseAndValidatePermission(
throw new Error(extractZodError(validationError.errors));
}

validatePermissionData(validationResult);
validatePermissionData(validationResult, permissionRequest.expiry);

return {
...permissionRequest,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
validateExpiry,
validateMaxAmountVsInitialAmount,
calculateAmountPerSecond,
validateStartTimeVsExpiry,
} from '../contextValidation';
import type {
Erc20TokenStreamContext,
Expand Down Expand Up @@ -99,6 +100,7 @@ export async function populatePermission({
...permission.data,
initialAmount: permission.data.initialAmount ?? DEFAULT_INITIAL_AMOUNT,
maxAmount: permission.data.maxAmount ?? DEFAULT_MAX_AMOUNT,
startTime: permission.data.startTime ?? Math.floor(Date.now() / 1000),
},
rules: permission.rules ?? {},
};
Expand Down Expand Up @@ -183,7 +185,9 @@ export async function buildContext({
decimals,
});

const startTime = permissionRequest.permission.data.startTime.toString();
const startTime =
permissionRequest.permission.data.startTime?.toString() ??
Math.floor(Date.now() / 1000).toString();

const balance = bigIntToHex(rawBalance);

Expand Down Expand Up @@ -281,6 +285,17 @@ export async function deriveMetadata({
validationErrors.expiryError = expiryError;
}

// Validate start time vs expiry (only if individual validations passed)
if (!validationErrors.startTimeError && !validationErrors.expiryError) {
const startTimeVsExpiryError = validateStartTimeVsExpiry(
permissionDetails.startTime,
expiry,
);
if (startTimeVsExpiryError) {
validationErrors.startTimeError = startTimeVsExpiryError;
}
}

// Validate max amount vs initial amount
const maxVsInitialError = validateMaxAmountVsInitialAmount(
maxAmountResult.amount,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {
zHexStr,
zPermission,
zMetaMaskPermissionData,
zAddress,
} from '@metamask/7715-permissions-shared/types';
import { z } from 'zod';

Expand All @@ -12,6 +13,7 @@ import type {
BaseContext,
BaseMetadata,
} from '../../core/types';
import { validateStartTimeZod } from '../../utils/validate';

export type Erc20TokenStreamMetadata = BaseMetadata & {
amountPerSecond: string;
Expand Down Expand Up @@ -42,8 +44,24 @@ export const zErc20TokenStreamPermission = zPermission.extend({
initialAmount: zHexStr.optional(),
maxAmount: zHexStr.optional(),
amountPerSecond: zHexStr,
startTime: z.number(),
tokenAddress: zHexStr,
startTime: z
.number()
.int()
.positive()
.nullable()
.optional()
.refine(
(value) => {
if (value === undefined || value === null) {
return true;
}
return validateStartTimeZod(value);
},
{
message: 'Start time must be today or later',
},
),
tokenAddress: zAddress,
}),
),
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,15 @@ import { zErc20TokenStreamPermission } from './types';
/**
* Validates a permission object data specific to the permission type.
* @param permission - The ERC20 token stream permission object to validate.
* @param expiry - The expiry time of permission request.
* @returns True if the permission data is valid, throws an error otherwise.
* @throws {Error} If any validation check fails.
*/
function validatePermissionData(permission: Erc20TokenStreamPermission): true {
const { initialAmount, maxAmount, amountPerSecond, startTime, tokenAddress } =
function validatePermissionData(
permission: Erc20TokenStreamPermission,
expiry: number,
): true {
const { initialAmount, maxAmount, amountPerSecond, startTime } =
permission.data;

validateHexInteger({
Expand All @@ -29,7 +33,7 @@ function validatePermissionData(permission: Erc20TokenStreamPermission): true {
name: 'initialAmount',
value: initialAmount,
required: false,
allowZero: false,
allowZero: true,
});

validateHexInteger({
Expand All @@ -43,18 +47,9 @@ function validatePermissionData(permission: Erc20TokenStreamPermission): true {
throw new Error('Invalid maxAmount: must be greater than initialAmount');
}

if (startTime <= 0) {
throw new Error('Invalid startTime: must be a positive number');
}

if (startTime !== Math.floor(startTime)) {
throw new Error('Invalid startTime: must be an integer');
}

if (!tokenAddress || tokenAddress === '0x') {
throw new Error(
'Invalid tokenAddress: must be a valid ERC20 token address',
);
// If startTime is not provided it default to Date.now(), expiry is always in the future so no need to check.
if (startTime && startTime >= expiry) {
throw new Error('Invalid startTime: must be before expiry');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Default startTime Validation Flaw

The validatePermissionData function's startTime validation is incomplete. The if (startTime && startTime >= expiry) check only applies when startTime is explicitly provided, missing validation for the default Date.now() value. This creates a race condition where the populated startTime could become >= expiry, allowing an invalid permission to pass this specific validation. The accompanying comment is misleading.

Additional Locations (1)
Fix in Cursor Fix in Web

}

return true;
Expand All @@ -79,7 +74,7 @@ export function parseAndValidatePermission(
throw new Error(extractZodError(validationError.errors));
}

validatePermissionData(validationResult);
validatePermissionData(validationResult, permissionRequest.expiry);

return {
...permissionRequest,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
validateStartTime,
validateExpiry,
validatePeriodDuration,
validateStartTimeVsExpiry,
} from '../contextValidation';
import type {
NativeTokenPeriodicContext,
Expand Down Expand Up @@ -77,6 +78,10 @@ export async function populatePermission({
}): Promise<PopulatedNativeTokenPeriodicPermission> {
return {
...permission,
data: {
...permission.data,
startTime: permission.data.startTime ?? Math.floor(Date.now() / 1000),
},
rules: permission.rules ?? {},
};
}
Expand Down Expand Up @@ -154,7 +159,9 @@ export async function buildContext({
periodType = 'Other';
}

const startTime = permissionRequest.permission.data.startTime.toString();
const startTime =
permissionRequest.permission.data.startTime?.toString() ??
Math.floor(Date.now() / 1000).toString();

const balance = bigIntToHex(rawBalance);

Expand Down Expand Up @@ -230,6 +237,17 @@ export async function deriveMetadata({
validationErrors.expiryError = expiryError;
}

// Validate start time vs expiry (only if individual validations passed)
if (!validationErrors.startTimeError && !validationErrors.expiryError) {
const startTimeVsExpiryError = validateStartTimeVsExpiry(
permissionDetails.startTime,
expiry,
);
if (startTimeVsExpiryError) {
validationErrors.startTimeError = startTimeVsExpiryError;
}
}

return {
validationErrors,
};
Expand Down
Loading
Loading