Skip to content

Unbounded Loop in getSentInvoices / getReceivedInvoices (Potential DoS) --->add pagination #129

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

Draft
aniket866 wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Unbounded Loop in getSentInvoices / getReceivedInvoices (Potential DoS) --->add pagination #129

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a3ff297
fixing-unbounded-loop
aniket866 Feb 28, 2026
e52f917
Merge branch 'main' into contract-fix
aniket866 Mar 25, 2026
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
45 changes: 37 additions & 8 deletions contracts/src/Chainvoice.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,19 +378,48 @@ contract Chainvoice {
}
}

function getSentInvoices(address user) external view returns (InvoiceDetails[] memory) {
return _getInvoices(sentInvoices[user]);
function getSentInvoices(
address user,
uint256 offset,
uint256 limit
) external view returns (InvoiceDetails[] memory) {
return _getInvoicesPaginated(sentInvoices[user], offset, limit);
}

function getReceivedInvoices(address user) external view returns (InvoiceDetails[] memory) {
return _getInvoices(receivedInvoices[user]);
function getReceivedInvoices(
address user,
uint256 offset,
uint256 limit
) external view returns (InvoiceDetails[] memory) {
return _getInvoicesPaginated(receivedInvoices[user], offset, limit);
}
Comment on lines +381 to 395
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 25, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for getSentInvoices and getReceivedInvoices calls that may use old signature
echo "=== Checking getSentInvoices calls ==="
rg -n "getSentInvoices\s*\(" --type=js --type=jsx -C2

echo "=== Checking getReceivedInvoices calls ==="
rg -n "getReceivedInvoices\s*\(" --type=js --type=jsx -C2

echo "=== Checking ABI definitions ==="
rg -n -A10 '"getSentInvoices"' frontend/
rg -n -A10 '"getReceivedInvoices"' frontend/

Repository: StabilityNexus/Chainvoice

Length of output: 1670

🏁 Script executed:

#!/bin/bash
# Search more broadly across all file types
echo "=== All files in frontend directory ==="
find frontend -type f \( -name "*.js" -o -name "*.jsx" -o -name "*.ts" -o -name "*.tsx" \) | head -20

echo "=== Search for getSentInvoices in all frontend files ==="
rg -l "getSentInvoices" frontend/ 2>/dev/null || echo "No matches found"

echo "=== Search for getReceivedInvoices in all frontend files ==="
rg -l "getReceivedInvoices" frontend/ 2>/dev/null || echo "No matches found"

echo "=== Check if SentInvoice.jsx or similar file exists ==="
find frontend -name "*SentInvoice*" -type f

echo "=== Check contract function signatures in Chainvoice.sol ==="
sed -n '381,395p' contracts/src/Chainvoice.sol

Repository: StabilityNexus/Chainvoice

Length of output: 1808

🏁 Script executed:

#!/bin/bash
echo "=== SentInvoice.jsx getSentInvoices calls ==="
rg -n "getSentInvoices" frontend/src/page/SentInvoice.jsx -B3 -A3

echo "=== ReceivedInvoice.jsx getReceivedInvoices calls ==="
rg -n "getReceivedInvoices" frontend/src/page/ReceivedInvoice.jsx -B3 -A3

echo "=== BatchPayment.jsx getReceivedInvoices calls ==="
rg -n "getReceivedInvoices" frontend/src/page/BatchPayment.jsx -B3 -A3

Repository: StabilityNexus/Chainvoice

Length of output: 1178

Update ABI definitions and SentInvoice.jsx to match new function signatures.

The contract's getSentInvoices and getReceivedInvoices functions now require three parameters: (address user, uint256 offset, uint256 limit). However, frontend/src/contractsABI/ChainvoiceABI.js still declares only a single user parameter for both functions, causing ethers.js to encode/decode calls incorrectly at runtime.

Additionally, frontend/src/page/SentInvoice.jsx (line 181) calls getSentInvoices(address) with only one argument and must be updated to pass offset and limit parameters like the other call sites. Note that ReceivedInvoice.jsx (line 690) and BatchPayment.jsx (line 363) have already been updated to use the correct three-parameter signature.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@contracts/src/Chainvoice.sol` around lines 381 - 395, The ABI for
getSentInvoices and getReceivedInvoices is out-of-date and SentInvoice.jsx calls
getSentInvoices with only the user address; update
frontend/src/contractsABI/ChainvoiceABI.js to declare both functions with three
inputs (address user, uint256 offset, uint256 limit) matching the contract
signature, and update frontend/src/page/SentInvoice.jsx to call
getSentInvoices(userAddress, offset, limit) (use the same offset/limit variables
or defaults used in ReceivedInvoice.jsx/BatchPayment.jsx) so ethers.js
encodes/decodes calls correctly.


function _getInvoices(uint256[] storage ids) internal view returns (InvoiceDetails[] memory) {
InvoiceDetails[] memory result = new InvoiceDetails[](ids.length);
for (uint256 i = 0; i < ids.length; i++) {
result[i] = invoices[ids[i]];
function _getInvoicesPaginated(
uint256[] storage ids,
uint256 offset,
uint256 limit
) internal view returns (InvoiceDetails[] memory) {
uint256 total = ids.length;

// If the offset is beyond the array size, return an empty array
if (offset >= total) {
return new InvoiceDetails[](0);
}

// Calculate the actual limit (prevent out-of-bounds)
uint256 end = offset + limit;
if (end > total) {
end = total;
}

uint256 size = end - offset;
InvoiceDetails[] memory result = new InvoiceDetails[](size);

// Only loop through the requested chunk
for (uint256 i = 0; i < size; i++) {
result[i] = invoices[ids[offset + i]];
}

return result;
}

Expand Down
10 changes: 7 additions & 3 deletions contracts/test/Chainvoice.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ contract ChainvoiceTest is Test {
}

/* ------------------------------------------------------------ */
/* CREATE INVOICE */
/* CREATE INVOICE */
/* ------------------------------------------------------------ */

function testCreateInvoice_Native() public {
Expand All @@ -31,12 +31,16 @@ contract ChainvoiceTest is Test {
"hash123"
);

// FIX: Added offset (0) and limit (10) for pagination
Chainvoice.InvoiceDetails[] memory sent = chainvoice.getSentInvoices(
alice
alice,
0,
10
);

// FIX: Added offset (0) and limit (10) for pagination
Chainvoice.InvoiceDetails[] memory received = chainvoice
.getReceivedInvoices(bob);
.getReceivedInvoices(bob, 0, 10);
Comment on lines +34 to +43
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 25, 2026

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Add test coverage for pagination edge cases.

The updated test only verifies the happy path with (0, 10). Consider adding tests for:

  • offset >= total returns empty array
  • offset + limit > total returns partial results
  • limit = 0 behavior
  • Large offset values
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@contracts/test/Chainvoice.t.sol` around lines 34 - 43, Extend the
Chainvoice.t.sol tests to cover pagination edge cases for getSentInvoices and
getReceivedInvoices: add assertions that calling
chainvoice.getSentInvoices(alice, offset, limit) and
chainvoice.getReceivedInvoices(bob, offset, limit) returns an empty
Chainvoice.InvoiceDetails[] when offset >= total, returns a partial array when
offset + limit > total (verify length equals total - offset), handles limit = 0
(expect empty array or defined behavior) and tolerates very large offsets (e.g.,
huge offset values) without reverting; implement these as separate test cases
reusing the existing setup (alice, bob, and populated invoices) and assert
lengths/contents accordingly.


assertEq(sent.length, 1);
assertEq(received.length, 1);
Expand Down
3 changes: 2 additions & 1 deletion frontend/src/page/BatchPayment.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,8 @@ function BatchPayment() {

const contract = new Contract(contractAddress, ChainvoiceABI, signer);

const res = await contract.getReceivedInvoices(address);
// Fetching the first 100 invoices (Offset: 0, Limit: 100)
const res = await contract.getReceivedInvoices(address, 0, 100);
Comment on lines +362 to +363
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 25, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Hardcoded pagination limits users to first 100 invoices.

With offset=0, limit=100, users with more than 100 invoices cannot access older records. This is a UX limitation noted in PR review comments.

Consider implementing:

  1. A "Load More" button that increments offset
  2. Iterative page fetching to retrieve all invoices
  3. Using the component's page and rowsPerPage state to drive contract-level pagination
💡 Example: Load more implementation 
+ const [contractOffset, setContractOffset] = useState(0);
+ const CONTRACT_PAGE_SIZE = 100;
+ const [hasMore, setHasMore] = useState(true);

- const res = await contract.getReceivedInvoices(address, 0, 100);
+ const res = await contract.getReceivedInvoices(address, contractOffset, CONTRACT_PAGE_SIZE);
+ if (res.length < CONTRACT_PAGE_SIZE) {
+   setHasMore(false);
+ }

// Add a "Load More" button in the UI when hasMore is true
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@frontend/src/page/BatchPayment.jsx` around lines 362 - 363, The contract call
in BatchPayment.jsx currently uses hardcoded pagination
(getReceivedInvoices(address, 0, 100)); change it to use the component's page
and rowsPerPage state for pagination (e.g., compute offset = page * rowsPerPage
and pass rowsPerPage as limit), and add either a "Load More" handler that
increments page or implement iterative fetching to accumulate pages until no
more results; update any fetchInvoice or loadInvoices helper used by the
component to accept offset/limit and ensure UI controls (Load More button or
pagination controls) update page so additional invoices are requested from
getReceivedInvoices rather than being capped at 100.


if (!res || !Array.isArray(res) || res.length === 0) {
setReceivedInvoices([]);
Expand Down
3 changes: 2 additions & 1 deletion frontend/src/page/ReceivedInvoice.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -686,7 +686,8 @@ function ReceivedInvoice() {

const contract = new Contract(contractAddress, ChainvoiceABI, signer);

const res = await contract.getReceivedInvoices(address);
// Fetching the first 100 invoices (Offset: 0, Limit: 100)
const res = await contract.getReceivedInvoices(address, 0, 100);
Comment on lines +689 to +690
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Mar 25, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Same hardcoded pagination limitation as BatchPayment.jsx.

Users with more than 100 invoices won't see older records. Apply the same fix pattern as suggested for BatchPayment.jsx to enable loading additional pages.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@frontend/src/page/ReceivedInvoice.jsx` around lines 689 - 690, The current
call to contract.getReceivedInvoices(address, 0, 100) hardcodes a 100-item page
and hides older invoices; replace this with the same pagination pattern used in
BatchPayment.jsx by introducing a PAGE_SIZE constant and either (a) perform
sequential fetches in a loop (offset = 0; while true: fetch
getReceivedInvoices(address, offset, PAGE_SIZE), append results to the invoices
state, break when returned.length < PAGE_SIZE) or (b) implement a "Load more"
handler that fetches the next page using offset += PAGE_SIZE and appends to
state; ensure you dedupe/merge results into the component state (the invoices
array) and remove the hardcoded 100 usage so older records can be loaded.


if (!res || !Array.isArray(res) || res.length === 0) {
setReceivedInvoice([]);
Expand Down
Loading