sherlock-audit / 2024-06-velocimeter-judging

11 stars 7 forks source link

KupiaSec - Anyone can break the constant product by minting into `Pair` directly #631

Closed sherlock-admin2 closed 3 months ago

sherlock-admin2 commented 3 months ago

KupiaSec

High

Anyone can break the constant product by minting into Pair directly

Summary

There is a following comment for mint, burn and swap function in the Pair contract: "this low-level function should be called by addLiquidity functions in Router.sol, which performs important safety checks standard uniswap v2 implementation." The addLiquidity function in Router.sol perform safety check to maintain the product of the amounts of pair reserves. But anyone can mint into pair directly without safety check. Thus, attackers can break the product of the amounts of pair reserves.

Vulnerability Detail

The Router.addLiquidity function checks the input amount of assets.

https://github.com/sherlock-audit/2024-06-velocimeter/blob/63818925987a5115a80eff4bd12578146a844cfd/v4-contracts/contracts/Router.sol#L186-L195

        uint amountBOptimal = quoteLiquidity(amountADesired, reserveA, reserveB);
        if (amountBOptimal <= amountBDesired) {
        require(amountBOptimal >= amountBMin, 'Router: INSUFFICIENT_B_AMOUNT');
        (amountA, amountB) = (amountADesired, amountBOptimal);
        } else {
        uint amountAOptimal = quoteLiquidity(amountBDesired, reserveB, reserveA);
        assert(amountAOptimal <= amountADesired);
        require(amountAOptimal >= amountAMin, 'Router: INSUFFICIENT_A_AMOUNT');
        (amountA, amountB) = (amountAOptimal, amountBDesired);
        }

It is for maintaining the product of the amounts of pair reserves.

https://github.com/sherlock-audit/2024-06-velocimeter/blob/63818925987a5115a80eff4bd12578146a844cfd/v4-contracts/contracts/Router.sol#L186-L195

        function quoteLiquidity(uint amountA, uint reserveA, uint reserveB) internal pure returns (uint amountB) {
                require(amountA > 0, 'Router: INSUFFICIENT_AMOUNT');
                require(reserveA > 0 && reserveB > 0, 'Router: INSUFFICIENT_LIQUIDITY');
                amountB = amountA * reserveB / reserveA;
        }

After this checking, the addLiquidity function calls IPair(pair).mint. But attacker can break the product of the amounts of pair reserves by transfering tokens to pair and calling IPair(pair).mint directly.

https://github.com/sherlock-audit/2024-06-velocimeter/blob/63818925987a5115a80eff4bd12578146a844cfd/v4-contracts/contracts/Pair.sol#L250

        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);
        }

Impact

Attacker can break the constant product of pair’s reserve balances.

Code Snippet

https://github.com/sherlock-audit/2024-06-velocimeter/blob/63818925987a5115a80eff4bd12578146a844cfd/v4-contracts/contracts/Router.sol#L186-L195

https://github.com/sherlock-audit/2024-06-velocimeter/blob/63818925987a5115a80eff4bd12578146a844cfd/v4-contracts/contracts/Router.sol#L186-L195

https://github.com/sherlock-audit/2024-06-velocimeter/blob/63818925987a5115a80eff4bd12578146a844cfd/v4-contracts/contracts/Pair.sol#L250

Tool used

Manual Review

Recommendation

It is recommended to check that the caller of the mint, swap and burn functions is Router contract.

nevillehuang commented 3 months ago

Invalid, no product is broken, the mint() function will force the reserves to be 1:1 with balance of contract, and the attacker will lose all donated funds