Submission Details
Severity:
The `TokenDivider::buyOrder` doesn't have a mechanism to refund users if they send more ETH than `order.price + sellerFee`
Description
The function doesn't implement a way to refund users if they send more ETH than needed to buy the order.
Impact
Users will get their extra ETH sent locked in the contract.
Proof of Concepts
Add the following test to TokenDividerTest.t.sol file:
function testBuyOrderDoesntRefundExcessEthSent() public nftDivided {
ERC20Mock erc20Mock = ERC20Mock(tokenDivider.getErc20InfoFromNft(address(erc721Mock)).erc20Address);
vm.startPrank(USER);
erc20Mock.approve(address(tokenDivider), AMOUNT);
tokenDivider.sellErc20(address(erc721Mock), AMOUNT, 1e18);
uint256 fees = AMOUNT / 100;
vm.stopPrank();
uint256 user2BalanceBefore = address(USER2).balance;
vm.prank(USER2);
tokenDivider.buyOrder{value: (3e18)}(0, USER);
uint256 user2BalanceAfter = address(USER2).balance;
// The assertion will fail
assertEq(user2BalanceBefore - 1e18, user2BalanceAfter);
}
Tools Used
Foundry, Manual analysis
Recommended mitigation
Add the following change to the contract:
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);
// Transfer The Ether
(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();
}
+ if(msg.value > order.price + sellerFee){
+ (bool refundSuccess, ) = payable(msg.sender).call{value: msg.value - (order.price + sellerFee)}("");
+ if(!refundSuccess) {
+ revert TokenDivider__TransferFailed();
+ }
+ }
IERC20(order.erc20Address).transfer(msg.sender, order.amount);
}
Updates
Lead Judging Commences
fishy 8 months ago
Submission Judgement Published Assigned finding tags:
Token misshandling
The extra eth sent by the user in the buy order will be locked in the contract forever
Rewards breakdown
nSLOC
181This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.