Skip to content

fix: reject future timestamps for Lock/Decrease/Unlock modes #64

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.

Sign up for GitHub

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
re1ro wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: reject future timestamps for Lock/Decrease/Unlock modes #64

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 6 additions & 6 deletions src/Permit3.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,11 @@ contract Permit3 is IPermit3, MultiTokenPermit, NonceManager {
revert ZeroAccount();
}

// Prevent setting timestamps in the future for all timestamp-storing modes
if (block.timestamp < timestamp) {
revert InvalidTimestamp(timestamp, uint48(block.timestamp));
}

Allowance memory allowed = allowances[owner][p.tokenKey][p.account];

// Validate lock status before processing
Expand Down Expand Up @@ -465,7 +470,7 @@ contract Permit3 is IPermit3, MultiTokenPermit, NonceManager {
Allowance memory allowed,
AllowanceOrTransfer memory p,
uint48 timestamp
) private view {
) private pure {
// Handle amount increase if specified
if (p.amountDelta > 0) {
if (allowed.amount != MAX_ALLOWANCE) {
Expand All @@ -477,11 +482,6 @@ contract Permit3 is IPermit3, MultiTokenPermit, NonceManager {
}
}

// Prevent setting timestamps in the future
if (block.timestamp < timestamp) {
revert InvalidTimestamp(timestamp, uint48(block.timestamp));
}

// Update expiration and timestamp based on precedence rules
if (timestamp > allowed.timestamp) {
allowed.expiration = p.modeOrExpiration;
Expand Down
76 changes: 69 additions & 7 deletions test/Permit3Edge.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -712,6 +712,59 @@ contract Permit3EdgeTest is Test {
);
}

function test_lockTimestampInFuture() public {
// A Lock permit must not be allowed to store a future timestamp. Otherwise the
// lock would be effectively un-unlockable, since Unlock requires a timestamp
// strictly greater than the stored lock timestamp.

// Create a permit to lock the allowance with a future timestamp
PermitInputs memory lockInputs;
lockInputs.permits = new IPermit3.AllowanceOrTransfer[](1);
lockInputs.permits[0] = IPermit3.AllowanceOrTransfer({
modeOrExpiration: uint48(IPermit3.PermitType.Lock), // Lock mode (2)
tokenKey: bytes32(uint256(uint160(address(token)))),
account: spender,
amountDelta: 0 // Not used for lock
});

lockInputs.chainPermits = IPermit3.ChainPermits({ chainId: uint64(block.chainid), permits: lockInputs.permits });

TestParams memory lockParams;
lockParams.salt = bytes32(uint256(0x446));
lockParams.deadline = uint48(block.timestamp + 1 hours);
lockParams.timestamp = uint48(block.timestamp + 1000); // Future timestamp

// Sign the lock permit
bytes32 lockDataHash = permit3Tester.hashChainPermits(lockInputs.chainPermits);
bytes32 lockSignedHash = keccak256(
abi.encode(
permit3.SIGNED_PERMIT3_TYPEHASH(),
owner,
lockParams.salt,
lockParams.deadline,
lockParams.timestamp,
lockDataHash
)
);

bytes32 lockDigest = _getDigest(lockSignedHash);
(uint8 v, bytes32 r, bytes32 s) = vm.sign(ownerPrivateKey, lockDigest);
lockParams.signature = abi.encodePacked(r, s, v);

// Should revert with InvalidTimestamp - a Lock cannot store a future timestamp
vm.expectRevert(
abi.encodeWithSelector(IPermit.InvalidTimestamp.selector, lockParams.timestamp, uint48(block.timestamp))
);
permit3.permit(
owner,
lockParams.salt,
lockParams.deadline,
lockParams.timestamp,
lockInputs.chainPermits.permits,
lockParams.signature
);
}

function test_lockAndUnlockAllowance() public {
// First set up an initial allowance
vm.prank(owner);
Expand Down Expand Up @@ -767,6 +820,12 @@ contract Permit3EdgeTest is Test {
assertEq(expiration, 2); // LOCKED_ALLOWANCE is 2
assertEq(ts, lockParams.timestamp);

// Remember the lock timestamp before advancing the clock
uint48 lockTimestamp = lockParams.timestamp;

// Advance the chain so the unlock can use a newer (but not future) timestamp
vm.warp(block.timestamp + 100);

// Now create a permit to unlock with a newer timestamp
PermitInputs memory unlockInputs;
unlockInputs.permits = new IPermit3.AllowanceOrTransfer[](1);
Expand All @@ -783,7 +842,7 @@ contract Permit3EdgeTest is Test {
TestParams memory unlockParams;
unlockParams.salt = bytes32(uint256(0x777));
unlockParams.deadline = uint48(block.timestamp + 1 hours);
unlockParams.timestamp = uint48(block.timestamp + 100); // Newer timestamp
unlockParams.timestamp = uint48(block.timestamp); // Newer than lock timestamp, not in the future

// Sign the unlock permit
bytes32 unlockDataHash = permit3Tester.hashChainPermits(unlockInputs.chainPermits);
Expand Down Expand Up @@ -817,7 +876,7 @@ contract Permit3EdgeTest is Test {
assertEq(amount, 0); // Amount remains unchanged by unlock operation
assertEq(expiration, 0); // No expiration (unlocked)
// Note: timestamp should remain from lock operation since unlock only changes expiration
assertEq(ts, uint48(block.timestamp)); // Timestamp remains from lock operation
assertEq(ts, lockTimestamp); // Timestamp remains from lock operation
}

function test_attemptUnlockWithOlderTimestamp() public {
Expand Down Expand Up @@ -935,7 +994,7 @@ contract Permit3EdgeTest is Test {
TestParams memory params;
params.salt = bytes32(uint256(0xaaa));
params.deadline = uint48(block.timestamp + 1 hours);
params.timestamp = uint48(block.timestamp + 100);
params.timestamp = uint48(block.timestamp);

PermitInputs memory inputs;
inputs.permits = new IPermit3.AllowanceOrTransfer[](1);
Expand Down Expand Up @@ -979,7 +1038,7 @@ contract Permit3EdgeTest is Test {
TestParams memory params;
params.salt = bytes32(uint256(0xbbb));
params.deadline = uint48(block.timestamp + 1 hours);
params.timestamp = uint48(block.timestamp + 100);
params.timestamp = uint48(block.timestamp);

PermitInputs memory inputs;
inputs.permits = new IPermit3.AllowanceOrTransfer[](1);
Expand Down Expand Up @@ -1070,7 +1129,7 @@ contract Permit3EdgeTest is Test {
TestParams memory params;
params.salt = bytes32(uint256(0xddd));
params.deadline = uint48(block.timestamp + 1 hours);
params.timestamp = uint48(block.timestamp + 100);
params.timestamp = uint48(block.timestamp);

PermitInputs memory inputs;
inputs.permits = new IPermit3.AllowanceOrTransfer[](1);
Expand Down Expand Up @@ -1288,8 +1347,11 @@ contract Permit3EdgeTest is Test {
vm.prank(owner);
permit3.lockdown(pairs);

// Create unlock permit with newer timestamp
uint48 newerTimestamp = uint48(block.timestamp) + 1;
// Advance the clock so the unlock can use a newer (but not future) timestamp than the lock
vm.warp(block.timestamp + 1);

// Create unlock permit with newer timestamp (now equal to the current block time)
uint48 newerTimestamp = uint48(block.timestamp);

PermitInputs memory inputs;
inputs.permits = new IPermit3.AllowanceOrTransfer[](1);
Expand Down
Loading