Skip to content

fix(recurring): reject zero-amount legs #1756

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

Closed
LeoSlrRf wants to merge 1 commit into from
Closed

fix(recurring): reject zero-amount legs #1756

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
112 changes: 106 additions & 6 deletions packages/smart-contracts/src/contracts/ERC20RecurringPaymentProxy.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ contract ERC20RecurringPaymentProxy is EIP712, AccessControl, Pausable, Reentran

error ERC20RecurringPaymentProxy__BadSignature();
error ERC20RecurringPaymentProxy__SignatureExpired();
error ERC20RecurringPaymentProxy__IndexTooLarge();
error ERC20RecurringPaymentProxy__PaymentOutOfOrder();
error ERC20RecurringPaymentProxy__IndexOutOfBounds();
error ERC20RecurringPaymentProxy__NotDueYet();
Expand All @@ -28,6 +27,12 @@ contract ERC20RecurringPaymentProxy is EIP712, AccessControl, Pausable, Reentran
error ERC20RecurringPaymentProxy__TransferFailed();
error ERC20RecurringPaymentProxy__ShortPull();
error ERC20RecurringPaymentProxy__ZeroScheduleId();
error ERC20RecurringPaymentProxy__InvalidDueTimes();
error ERC20RecurringPaymentProxy__TooManyLegs();
error ERC20RecurringPaymentProxy__EmptyLegs();
error ERC20RecurringPaymentProxy__ZeroAmount();

uint8 public constant MAX_LEGS = 8;

bytes32 public constant RELAYER_ROLE = keccak256('RELAYER_ROLE');

Expand Down Expand Up @@ -259,11 +264,15 @@ contract ERC20RecurringPaymentProxy is EIP712, AccessControl, Pausable, Reentran
}
}

function _approveFeeProxy(IERC20 token, uint256 amount) private {
if (!token.safeApprove(address(erc20FeeProxy), 0)) {
function _approveFeeProxy(
IERC20 token,
IERC20FeeProxy proxy,
uint256 amount
) private {
if (!token.safeApprove(address(proxy), 0)) {
revert ERC20RecurringPaymentProxy__TransferFailed();
}
if (!token.safeApprove(address(erc20FeeProxy), amount)) {
if (!token.safeApprove(address(proxy), amount)) {
revert ERC20RecurringPaymentProxy__TransferFailed();
}
}
Expand All @@ -283,6 +292,42 @@ contract ERC20RecurringPaymentProxy is EIP712, AccessControl, Pausable, Reentran
}
}

function _assertLegArrays(SchedulePermitBatch calldata p) private pure {
if (p.initialLegs.length > MAX_LEGS || p.recurringLegs.length > MAX_LEGS) {
revert ERC20RecurringPaymentProxy__TooManyLegs();
}
}

function _sumAndAssertLegs(Leg[] calldata legs) private pure returns (uint256 sum) {
if (legs.length == 0) revert ERC20RecurringPaymentProxy__EmptyLegs();
for (uint256 i = 0; i < legs.length; ++i) {
if (legs[i].recipient == address(0)) {
revert ERC20RecurringPaymentProxy__ZeroAddress();
}
if (legs[i].amount == 0) {
revert ERC20RecurringPaymentProxy__ZeroAmount();
}
sum += legs[i].amount;
}
}

function _settleLegs(
IERC20FeeProxy proxy,
address token,
Leg[] calldata legs
) private {
for (uint256 i = 0; i < legs.length; ++i) {
proxy.transferFromWithReferenceAndFee(
token,
legs[i].recipient,
legs[i].amount,
abi.encodePacked(legs[i].paymentReference),
0,
address(0)
);
}
}

function _proxyTransfer(SchedulePermit calldata p, bytes calldata paymentReference) private {
erc20FeeProxy.transferFromWithReferenceAndFee(
p.token,
Expand All @@ -306,7 +351,6 @@ contract ERC20RecurringPaymentProxy is EIP712, AccessControl, Pausable, Reentran
if (block.timestamp > p.deadline) revert ERC20RecurringPaymentProxy__SignatureExpired();

if (index == 0) revert ERC20RecurringPaymentProxy__IndexOutOfBounds();
if (index >= 256) revert ERC20RecurringPaymentProxy__IndexTooLarge();
if (index > p.totalPayments) revert ERC20RecurringPaymentProxy__IndexOutOfBounds();

bytes32 scheduleKey = _scheduleKeyFromPermit(p);
Expand All @@ -322,12 +366,68 @@ contract ERC20RecurringPaymentProxy is EIP712, AccessControl, Pausable, Reentran

IERC20 token = IERC20(p.token);
_pullExact(token, p.subscriber, total);
_approveFeeProxy(token, p.amount + p.feeAmount);
_approveFeeProxy(token, erc20FeeProxy, p.amount + p.feeAmount);
_proxyTransfer(p, paymentReference);
_payRelayer(token, p.relayerFee);
_markPaid(scheduleKey, index, p.strictOrder);
}

function triggerRecurringPaymentBatch(
SchedulePermitBatch calldata p,
bytes calldata signature,
uint8 index
) external whenNotPaused onlyRole(RELAYER_ROLE) nonReentrant {
if (p.token == address(0) || p.subscriber == address(0)) {
revert ERC20RecurringPaymentProxy__ZeroAddress();
}

bytes32 digest = _hashScheduleBatch(p);

_assertSigner(p.subscriber, digest, signature);
if (block.timestamp > p.deadline) revert ERC20RecurringPaymentProxy__SignatureExpired();

if (index == 0) revert ERC20RecurringPaymentProxy__IndexOutOfBounds();
if (p.totalPayments == 0 || index > p.totalPayments) {
revert ERC20RecurringPaymentProxy__IndexOutOfBounds();
}
if (p.dueTimes.length != p.totalPayments) {
revert ERC20RecurringPaymentProxy__InvalidDueTimes();
}
for (uint256 i = 1; i < p.dueTimes.length; ++i) {
if (p.dueTimes[i] <= p.dueTimes[i - 1]) {
revert ERC20RecurringPaymentProxy__InvalidDueTimes();
}
}
if (block.timestamp < p.dueTimes[index - 1]) {
revert ERC20RecurringPaymentProxy__NotDueYet();
}

_assertLegArrays(p);

bytes32 scheduleKey = _scheduleKeyFromBatch(p);
_assertOrder(scheduleKey, index, p.strictOrder);
_assertUnpaid(scheduleKey, index);

bool useInitial = p.initialLegs.length != 0 && index == 1;
uint256 legsSum = useInitial
Comment on lines +409 to +412

@greptile-apps greptile-apps Bot Aug 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 Malformed schedules execute partially

When a multi-payment permit has valid initial legs but a zero-amount recurring leg, index 1 validates only initialLegs and completes successfully; index 2 then rejects recurringLegs, leaving the malformed schedule partially executed and unable to complete.

? _sumAndAssertLegs(p.initialLegs)
: _sumAndAssertLegs(p.recurringLegs);
uint256 payerTotal = legsSum + p.relayerFee;

_markPaid(scheduleKey, index, p.strictOrder);

IERC20 token = IERC20(p.token);
IERC20FeeProxy proxy = erc20FeeProxy;
_pullExact(token, p.subscriber, payerTotal);
_approveFeeProxy(token, proxy, legsSum);
if (useInitial) {
_settleLegs(proxy, p.token, p.initialLegs);
} else {
_settleLegs(proxy, p.token, p.recurringLegs);
}
_payRelayer(token, p.relayerFee);
}

function setRelayer(address oldRelayer, address newRelayer) external onlyOwner {
if (newRelayer == address(0)) revert ERC20RecurringPaymentProxy__ZeroAddress();
_revokeRole(RELAYER_ROLE, oldRelayer);
Expand Down
23 changes: 23 additions & 0 deletions packages/smart-contracts/src/contracts/test/ERC20PullTestTokens.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,3 +60,26 @@ contract ERC20FailTransfer is ERC20 {
return false;
}
}

/**
* @notice ERC-20 that reverts transfers to a chosen recipient so a batch leg can fail.
*/
contract ERC20BlockRecipient is ERC20 {
address public blocked;

constructor(uint256 initialSupply) ERC20('Block Recipient', 'BLK') {
_mint(msg.sender, initialSupply);
}

function setBlocked(address account) external {
blocked = account;
}

function _beforeTokenTransfer(
address,
address to,
uint256
) internal view override {
require(to != blocked, 'ERC20BlockRecipient: blocked');
}
}
Loading