diff --git a/packages/smart-contracts/src/contracts/ERC20RecurringPaymentProxy.sol b/packages/smart-contracts/src/contracts/ERC20RecurringPaymentProxy.sol index 140daa624c..faaabc3cd9 100644 --- a/packages/smart-contracts/src/contracts/ERC20RecurringPaymentProxy.sol +++ b/packages/smart-contracts/src/contracts/ERC20RecurringPaymentProxy.sol @@ -5,7 +5,7 @@ import '@openzeppelin/contracts/access/AccessControl.sol'; import '@openzeppelin/contracts/security/Pausable.sol'; import '@openzeppelin/contracts/security/ReentrancyGuard.sol'; import '@openzeppelin/contracts/utils/cryptography/EIP712.sol'; -import '@openzeppelin/contracts/utils/cryptography/ECDSA.sol'; +import '@openzeppelin/contracts/utils/cryptography/SignatureChecker.sol'; import '@openzeppelin/contracts/access/Ownable.sol'; import './interfaces/ERC20FeeProxy.sol'; import './lib/SafeERC20.sol'; @@ -16,7 +16,6 @@ import './lib/SafeERC20.sol'; */ contract ERC20RecurringPaymentProxy is EIP712, AccessControl, Pausable, ReentrancyGuard, Ownable { using SafeERC20 for IERC20; - using ECDSA for bytes32; error ERC20RecurringPaymentProxy__BadSignature(); error ERC20RecurringPaymentProxy__SignatureExpired(); @@ -161,6 +160,16 @@ contract ERC20RecurringPaymentProxy is EIP712, AccessControl, Pausable, Reentran return _hashScheduleBatch(p); } + function _assertSigner( + address subscriber, + bytes32 digest, + bytes calldata signature + ) private view { + if (!SignatureChecker.isValidSignatureNow(subscriber, digest, signature)) { + revert ERC20RecurringPaymentProxy__BadSignature(); + } + } + function _proxyTransfer(SchedulePermit calldata p, bytes calldata paymentReference) private { erc20FeeProxy.transferFromWithReferenceAndFee( p.token, @@ -180,8 +189,7 @@ contract ERC20RecurringPaymentProxy is EIP712, AccessControl, Pausable, Reentran ) external whenNotPaused onlyRole(RELAYER_ROLE) nonReentrant { bytes32 digest = _hashSchedule(p); - if (digest.recover(signature) != p.subscriber) - revert ERC20RecurringPaymentProxy__BadSignature(); + _assertSigner(p.subscriber, digest, signature); if (block.timestamp > p.deadline) revert ERC20RecurringPaymentProxy__SignatureExpired(); if (index >= 256) revert ERC20RecurringPaymentProxy__IndexTooLarge(); diff --git a/packages/smart-contracts/src/contracts/test/MockERC1271.sol b/packages/smart-contracts/src/contracts/test/MockERC1271.sol new file mode 100644 index 0000000000..810da72204 --- /dev/null +++ b/packages/smart-contracts/src/contracts/test/MockERC1271.sol @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.0; + +import '@openzeppelin/contracts/token/ERC20/IERC20.sol'; + +/** + * @notice Minimal ERC-1271 wallet for recurring-proxy signature tests. + */ +contract MockERC1271 { + bytes4 private constant _MAGICVALUE = 0x1626ba7e; + + address public immutable owner; + + constructor(address _owner) { + owner = _owner; + } + + function isValidSignature(bytes32 hash, bytes memory signature) external view returns (bytes4) { + if (signature.length != 65) { + return 0xffffffff; + } + + bytes32 r; + bytes32 s; + uint8 v; + // solhint-disable-next-line no-inline-assembly + assembly { + r := mload(add(signature, 32)) + s := mload(add(signature, 64)) + v := byte(0, mload(add(signature, 96))) + } + + address recovered = ecrecover(hash, v, r, s); + if (recovered != address(0) && recovered == owner) { + return _MAGICVALUE; + } + return 0xffffffff; + } + + function approveToken( + address token, + address spender, + uint256 amount + ) external { + require(msg.sender == owner, 'MockERC1271: not owner'); + IERC20(token).approve(spender, amount); + } +} diff --git a/packages/smart-contracts/test/contracts/ERC20RecurringPaymentProxy.test.ts b/packages/smart-contracts/test/contracts/ERC20RecurringPaymentProxy.test.ts index 155dc3fd03..dec7e050c5 100644 --- a/packages/smart-contracts/test/contracts/ERC20RecurringPaymentProxy.test.ts +++ b/packages/smart-contracts/test/contracts/ERC20RecurringPaymentProxy.test.ts @@ -675,6 +675,73 @@ describe('ERC20RecurringPaymentProxy', () => { }); }); + describe('EIP-1271 signatures', () => { + const paymentReference = '0x1234567890abcdef'; + + it('accepts a valid smart-account signature', async () => { + const MockERC1271Factory = await ethers.getContractFactory('MockERC1271'); + const mockWallet = await MockERC1271Factory.deploy(subscriberAddress); + await mockWallet.deployed(); + + await testERC20.transfer(mockWallet.address, 500); + await mockWallet + .connect(subscriber) + .approveToken(testERC20.address, erc20RecurringPaymentProxy.address, 500); + + const permit = createSchedulePermit({ subscriber: mockWallet.address }); + const signature = await createSignature(permit, subscriber); + + await expect( + erc20RecurringPaymentProxy + .connect(relayer) + .triggerRecurringPayment(permit, signature, 1, paymentReference), + ) + .to.emit(erc20FeeProxy, 'TransferWithReferenceAndFee') + .withArgs( + testERC20.address, + recipientAddress, + permit.amount, + ethers.utils.keccak256(paymentReference), + permit.feeAmount, + feeAddressString, + ); + }); + + it('rejects a malformed smart-account signature', async () => { + const MockERC1271Factory = await ethers.getContractFactory('MockERC1271'); + const mockWallet = await MockERC1271Factory.deploy(subscriberAddress); + await mockWallet.deployed(); + + await testERC20.transfer(mockWallet.address, 500); + await mockWallet + .connect(subscriber) + .approveToken(testERC20.address, erc20RecurringPaymentProxy.address, 500); + + const permit = createSchedulePermit({ subscriber: mockWallet.address }); + const signature = '0x' + '11'.repeat(65); + + await expect( + erc20RecurringPaymentProxy + .connect(relayer) + .triggerRecurringPayment(permit, signature, 1, paymentReference), + ).to.be.reverted; + }); + + it('still accepts an EOA signature through SignatureChecker', async () => { + await testERC20.transfer(subscriberAddress, 500); + await testERC20.connect(subscriber).approve(erc20RecurringPaymentProxy.address, 500); + + const permit = createSchedulePermit(); + const signature = await createSignature(permit, subscriber); + + await expect( + erc20RecurringPaymentProxy + .connect(relayer) + .triggerRecurringPayment(permit, signature, 1, paymentReference), + ).to.emit(erc20FeeProxy, 'TransferWithReferenceAndFee'); + }); + }); + describe('Integration: Paused state affects execution', () => { it('should revert trigger when contract is paused', async () => { await erc20RecurringPaymentProxy.pause();