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

The Excess ETH in `buyOrder` Function Is Not Returned to the User

udogodwin2k22

Summary

The buyOrder function lacks a mechanism to refund excess ETH when buyers send more than the required amount for a purchase. Any excess ETH sent becomes trapped in the contract.

Vulnerability Details

The buyOrder function only validates that the sent ETH meets minimum requirements but doesn't handle excess amounts:

function buyOrder(uint256 orderIndex, address seller) external payable {
if(seller == address(0)) {
revert TokenDivider__InvalidSeller();
}
SellOrder memory order = s_userToSellOrders[seller][orderIndex];
​
if(msg.value < order.price) {
revert TokenDivider__IncorrectEtherAmount();
}
​
uint256 fee = order.price / 100;
uint256 sellerFee = fee / 2;
​
if(msg.value < order.price + sellerFee) {
revert TokenDivider__InsuficientEtherForFees();
}
​
balances[msg.sender][order.erc20Address] += order.amount;
s_userToSellOrders[seller][orderIndex] = s_userToSellOrders[seller][s_userToSellOrders[seller].length - 1];
s_userToSellOrders[seller].pop();
emit OrderSelled(msg.sender, order.price);
(bool success, ) = payable(order.seller).call{value: (order.price - sellerFee)}("");
if(!success) {
revert TokenDivider__TransferFailed();
}
​
(bool taxSuccess, ) = payable(owner()).call{value: fee}("");
if(!taxSuccess) {
revert TokenDivider__TransferFailed();
}
​
IERC20(order.erc20Address).transfer(msg.sender, order.amount);
}

The function validates:

  1. That sent ETH covers the order price (msg.value < order.price)

  2. That sent ETH covers price plus fees (msg.value < order.price + sellerFee)

However, there is no mechanism to:

  1. Check for excess ETH sent above these requirements

  2. Return any excess back to the sender

  3. Prevent users from accidentally overpaying

Impact

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

  • These excess funds become permanently trapped in the contract

  • No mechanism exists to recover these funds

Tools used

Manual review

Recommendation

Implement a refund mechanism for excess ETH. After buying and deducting the required ETH, return any excess ETH to the user.

function buyOrder(uint256 orderIndex, address seller) external payable {
// [...existing checks...]
uint256 totalRequired = order.price + sellerFee;
if(msg.value < totalRequired) {
revert TokenDivider__InsuficientEtherForFees();
}
​
// [...existing logic...]
​
// Refund excess ETH
uint256 excess = msg.value - totalRequired;
if(excess > 0) {
(bool refundSuccess, ) = payable(msg.sender).call{value: excess}("");
if(!refundSuccess) {
revert TokenDivider__TransferFailed();
}
}
​
IERC20(order.erc20Address).transfer(msg.sender, order.amount);
}

This ensures users receive back any ETH sent above the required amount, preventing accidental loss of funds.

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.