From 8b5ecb3792b14de7924601509c81ad232edf74ab Mon Sep 17 00:00:00 2001 From: LeoSlrRf Date: Mon, 24 Aug 2026 14:07:25 +0200 Subject: [PATCH] fix(recurring): guard rescueTokens with nonReentrant --- .../contracts/ERC20RecurringPaymentProxy.sol | 21 ++++++++ .../ERC20RecurringPaymentProxy.test.ts | 50 +++++++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/packages/smart-contracts/src/contracts/ERC20RecurringPaymentProxy.sol b/packages/smart-contracts/src/contracts/ERC20RecurringPaymentProxy.sol index 17d52686e..f7468f3c9 100644 --- a/packages/smart-contracts/src/contracts/ERC20RecurringPaymentProxy.sol +++ b/packages/smart-contracts/src/contracts/ERC20RecurringPaymentProxy.sol @@ -277,6 +277,12 @@ contract ERC20RecurringPaymentProxy is EIP712, AccessControl, Pausable, Reentran } } + function _assertNonZeroRecipient(address account, uint256 amount) private pure { + if (amount > 0 && account == address(0)) { + revert ERC20RecurringPaymentProxy__ZeroAddress(); + } + } + function _proxyTransfer(SchedulePermit calldata p, bytes calldata paymentReference) private { erc20FeeProxy.transferFromWithReferenceAndFee( p.token, @@ -310,6 +316,8 @@ contract ERC20RecurringPaymentProxy is EIP712, AccessControl, Pausable, Reentran uint256 execTime = uint256(p.firstPayment) + uint256(index - 1) * p.periodSeconds; if (block.timestamp < execTime) revert ERC20RecurringPaymentProxy__NotDueYet(); + _assertNonZeroRecipient(p.feeAddress, p.feeAmount); + uint256 total = p.amount + p.feeAmount + p.relayerFee; IERC20 token = IERC20(p.token); @@ -338,4 +346,17 @@ contract ERC20RecurringPaymentProxy is EIP712, AccessControl, Pausable, Reentran function unpause() external onlyOwner { _unpause(); } + + function rescueTokens( + address token, + address to, + uint256 amount + ) external onlyOwner nonReentrant { + if (token == address(0) || to == address(0)) { + revert ERC20RecurringPaymentProxy__ZeroAddress(); + } + if (!IERC20(token).safeTransfer(to, amount)) { + revert ERC20RecurringPaymentProxy__TransferFailed(); + } + } } diff --git a/packages/smart-contracts/test/contracts/ERC20RecurringPaymentProxy.test.ts b/packages/smart-contracts/test/contracts/ERC20RecurringPaymentProxy.test.ts index 79b7925ce..6bf857c92 100644 --- a/packages/smart-contracts/test/contracts/ERC20RecurringPaymentProxy.test.ts +++ b/packages/smart-contracts/test/contracts/ERC20RecurringPaymentProxy.test.ts @@ -380,6 +380,56 @@ describe('ERC20RecurringPaymentProxy', () => { }); }); + describe('Fee destination and rescue', () => { + const paymentReference = '0x1234567890abcdef'; + + it('reverts when feeAmount is non-zero and feeAddress is zero', async () => { + await testERC20.transfer(subscriberAddress, 500); + await testERC20.connect(subscriber).approve(erc20RecurringPaymentProxy.address, 500); + + const permit = createSchedulePermit({ feeAddress: ethers.constants.AddressZero }); + const signature = await createSignature(permit, subscriber); + const scheduleKey = await erc20RecurringPaymentProxy.scheduleKeyFromPermit(permit); + + await expect( + erc20RecurringPaymentProxy + .connect(relayer) + .triggerRecurringPayment(permit, signature, 1, paymentReference), + ).to.be.revertedWith('ERC20RecurringPaymentProxy__ZeroAddress'); + expect(await erc20RecurringPaymentProxy.triggeredPaymentsBitmap(scheduleKey)).to.equal(0); + }); + + it('allows the owner to rescue a residual balance', async () => { + await testERC20.transfer(erc20RecurringPaymentProxy.address, 40); + const ownerBalanceBefore = await testERC20.balanceOf(ownerAddress); + + await erc20RecurringPaymentProxy.rescueTokens(testERC20.address, ownerAddress, 40); + + expect(await testERC20.balanceOf(erc20RecurringPaymentProxy.address)).to.equal(0); + expect(await testERC20.balanceOf(ownerAddress)).to.equal(ownerBalanceBefore.add(40)); + }); + + it('reverts when a non-owner tries to rescue tokens', async () => { + await testERC20.transfer(erc20RecurringPaymentProxy.address, 10); + + await expect( + erc20RecurringPaymentProxy.connect(user).rescueTokens(testERC20.address, userAddress, 10), + ).to.be.revertedWith('Ownable: caller is not the owner'); + }); + + it('reverts rescue to the zero address', async () => { + await testERC20.transfer(erc20RecurringPaymentProxy.address, 10); + + await expect( + erc20RecurringPaymentProxy.rescueTokens( + testERC20.address, + ethers.constants.AddressZero, + 10, + ), + ).to.be.revertedWith('ERC20RecurringPaymentProxy__ZeroAddress'); + }); + }); + describe('Trigger Recurring Payment', () => { beforeEach(async () => { // Transfer tokens to subscriber and approve the recurring payment proxy