StabilityNexus / StabilityNexus/Chainvoice

[BUG]: Duplicate invoice IDs in payInvoicesBatch() cause double payment

Open Beginner friendly
#202 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
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
  1. Alice creates invoice #N to Bob for 1 ETH.
  2. Bob calls payInvoicesBatch([N, N]) with msg.value = 2 * (1 ether + fee).
  3. Validation loop: both entries pass since isPaid is false for both checks.
  4. Payout loop: payable(alice).call{value: 1 ether}("") executes twice.
  5. 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.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.