Description:
In the TokenDivider::buyOrder
function, the fee calculation and distribution logic is inconsistent, causing the buyer to overpay. The fees are calculated in two steps: fee
and sellerFee
. However, during settlement, different variables are used for the seller and the contract owner:
For the seller: The variable fee
is used.
For the contract owner: The variable sellerFee
is used.
This inconsistency results in a mismatch between the expected and actual amounts, forcing the buyer to pay more than required.
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:
High. The buyer is overpaying.
Proof of Concept:
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();
uint256 ownerBalanceBefore = address(tokenDivider.owner()).balance;
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);
uint256 fee = 1e18 / 100;
uint256 sellerFee = fee / 2;
assertEq(
address(tokenDivider.owner()).balance - ownerBalanceBefore,
sellerFee
);
}
Recommended Mitigation:
Match the expected and actual 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);
// 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}("");
+ (bool taxSuccess, ) = payable(owner()).call{value: sellerFee}("");
if (!taxSuccess) {
revert TokenDivider__TransferFailed();
}
IERC20(order.erc20Address).transfer(msg.sender, order.amount);
}