Skip to content

Fix: ERC-4494 permit() does not consume the token nonce (signature replay / unrevocable approvals) #295

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

Fix: ERC-4494 permit() does not consume the token nonce (signature replay / unrevocable approvals) #295

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
2 changes: 2 additions & 0 deletions contracts/token/erc721/abstract/ERC721HybridPermit.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ abstract contract ERC721HybridPermit is ERC721Hybrid, IERC4494, EIP712 {

// smart contract wallet signature validation
if (_isValidERC1271Signature(ownerOf(tokenId), digest, sig)) {
_nonces[tokenId]++;
_approve(spender, tokenId);
return;
}
Expand All @@ -121,6 +122,7 @@ abstract contract ERC721HybridPermit is ERC721Hybrid, IERC4494, EIP712 {
}

if (_isValidEOASignature(recoveredSigner, tokenId)) {
_nonces[tokenId]++;
_approve(spender, tokenId);
} else {
revert InvalidSignature();
Expand Down
2 changes: 2 additions & 0 deletions contracts/token/erc721/abstract/ERC721HybridPermitV2.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ abstract contract ERC721HybridPermitV2 is ERC721HybridV2, IERC4494, EIP712 {

// smart contract wallet signature validation
if (_isValidERC1271Signature(ownerOf(tokenId), digest, sig)) {
_nonces[tokenId]++;
_approve(spender, tokenId);
return;
}
Expand All @@ -123,6 +124,7 @@ abstract contract ERC721HybridPermitV2 is ERC721HybridV2, IERC4494, EIP712 {
}

if (_isValidEOASignature(recoveredSigner, tokenId)) {
_nonces[tokenId]++;
_approve(spender, tokenId);
} else {
revert InvalidSignature();
Expand Down
2 changes: 2 additions & 0 deletions contracts/token/erc721/abstract/ERC721Permit.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ abstract contract ERC721Permit is ERC721Burnable, IERC4494, EIP712, IImmutableER

// smart contract signature validation
if (_isValidERC1271Signature(ownerOf(tokenId), digest, sig)) {
_nonces[tokenId]++;
_approve(spender, tokenId);
return;
}
Expand All @@ -122,6 +123,7 @@ abstract contract ERC721Permit is ERC721Burnable, IERC4494, EIP712, IImmutableER
}

if (_isValidEOASignature(recoveredSigner, tokenId)) {
_nonces[tokenId]++;
_approve(spender, tokenId);
} else {
revert InvalidSignature();
Expand Down
33 changes: 33 additions & 0 deletions test/token/erc721/ERC721OperationalBase.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -484,4 +484,37 @@ abstract contract ERC721OperationalBaseTest is ERC721BaseTest {
erc721.permit(user2, tokenId, deadline, signature);
assertEq(erc721.getApproved(tokenId), user2);
}

/**
* @notice A permit must consume the token nonce (ERC-4494). A successful
* permit therefore cannot be replayed, and an owner's `approve(address(0))`
* durably revokes the approval (the prior signature is no longer valid).
*/
function testPermitConsumesNonceAndCannotBeReplayed() public {
uint256 tokenId = 1;
vm.prank(minter);
erc721.mint(user1, tokenId);

uint256 deadline = block.timestamp + 1 days;
uint256 nonce = erc721.nonces(tokenId);
bytes memory signature = getSignature(user1Pkey, user2, tokenId, nonce, deadline);

vm.prank(user2);
erc721.permit(user2, tokenId, deadline, signature);
assertEq(erc721.getApproved(tokenId), user2);

// The nonce must advance on a successful permit.
assertEq(erc721.nonces(tokenId), nonce + 1);

// Owner revokes the approval.
vm.prank(user1);
erc721.approve(address(0), tokenId);
assertEq(erc721.getApproved(tokenId), address(0));

// Replaying the original signature must now fail: revocation is durable.
vm.prank(user2);
vm.expectRevert(abi.encodeWithSelector(IImmutableERC721Errors.InvalidSignature.selector));
erc721.permit(user2, tokenId, deadline, signature);
assertEq(erc721.getApproved(tokenId), address(0));
}
}