Description:
The seller is receiving less ETH than the price specified in their SellOrder. This occurs because the fee is being deducted from the seller’s payout instead of being added to the amount the buyer must pay. The fee should be an additional charge borne by the buyer rather than reducing the seller’s expected proceeds.
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);
}
Impact:
Medium: The seller does not receive the full ETH amount they set in their sell order, undermining trust in the contract. Sellers may perceive the platform as unfair or broken if their expectations are not met.
Proof of Concept:
Consider a sell order where the price is 1e18 (1 ETH), and the fee is 0.5% (5e15 or 0.005 ETH). The buyer sends 1e18 (1 ETH), but the seller only receives 0.995e18 (0.995 ETH) instead of the full 1e18.
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: (1005000000000000000)}(0, address(USER));
vm.stopPrank();
assertEq(tokenDivider.getBalanceOf(USER, address(erc20Mock)), 0);
assertEq(tokenDivider.getBalanceOf(USER2, address(erc20Mock)), AMOUNT);
assertEq(USER.balance, 1e18);
}
Recommended Mitigation:
Modify the logic so that the seller receives the full order.price, and the buyer pays an additional amount to cover the fee.
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)
+ value: order.price
}("");
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);
}
