Bug Description
Description
payInvoicesBatch() (contracts/src/Chainvoice.sol:279-345) validates all invoice IDs
in a single loop (checking isPaid/isCancelled) before marking any of them as paid
in a separate, subsequent loop. If the input array contains a duplicate invoice ID,
both occurrences pass validation — since neither has been marked paid yet at that
point — and the payout loop executes the transfer twice for the same invoice.
This also causes the InvoicePaid event to emit twice for a single invoice.
Impact
Not a third-party fund theft vector — the extra value comes from whatever the caller
supplies via msg.value/allowance themselves. It's a correctness/invariant bug: the
contract doesn't enforce that a batch of invoice IDs is unique, so "an invoice settles
exactly once" isn't guaranteed at the contract level.
The current official frontend is not affected, since it builds the ID list from a JS
Set and naturally deduplicates. However, the contract itself has no such guard, so
any other integration, script, or forked frontend could trigger it.
Reproduction
- Alice creates invoice #N to Bob for 1 ETH.
- Bob calls
payInvoicesBatch([N, N]) with msg.value = 2 * (1 ether + fee).
- Validation loop: both entries pass since
isPaid is false for both checks.
- Payout loop:
payable(alice).call{value: 1 ether}("") executes twice.
- Alice receives 2 ETH for a single 1 ETH invoice;
InvoicePaid fires twice.
Suggested Foundry test:
```solidity
function testDuplicateIdDoublePayment() public {
vm.prank(alice);
chainvoice.createInvoice(bob, 1 ether, address(0), "", "");
uint256 fee = chainvoice.fee();
uint256[] memory ids = new uint256;
ids[0] = 0; ids[1] = 0; // duplicate
uint256 aliceStart = alice.balance;
vm.prank(bob);
chainvoice.payInvoicesBatch{value: 2 * (1 ether + fee)}(ids);
assertEq(alice.balance, aliceStart + 2 ether); // BUG: should be + 1 ether
}
```
Suggested fix
Either:
- Mark
invoices[id].isPaid = true within the validation loop itself (fold the two
loops into one, check-then-immediately-set per ID), or
- Reject the batch upfront if it contains duplicate IDs.
Bug Description
Description
payInvoicesBatch()(contracts/src/Chainvoice.sol:279-345) validates all invoice IDsin a single loop (checking
isPaid/isCancelled) before marking any of them as paidin a separate, subsequent loop. If the input array contains a duplicate invoice ID,
both occurrences pass validation — since neither has been marked paid yet at that
point — and the payout loop executes the transfer twice for the same invoice.
This also causes the
InvoicePaidevent to emit twice for a single invoice.Impact
Not a third-party fund theft vector — the extra value comes from whatever the caller
supplies via
msg.value/allowance themselves. It's a correctness/invariant bug: thecontract doesn't enforce that a batch of invoice IDs is unique, so "an invoice settles
exactly once" isn't guaranteed at the contract level.
The current official frontend is not affected, since it builds the ID list from a JS
Setand naturally deduplicates. However, the contract itself has no such guard, soany other integration, script, or forked frontend could trigger it.
Reproduction
payInvoicesBatch([N, N])withmsg.value = 2 * (1 ether + fee).isPaidis false for both checks.payable(alice).call{value: 1 ether}("")executes twice.InvoicePaidfires twice.Suggested Foundry test:
```solidity
function testDuplicateIdDoublePayment() public {
vm.prank(alice);
chainvoice.createInvoice(bob, 1 ether, address(0), "", "");
uint256 fee = chainvoice.fee();
uint256[] memory ids = new uint256;
ids[0] = 0; ids[1] = 0; // duplicate
}
```
Suggested fix
Either:
invoices[id].isPaid = truewithin the validation loop itself (fold the twoloops into one, check-then-immediately-set per ID), or