Summary
The TokenDivider::buyOrder function does not refund excess ETH, making some ether balance stuck in the contract.
Vulnerability Details
In the TokenDivider::buyOrder, the condition is checking if msg.value < order.price + sellerFee, therefore any excess Ether sent gets stuck in the contract
function buyOrder(uint256 orderIndex, address seller) external payable {
...
@> if (msg.value < order.price + sellerFee) {
revert TokenDivider__InsuficientEtherForFees();
}
...
(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();
}
...
}
Proof of Concept
Consider adding this test to the TokenDividerTest.t.soltest file. After successfully buying the ERC20 token, the ETH balance of the tokenDivideris greater than the ETH balance before the purchase.
function testBuyErc20LostFund() 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 tokenDividerEthBalanceBefore = address(tokenDivider).balance;
console.log("Contract ETH Balance Before", tokenDividerEthBalanceBefore);
uint256 amountToBuyWith = AMOUNT + fees;
vm.prank(USER2);
tokenDivider.buyOrder{value: amountToBuyWith}(0, USER);
uint256 tokenDividerEthBalanceAfter = address(tokenDivider).balance;
console.log("Contract ETH Balance After", tokenDividerEthBalanceAfter);
@> assertGt(tokenDividerEthBalanceAfter , tokenDividerEthBalanceBefore);
}
Impact
All excess Ether collected gets stuck in the contract and there is no withdraw function to get them out.
Tools Used
Manual review
Recommendations
Consider making sure the ether sent is equal to the amount required. Or refund excess Ether after the actual implementation is completed.
function buyOrder(uint256 orderIndex, address seller) external payable {
...
// Transfer The Ether
- if (msg.value < order.price + sellerFee) {
+ if (msg.value != order.price + sellerFee) {
revert TokenDivider__InsuficientEtherForFees();
}
if (!taxSuccess) {
revert TokenDivider__TransferFailed();
}
...
}
