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

ETH balance stuck in contract forever

joesepherus

Vulnerability Details

The TokenDivider::buyOrder function allows users to send any amount of ETH, then it substracts from this amount the order.price and calculated fees. What is left over from the original sent amount of ETH will be forever stuck in the contract. There is no withdraw or send ETH function back to the user.

Impact

All the extra ETHER sent to the TokenDivider::buyOrder will the stuck in the contract forever.

Tools Used

Manual review

Proof of Concept

  1. USER mints nft and divides it, approves the erc20 tokens to the tokenDivider contract and sets up a sell order using TokenDivider::sellErc20

  2. USER2 then calls TokenDivider::buyOrder with more than enough ETH balance

  3. Function TokenDivider::buyOrder calculates how much of the sent ETH balance from USER2 should be sent to the seller and how much to the owner of the contact. The rest of the sent ETH is stuck in the TokenDivider contract.

PoC Code

Add following test:

function testBuyErc20BalanceStuck() public nftDivided {
ERC20Mock erc20Mock = ERC20Mock(
tokenDivider.getErc20InfoFromNft(address(erc721Mock)).erc20Address
);
​
uint256 contractBalanceBefore = address(tokenDivider).balance;
​
vm.startPrank(USER);
erc20Mock.approve(address(tokenDivider), AMOUNT);
tokenDivider.sellErc20(address(erc721Mock), AMOUNT, 1e18);
vm.stopPrank();
​
uint256 fees = AMOUNT / 100;
​
vm.prank(USER2);
tokenDivider.buyOrder{value: (3e18)}(0, USER);
​
uint256 contractBalanceAfter = address(tokenDivider).balance;
// the ether left from the order.price and fees is left in the contract stuck forever
assertNotEq(contractBalanceAfter, contractBalanceBefore);
}

Recommendation

To prevent this, we should send back the left over ETH amount to the buyer.

function buyOrder(uint256 orderIndex, address seller) external payable {
...
(bool success, ) = payable(order.seller).call{
value: (order.price - sellerFee)
}("");
​
if (!success) {
revert TokenDivider__TransferFailed();
}
​
+ (bool leftoverSuccess, ) = payable(msg.sender).call{
+ value:msg.value - fee - (order.price - sellerFee)
+ }("");
+ if (!leftoverSuccess) {
+ revert TokenDivider__TransferFailed();
+ }
...
}
Updates

Lead Judging Commences

fishy Lead Judge 7 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Token misshandling

The extra eth sent by the user in the buy order will be locked in the contract forever

Support

FAQs

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