StabilityNexus / StabilityNexus/Chainvoice
[BUG]: Duplicate invoice IDs in payInvoicesBatch() cause double payment
Nobody has claimed this yet.
- Dominant language
- JavaScript
- Stars
- 10
- Forks
- 47
- Avg merge
- 17h 47m
- Merged PRs (30d)
- 4
Description
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])withmsg.value = 2 * (1 ether + fee). - Validation loop: both entries pass since
isPaidis false for both checks. - Payout loop:
payable(alice).call{value: 1 ether}("")executes twice. - Alice receives 2 ETH for a single 1 ETH invoice;
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
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 = truewithin 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.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in contracts/src/Chainvoice.sol at payInvoicesBatch() (lines 279-345), then add the suggested Foundry regression test for duplicate invoice IDs. Verify that a batch containing the same ID twice cannot pay the invoice twice or emit InvoicePaid twice, and run the relevant contract tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- solidity
- Domain
- blockchain, payments
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100