hats-finance / Fenix-Finance-0x83dbe5aa378f3ce160ed084daf85f621289fb92f

0 stars 0 forks source link

First liquidity provider of a stable pair can DOS the pool #45

Open hats-bug-reporter[bot] opened 4 months ago

hats-bug-reporter[bot] commented 4 months ago

Github username: @0xRizwan Twitter username: 0xRizwann Submission hash (on-chain): 0x2368b900edbbfda253d23d27ecac06caf42e72fc453a0e8bdb4dcc57d31b3150 Severity: high

Description: Description\

The invariant k of a stable pool is calculated as follows which can be checked here,


    function _k(uint x, uint y) internal view returns (uint) {
        if (stable) {
            uint _x = (x * 1e18) / decimals0;
            uint _y = (y * 1e18) / decimals1;
            uint _a = (_x * _y) / 1e18;
            uint _b = ((_x * _x) / 1e18 + (_y * _y) / 1e18);
            return (_a * _b) / 1e18; // x3y+y3x >= k
        } else {
            return x * y; // xy >= k
        }
    }

The value of _a = (x * y ) / 1e18 = 0 due to rounding error when x*y < 1e18. The rounding error can lead to the invariant k of stable pools equals zero, and the trader can steal whatever is left in the pool.

The first liquidity provider can DOS the pair by: 1) mint a small amount of liquidity to the pool, 2) Steal whatever is left in the pool, 3) Repeat step 1, and step 2 until the overflow of the total supply.

To prevent the issue of rounding error, the reserve of a pool should never go too small. The mint function which was borrowed from uniswapV2 has a minimum liquidity check of sqrt(a * b) > MINIMUM_LIQUIDITY; This, however, isn't safe enough to protect the invariant formula of stable pools. mint() function can be checked here

    uint internal constant MINIMUM_LIQUIDITY = 10 ** 3;

    // 

    function mint(address to) external lock returns (uint liquidity) {
        (uint _reserve0, uint _reserve1) = (reserve0, reserve1);
        uint _balance0 = IERC20(token0).balanceOf(address(this));
        uint _balance1 = IERC20(token1).balanceOf(address(this));
        uint _amount0 = _balance0 - _reserve0;
        uint _amount1 = _balance1 - _reserve1;

        uint _totalSupply = totalSupply; // gas savings, must be defined here since totalSupply can update in _mintFee
        if (_totalSupply == 0) {
            liquidity = Math.sqrt(_amount0 * _amount1) - MINIMUM_LIQUIDITY;
            _mint(address(0), MINIMUM_LIQUIDITY); // permanently lock the first MINIMUM_LIQUIDITY tokens
        } else {
            liquidity = Math.min((_amount0 * _totalSupply) / _reserve0, (_amount1 * _totalSupply) / _reserve1);
        }
        require(liquidity > 0, "ILM"); // Pair: INSUFFICIENT_LIQUIDITY_MINTED
        _mint(to, liquidity);

        _update(_balance0, _balance1, _reserve0, _reserve1);
        emit Mint(msg.sender, _amount0, _amount1);
    }

Recommendations\

Recommend to add two restrictions on the first lp of stable pools: 1) only allows equal amounts of liquidity. 2) invariant _k should be larger than the MINIMUM_K

Please check this commit by velodrome finance to get the correct mitigation in contract.

References:

This issue is directly reference from this issue found in Velodrome audit at spearbit. The issue explanation from spearbit is excellent which led me to keep it as it. This reference also consist of POC and extensive discussions on this issue. Thankful to spearbit team for getting and learning from this issue.

BohdanHrytsak commented 4 months ago

Thank you for the submission.

Duplicate #28