From e315ef9275c790b2d6722d102dfd42454159b990 Mon Sep 17 00:00:00 2001 From: giwaov Date: Tue, 7 Apr 2026 21:36:51 +0100 Subject: [PATCH 1/2] fix: security hardening - CEI pattern, over-withdrawal guard, debt settlement - Fix reentrancy risk in _executeRefund by setting refunded=true before external transfer call (checks-effects-interactions pattern) (#12) - Add cumulative withdrawal check in earlyWithdrawByArbiter to prevent over-withdrawal via repeated calls (#11) - Add _settleDebt call in refundByRecipient to prevent debt bypass (#10) --- src/RefundProtocol.sol | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/RefundProtocol.sol b/src/RefundProtocol.sol index 9fc5ece..0939824 100644 --- a/src/RefundProtocol.sol +++ b/src/RefundProtocol.sol @@ -122,6 +122,8 @@ contract RefundProtocol is EIP712 { revert CallerNotAllowed(); } + _settleDebt(msg.sender); + uint256 recipientBalance = balances[payment.to]; if (payment.amount > recipientBalance) { @@ -298,6 +300,9 @@ contract RefundProtocol is EIP712 { if (withdrawalAmount > payment.amount) { revert InvalidWithdrawalAmount(paymentID, withdrawalAmount); } + if (payment.withdrawnAmount + withdrawalAmount > payment.amount) { + revert InvalidWithdrawalAmount(paymentID, withdrawalAmount); + } if (payment.to != recipient) { revert PaymentDoesNotBelongToRecipient(); } @@ -368,10 +373,11 @@ contract RefundProtocol is EIP712 { if (payment.refunded) { revert PaymentRefunded(paymentID); } - fiatToken.transfer(payment.refundTo, payment.amount); payments[paymentID].refunded = true; + fiatToken.transfer(payment.refundTo, payment.amount); + emit Refund(paymentID, payment.refundTo, payment.amount); } From 7e6c66b144f0da23e05fb7df61d120acf8d5f2d2 Mon Sep 17 00:00:00 2001 From: giwaov Date: Wed, 13 May 2026 06:24:34 +0100 Subject: [PATCH 2/2] test: cover shared balance over-withdrawal guard --- test/RefundProtocol.t.sol | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/test/RefundProtocol.t.sol b/test/RefundProtocol.t.sol index 8630d02..178787e 100644 --- a/test/RefundProtocol.t.sol +++ b/test/RefundProtocol.t.sol @@ -372,6 +372,45 @@ contract RefundProtocolTest is Test { refundProtocol.earlyWithdrawByArbiter(paymentIDs, withdrawalAmounts, feeAmount, expiry, 0, receiver, v, r, s); } + function testEarlyWithdrawByArbiterCannotOverWithdrawPaymentWithSharedRecipientBalance() public { + vm.prank(arbiter); + refundProtocol.setLockupSeconds(receiver, 3600); + + vm.startPrank(user); + refundProtocol.pay(receiver, 100, refundTo); + refundProtocol.pay(receiver, 100, refundTo); + vm.stopPrank(); + + uint256[] memory paymentIDs = new uint256[](1); + paymentIDs[0] = 0; + uint256[] memory withdrawalAmounts = new uint256[](1); + withdrawalAmounts[0] = 60; + uint256 feeAmount = 0; + + (uint8 v1, bytes32 r1, bytes32 s1) = + _generateEarlyWithdrawalSignature(paymentIDs, withdrawalAmounts, feeAmount, expiry, 0, receiverPrivateKey); + + vm.prank(arbiter); + refundProtocol.earlyWithdrawByArbiter( + paymentIDs, withdrawalAmounts, feeAmount, expiry, 0, receiver, v1, r1, s1 + ); + + assertEq(refundProtocol.balances(receiver), 140); + assertEq(usdc.balanceOf(receiver), 60); + + (uint8 v2, bytes32 r2, bytes32 s2) = + _generateEarlyWithdrawalSignature(paymentIDs, withdrawalAmounts, feeAmount, expiry, 1, receiverPrivateKey); + + vm.prank(arbiter); + vm.expectRevert(abi.encodeWithSelector(RefundProtocol.InvalidWithdrawalAmount.selector, 0, 60)); + refundProtocol.earlyWithdrawByArbiter( + paymentIDs, withdrawalAmounts, feeAmount, expiry, 1, receiver, v2, r2, s2 + ); + + assertEq(refundProtocol.balances(receiver), 140); + assertEq(usdc.balanceOf(receiver), 60); + } + function testEarlyWithdrawByArbiterInvalidFeeAmount() public { vm.prank(arbiter); refundProtocol.setLockupSeconds(receiver, 3600);