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

[H-03] The contract `TokenDivider` can have eth locked

joaosantosjorge

Description:
The TokenDivider contract can unintentionally lock ETH when users send more than the required amount to fulfill a buy order. The excess ETH remains in the contract and cannot be withdrawn, as no mechanism exists to handle or refund the surplus.

Impact:

High. This behavior can lead to user funds being irretrievably locked, which is not the expected functionality and can harm user trust and experience.

Proof of Concept:

Consider a scenario where a user sends 2 ETH to fulfill an order requiring only 1 ETH. The excess 1 ETH remains trapped in the contract.

function testMyBuyOrder() public {
vm.startPrank(USER);
erc721Mock.approve(address(tokenDivider), TOKEN_ID);
tokenDivider.divideNft(address(erc721Mock), TOKEN_ID, AMOUNT);
​
ERC20Mock erc20Mock = ERC20Mock(
tokenDivider.getErc20InfoFromNft(address(erc721Mock)).erc20Address
);
erc20Mock.approve(address(tokenDivider), AMOUNT);
tokenDivider.sellErc20(address(erc721Mock), 1e18, AMOUNT);
vm.stopPrank();
​
vm.startPrank(USER2);
tokenDivider.buyOrder{value: (2e18)}(0, address(USER));
vm.stopPrank();
​
//check erc20 balance decreases in USER
assertEq(tokenDivider.getBalanceOf(USER, address(erc20Mock)), 0);
//check erc20 balance is increased in USER2
assertEq(tokenDivider.getBalanceOf(USER2, address(erc20Mock)), AMOUNT);
​
//check contract balance is zero
assertEq(address(tokenDivider).balance, 0);
}

Recommended Mitigation:

You have two options:

  1. send back the surplus of eth

uint256 requiredAmount = order.price + sellerFee;
​
uint256 excessAmount = msg.value - requiredAmount;
​
if (excessAmount > 0) {
(bool refundSuccess, ) = payable(msg.sender).call{value: excessAmount}("");
require(refundSuccess, "Refund failed");
}

  1. or require the user to send the exact amount of eth when fullfiling the buy order.

uint256 requiredAmount = order.price + fee;
require(msg.value == requiredAmount, "Exact ETH required");
Updates

Lead Judging Commences

fishy Lead Judge 5 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.