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

ETH Lock Due to Overpayment in buyOrder Function

0x180db

Summary

The buyOrder function does not refund excess ETH sent by buyers, leading to permanently locked funds in the contract.

Vulnerability Details

In the buyOrder function, if a buyer sends more ETH than required (msg.value > order.price + sellerFee), the excess amount remains trapped in the contract. The function only transfers the exact required amount to the seller and owner, with no mechanism to return the surplus to the buyer.

function buyOrder(uint256 orderIndex, address seller) external payable {
// ...
if(msg.value < order.price + sellerFee) {
revert TokenDivider__InsuficientEtherForFees();
}
// Only transfers exact amounts
(bool success, ) = payable(order.seller).call{value: (order.price - sellerFee)}("");
(bool taxSuccess, ) = payable(owner()).call{value: fee}("");
// Excess ETH remains locked
}

Proof-Of-Concept:

function testPocOverpayment() public nftDivided {
ERC20Mock erc20Mock = ERC20Mock(tokenDivider.getErc20InfoFromNft(address(erc721Mock)).erc20Address);
​
uint256 ownerBalanceBefore = address(tokenDivider.owner()).balance;
uint256 contractBalanceBefore = address(tokenDivider).balance;
uint256 sellerBalanceBefore = address(USER).balance;
uint256 buyerBalanceBefore = address(USER2).balance;
​
assertEq(contractBalanceBefore, 0);
vm.startPrank(USER);
​
erc20Mock.approve(address(tokenDivider), AMOUNT);
tokenDivider.sellErc20(address(erc721Mock), PRICE, AMOUNT);
​
uint256 fees = PRICE / 100;
vm.stopPrank();
​
vm.prank(USER2);
uint256 value = 5 ether;
tokenDivider.buyOrder{value: value}(0, USER);
uint256 contractBalanceAfter = address(tokenDivider).balance;
assertEq(contractBalanceBefore, contractBalanceAfter);
}

Impact

  • Users who accidentally send more ETH than required will lose their excess funds

  • ETH can accumulate in the contract with no way to withdraw it

  • Poor user experience and potential financial losses for buyers

Tools Used

  • Manual code review

  • Performing formal verification with Quint

Recommendations

  1. Add a refund mechanism for excess ETH:

function buyOrder(uint256 orderIndex, address seller) external payable {
// ... existing code ...
uint256 excess = msg.value - (order.price + sellerFee);
if(excess > 0) {
(bool refundSuccess,) = payable(msg.sender).call{value: excess}("");
require(refundSuccess, "Refund failed");
}
}

  1. Alternatively, revert if sent amount exceeds required amount:

if(msg.value != order.price + sellerFee) {
revert TokenDivider__IncorrectEtherAmount();
}
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.