Pieces Protocol
First Flight #32
Beginner FriendlyFoundrySolidityNFT
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Invalid

Array Out of Bounds in `buyOrder` Leading to Transaction Failures

damboy

Summary

https://github.com/Cyfrin/2025-01-pieces-protocol/blob/4ef5e96fced27334f2a62e388a8a377f97a7f8cb/src/TokenDivider.sol#L261

In the TokenDivider::buyOrder(uint256 orderIndex, address seller) function, if a user inputs an invalid orderIndex, it results in an array out-of-bounds error. This can lead to unexpected behavior, including transaction reverts, which disrupts the user experience.

Vulnerability Details

The buyOrder function does not perform checks to validate the orderIndex parameter. If a user accidentally inputs an invalid index (e.g., greater than or equal to the number of available orders), it will access an out-of-bounds index in the internal array, causing a revert. This can lead to Denial of Service (DoS) conditions where users cannot complete their transactions.

POC:

  • Setup: add the following code on TokenDividerTest.t.sol and run forge test --mt testBuyOrderInvalidIndex -vvvv

function testBuyOrderInvalidIndex() public nftDivided {
​
ERC20Mock erc20Mock = ERC20Mock(tokenDivider.getErc20InfoFromNft(address(erc721Mock)).erc20Address);
​
vm.startPrank(USER);
​
erc20Mock.approve(address(tokenDivider), AMOUNT);
​
tokenDivider.sellErc20(address(erc721Mock), AMOUNT, 1e18);
​
vm.stopPrank();
​
vm.prank(USER2);
​
vm.expectRevert(); //revert
​
tokenDivider.buyOrder{value: (1e18)}(1, USER); // invalid orderIndex input
​
​
vm.stopPrank();
​
}

OutPut:

When running the test, you will see output similar to the following:

[⠢] Compiling...
[⠒] Compiling 1 files with Solc 0.8.27
[⠑] Solc 0.8.27 finished in 2.28s
Compiler run successful!
​
Ran 1 test for test/unit/TokenDividerTest.t.sol:TokenDiverTest
[PASS] testBuyOrderInvalidIndex() (gas: 894641)
Traces:
[939241] TokenDiverTest::testBuyOrderInvalidIndex()
├─ [0] VM::startPrank(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D])
│ └─ ← [Return]
├─ [27119] ERC721Mock::approve(TokenDivider: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 0)
│ ├─ emit Approval(owner: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], approved: TokenDivider: [0x90193C961A926261B756D1E5bb255e67ff9498A1], tokenId: 0)
│ └─ ← [Stop]
├─ [717503] TokenDivider::divideNft(ERC721Mock: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], 0, 2000000000000000000 [2e18])
│ ├─ [620] ERC721Mock::ownerOf(0) [staticcall]
│ │ └─ ← [Return] user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]
│ ├─ [620] ERC721Mock::ownerOf(0) [staticcall]
│ │ └─ ← [Return] user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D]
│ ├─ [3285] ERC721Mock::name() [staticcall]
│ │ └─ ← [Return] "MockToken"
│ ├─ [3328] ERC721Mock::symbol() [staticcall]
│ │ └─ ← [Return] "MTK"
│ ├─ [466357] → new ERC20ToGenerateNftFraccion@0xd04404bcf6d969FC0Ec22021b4736510CAcec492
│ │ └─ ← [Return] 2099 bytes of code
│ ├─ [46900] ERC20ToGenerateNftFraccion::mint(TokenDivider: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 2000000000000000000 [2e18])
│ │ ├─ emit Transfer(from: 0x0000000000000000000000000000000000000000, to: TokenDivider: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 2000000000000000000 [2e18])
│ │ └─ ← [Stop]
│ ├─ [38532] ERC721Mock::safeTransferFrom(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], TokenDivider: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 0, 0x)
│ │ ├─ emit Transfer(from: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], to: TokenDivider: [0x90193C961A926261B756D1E5bb255e67ff9498A1], tokenId: 0)
│ │ ├─ [670] TokenDivider::onERC721Received(TokenDivider: [0x90193C961A926261B756D1E5bb255e67ff9498A1], user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], 0, 0x)
│ │ │ └─ ← [Return] 0x150b7a02
│ │ └─ ← [Stop]
│ ├─ [620] ERC721Mock::ownerOf(0) [staticcall]
│ │ └─ ← [Return] TokenDivider: [0x90193C961A926261B756D1E5bb255e67ff9498A1]
│ ├─ emit NftDivided(nftAddress: ERC721Mock: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], amountErc20Minted: 2000000000000000000 [2e18], erc20Minted: ERC20ToGenerateNftFraccion: [0xd04404bcf6d969FC0Ec22021b4736510CAcec492])
│ ├─ [25210] ERC20ToGenerateNftFraccion::transfer(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], 2000000000000000000 [2e18])
│ │ ├─ emit Transfer(from: TokenDivider: [0x90193C961A926261B756D1E5bb255e67ff9498A1], to: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], value: 2000000000000000000 [2e18])
│ │ └─ ← [Return] true
│ └─ ← [Stop]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [857] TokenDivider::getErc20InfoFromNft(ERC721Mock: [0x2e234DAe75C793f67A35089C9d99245E1C58470b]) [staticcall]
│ └─ ← [Return] ERC20Info({ erc20Address: 0xd04404bcf6d969FC0Ec22021b4736510CAcec492, tokenId: 0 })
├─ [0] VM::startPrank(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D])
│ └─ ← [Return]
├─ [24739] ERC20ToGenerateNftFraccion::approve(TokenDivider: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 2000000000000000000 [2e18])
│ ├─ emit Approval(owner: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], spender: TokenDivider: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 2000000000000000000 [2e18])
│ └─ ← [Return] true
├─ [138976] TokenDivider::sellErc20(ERC721Mock: [0x2e234DAe75C793f67A35089C9d99245E1C58470b], 2000000000000000000 [2e18], 1000000000000000000 [1e18])
│ ├─ emit OrderPublished(amount: 1000000000000000000 [1e18], seller: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], nftPegged: ERC721Mock: [0x2e234DAe75C793f67A35089C9d99245E1C58470b])
│ ├─ [24069] ERC20ToGenerateNftFraccion::transferFrom(user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], TokenDivider: [0x90193C961A926261B756D1E5bb255e67ff9498A1], 1000000000000000000 [1e18])
│ │ ├─ emit Transfer(from: user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D], to: TokenDivider: [0x90193C961A926261B756D1E5bb255e67ff9498A1], value: 1000000000000000000 [1e18])
│ │ └─ ← [Return] true
│ └─ ← [Stop]
├─ [0] VM::stopPrank()
│ └─ ← [Return]
├─ [0] VM::prank(user2: [0x537C8f3d3E18dF5517a58B3fB9D9143697996802])
│ └─ ← [Return]
├─ [0] VM::expectRevert(custom error f4844814:)
│ └─ ← [Return]
├─ [637] TokenDivider::buyOrder{value: 1000000000000000000}(1, user: [0x6CA6d1e2D5347Bfab1d91e883F1915560e09129D])
│ └─ ← [Revert] panic: array out-of-bounds access (0x32) //<== The ERROR Message
├─ [0] VM::stopPrank()
│ └─ ← [Return]
└─ ← [Stop]
​
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 6.48ms (1.62ms CPU time)
​
Ran 1 test suite in 2.08s (6.48ms CPU time): 1 tests passed, 0 failed, 0 skipped (1 total tests)

Impact

  • Transaction Failures: Users may experience unexpected transaction failures when attempting to buy orders with invalid index.

  • Denial of Service (DoS): Attackers could exploit this vulnerability by repeatedly calling the function with invalid indices, preventing legitimate users from completing transactions.

Tools Used

Manual Review and Foundry

Recommendations

  • Implement Index Validation: Add checks in the buyOrder function to validate that orderIndex is within bounds before accessing the array. you can add something like the below code :

​
// Check if orderIndex is valid
​
if (orderIndex >= s_userToSellOrders[seller].length)
{
revert TokenDivider__InvalidOrderIndex(); // Define this error in your contract
}
​
Updates

Lead Judging Commences

fishy Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.