Summary
The TokenDivider contract contains a precision loss vulnerability in its fee calculation logic within the buyOrder function. The issue arises from the use of integer division, which truncates any remainder when calculating the fee (order.price / 100) and the seller fee (fee / 2). This leads to an actual fee collected that is slightly less than the theoretical full fee, causing a financial discrepancy. While the issue may appear minor at first glance, its cumulative impact could be significant.
Vulnerability Details
The vulnerability lies in the calculation of the fee in the buyOrder function, specifically in the following lines:
uint256 fee = order.price / 100;
uint256 sellerFee = fee / 2;
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();
}
The root cause lies in Solidity's behavior when performing division with integers: it truncates the decimal portion of the result, leading to a loss of precision. Specifically:
The calculation of fee involves dividing order.price by 100 to determine 1% of the order value. The subsequent calculation of sellerFee divides the previously truncated fee by 2, compounding the precision loss.
For example, if the order price is $9999: The expected fee should be 9999 ÷ 100 = 99.9. However, due to truncation, the calculated fee is 99, resulting in a loss of $1.\
This precision loss affects downstream calculations, including the seller’s payout and the platform’s earnings.
Proof of concept
pragma solidity ^0.8.18;
import {Test, console2} from "forge-std/Test.sol";
import {TokenDivider} from "../src/TokenDivider.sol";
import {MockERC721} from "./MockERC721.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
contract TokenDividerTest is Test {
TokenDivider public tokenDivider;
MockERC721 public mockNFT;
address public owner;
address public buyer;
address public seller;
function setUp() public {
owner = makeAddr("owner");
buyer = makeAddr("buyer");
seller = makeAddr("seller");
vm.startPrank(owner);
tokenDivider = new TokenDivider();
mockNFT = new MockERC721();
vm.stopPrank();
}
function testPrecisionLossInFees() public {
vm.startPrank(owner);
mockNFT.mint(seller, 1);
vm.stopPrank();
vm.startPrank(seller);
mockNFT.approve(address(tokenDivider), 1);
tokenDivider.divideNft(address(mockNFT), 1, 100);
address erc20Address = tokenDivider.getErc20InfoFromNft(address(mockNFT)).erc20Address;
IERC20(erc20Address).approve(address(tokenDivider), type(uint256).max);
uint256 orderPrice = 9999;
tokenDivider.sellErc20(address(mockNFT), orderPrice, 50);
vm.stopPrank();
uint256 initialSellerBalance = seller.balance;
uint256 initialOwnerBalance = owner.balance;
uint256 theoreticalFee = (orderPrice * 1) / 100;
uint256 theoreticalFullFee = 100;
vm.startPrank(buyer);
vm.deal(buyer, orderPrice + 100);
tokenDivider.buyOrder{value: orderPrice + 100}(0, seller);
vm.stopPrank();
uint256 actualSellerReceived = seller.balance - initialSellerBalance;
uint256 actualOwnerReceived = owner.balance - initialOwnerBalance;
console2.log("Order Price:", orderPrice);
console2.log("Theoretical Full Fee (100):", theoreticalFullFee);
console2.log("Actual Fee Collected:", actualOwnerReceived);
console2.log("Fee Difference (precision loss):", theoreticalFullFee - actualOwnerReceived);
console2.log("Seller Received:", actualSellerReceived);
assertTrue(actualOwnerReceived < theoreticalFullFee, "Fee should be lost due to precision");
assertTrue(actualOwnerReceived == 99, "Fee should be truncated to 99");
}
function testEdgeCasePrecisionLoss() public {
vm.startPrank(owner);
mockNFT.mint(seller, 1);
vm.stopPrank();
vm.startPrank(seller);
mockNFT.approve(address(tokenDivider), 1);
tokenDivider.divideNft(address(mockNFT), 1, 100);
address erc20Address = tokenDivider.getErc20InfoFromNft(address(mockNFT)).erc20Address;
IERC20(erc20Address).approve(address(tokenDivider), type(uint256).max);
uint256 orderPrice = 199;
tokenDivider.sellErc20(address(mockNFT), orderPrice, 50);
vm.stopPrank();
uint256 initialOwnerBalance = owner.balance;
vm.startPrank(buyer);
vm.deal(buyer, orderPrice + 2);
tokenDivider.buyOrder{value: orderPrice + 2}(0, seller);
vm.stopPrank();
uint256 actualOwnerReceived = owner.balance - initialOwnerBalance;
console2.log("\nEdge Case Test:");
console2.log("Order Price:", orderPrice);
console2.log("Expected Fee (1.99):", "1.99");
console2.log("Actual Fee Collected:", actualOwnerReceived);
assertTrue(actualOwnerReceived == 1, "Fee should be truncated to 1 instead of 1.99");
}
}
The division of order.price by 100 and subsequently dividing fee by 2 introduces precision loss due to integer arithmetic. For instance, if the order price is 9999, the theoretical fee should be 100 (9999 ÷ 100). However, the actual fee collected is 99, as demonstrated in the Proof of Concept (PoC) test logs:
Order Price: 9999
Theoretical Full Fee (100): 100
Actual Fee Collected: 99
Fee Difference (precision loss): 1
Seller Received: 9950
The loss of 1 unit may appear trivial for a single transaction, but across thousands of transactions, these discrepancies can aggregate into a significant financial impact on both the protocol and its users.
Impact
-
Sellers and the platform may lose out on small amounts in each transaction. The collected fees are consistently lower than expected, resulting in revenue loss for the platform. This issue becomes more pronounced in high-frequency transaction scenarios, where even a small precision loss per transaction can add up to significant financial discrepancies over time.
-
Affected users might lose trust in the platform if they perceive fee calculations to be inaccurate.
Recommendation
To address the precision loss, it is advisable to use fixed-point arithmetic instead of integer division. This can be achieved by scaling the input values to include decimals.
uint256 scaledPrice = order.price * 100;
uint256 fee = scaledPrice / 10000;
uint256 sellerFee = fee / 2;
Here now, if you multiply 9999*100 you get, 999900, when you divide by 10000, you get 99.99 instead of 99
Previously, the fee would have been truncated to 99, causing a cumulative loss over multiple transactions.
Alternatively, consider using libraries such as PRBMath or OpenZeppelin’s utilities for handling fixed-point numbers accurately.
