[M-4] Precision Loss Due to Truncation in getWethAmountOutAmm Function

Summary

A precision loss issue has been identified in the getWethAmountOutAmm function of the GoatLibrary.sol contract, crucial for calculating the WETH output in AMM token swaps. This flaw particularly affects transactions involving small amounts of tokens, where the calculated WETH output may round down to zero due to Solidity's integer division mechanics, effectively rendering small but legitimate transactions futile.

Vulnerability Detail

Within the getWethAmountOutAmm function, an early scaling of the input token amount combined with Solidity’s integer arithmetic leads to a rounding down phenomenon. This is exacerbated in cases involving minimal input amounts, such as 1 wei, where the expected output becomes significantly less than 1 wei and is thus truncated to 0. This unintended consequence of the function's arithmetic logic undermines the contract's ability to handle a wide range of transaction sizes reliably.

Impact

The direct impact of this vulnerability includes the potential loss of functionality for users attempting to conduct swaps with very small amounts of tokens, possibly eroding user trust in the platform's reliability. While the financial impact per transaction may be small, the cumulative effect on user experience and the protocol's perceived precision and flexibility could be substantial.

Code Snippet

The precision loss issue occurs in the following snippet from the GoatLibrary.sol contract:

https://github.com/sherlock-audit/2024-03-goat-trading/blob/main/goat-trading/contracts/library/GoatLibrary.sol#L123-L141

// Original function showcasing the precision loss vulnerability
function getWethAmountOutAmm(uint256 amountTokenIn, uint256 reserveEth, uint256 reserveToken)
    internal
    pure
    returns (uint256 amountWethOut) {
    ...
}

POC

Add this function to GoatLibrary.t.sol

    function testPrecisionLoss() public {
        // Inputs that would lead to a fractional output in precise math but may result in truncation in Solidity
        uint256 amountTokenIn = 1; // 1 wei of token
        uint256 reserveEth = 100 ether; // Large ETH reserve
        uint256 reserveToken = 100 ether; // token and ETH reserve are 1:1 ratio

        // Calculate output
        uint256 amountWethOut = GoatLibrary.getWethAmountOutAmm(amountTokenIn, reserveEth, reserveToken);

        // Log the result for inspection
        console.log("Amount WETH Out:", amountWethOut);

        // Assertions or other checks to verify truncation behavior
        assertEq(amountWethOut, 0);
    }

Output:

[PASS] testPrecisionLoss() (gas: 4262)
Logs:
  Amount WETH Out: 0

Tool used

Manual Review

Recommendation

To mitigate this issue and enhance the handling of transactions across all sizes, the following adjustments are recommended:

function getWethAmountOutAmm(uint256 amountTokenIn, uint256 reserveEth, uint256 reserveToken)
    internal
    pure
    returns (uint256 amountWethOut) {
+   // Early return for cases where output would be negligible to prevent gas wastage
+   if (amountTokenIn < minEffectiveInputAmount) return 0;
-   amountTokenIn = amountTokenIn * 10000; // Existing scaling leading to precision loss
+   uint256 numerator = amountTokenIn * reserveEth;
+   uint256 denominator = reserveToken + amountTokenIn;
-   uint256 actualAmountWethOut = numerator / denominator; // Where truncation occurs
+   uint256 actualAmountWethOut = (numerator * 10000) / (denominator * 9901); // Adjusted to minimize precision loss
    ...
}

These recommendations aim to refine the swap function's handling of edge cases, ensuring that the platform can efficiently process transactions of all sizes without inadvertently nullifying those at the lower end of the scale. This approach not only preserves transaction integrity but also bolsters the protocol's inclusivity and user trust.

sherlock-audit / 2024-03-goat-trading-judging

MaanVader - [M-4] Precision Loss Due to Truncation in getWethAmountOutAmm Function #57