code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-12-Stealth-Project/blob/fc8589d7d8c1d8488fd97ccc46e1ff11c8426ac2/maverick-v1/contracts/models/Factory.sol#L60 https://github.com/code-423n4/2022-12-Stealth-Project/blob/fc8589d7d8c1d8488fd97ccc46e1ff11c8426ac2/maverick-v1/contracts/models/Factory.sol#L82

Vulnerability details

Impact

Pool with any fee tier can be created

Proof of Concept

There is a section in this medium:

https://medium.com/maverick-protocol/maverick-amm-the-revolutionary-amm-that-enables-directional-lping-unlocking-greater-capital-34427f5ac22f

In Fee Structure:

Maverick AMM supports initializing pools with arbitrary fee rates, but it is expected that LPs will choose from one of the following “standard” fee rates:

0.01%
0.04%
0.06%
0.10%
0.25%
0.35%
0.50%
1.00%
2.00%
3.00%

In the implement, the code does not restrict the fee setting:

    function create(uint256 _fee, uint256 _tickSpacing, uint16 _lookback, int32 _activeTick, IERC20 _tokenA, IERC20 _tokenB) external override returns (IPool pool) {
        require(_tokenA < _tokenB, "Factory:TOKENS_MUST_BE_SORTED_BY_ADDRESS");
        require(_fee > 0 && _fee < 1e18, "Factory:FEE_OUT_OF_BOUNDS");
        require(_tickSpacing > 0, "Factory:TICK_SPACING_OUT_OF_BOUNDS");
        require(_lookback >= 3600 && _lookback <= uint16(type(int16).max), "Factory:LOOKBACK_OUT_OF_BOUNDS");

        require(pools[_fee][_tickSpacing][_lookback][_tokenA][_tokenB] == IPool(address(0)), "Factory:POOL_ALREADY_EXISTS");

        pool = Deployer.deploy(
            _fee,
            _tickSpacing,
            _activeTick,
            _lookback,
            protocolFeeRatio,
            _tokenA,
            _tokenB,
            Math.scale(IERC20Metadata(address(_tokenA)).decimals()),
            Math.scale(IERC20Metadata(address(_tokenB)).decimals()),
            position
        );
        isFactoryPool[pool] = true;

        emit PoolCreated(address(pool), _fee, _tickSpacing, _activeTick, _lookback, protocolFeeRatio, _tokenA, _tokenB);

        pools[_fee][_tickSpacing][_lookback][_tokenA][_tokenB] = pool;
    }

The user indeed can create any fee tier, and the creator of the pool can set very high pool tier to rug the user.

The pool creator can create a pool with 30% fee or 50% or even 80% or 100% fee.

The user that does not choose the fee setting careful will lose all the trade to the fee.

Tools Used

Manual Review

Recommended Mitigation Steps

We recommend the project limit the pool fee tier setting to

0.01% 0.04% 0.06% 0.10% 0.25% 0.35% 0.50% 1.00% 2.00% 3.00%

just like the code add reasonable upper bound for the protocol fee setting.

        require(_protocolFeeRatio <= ONE_3_DECIMAL_SCALE, "Factory:PROTOCOL_FEE_CANNOT_EXCEED_ONE");

kirk-baird commented 1 year ago

This is deliberate as stated in the docs under the heading Fee Structure. Thus, I'm marking this issue invalid.

Maverick AMM supports initializing pools with arbitrary fee rates, but it is expected that LPs will choose from one of the following “standard” fee rates:

c4-judge commented 1 year ago

kirk-baird marked the issue as unsatisfactory: Invalid

code-423n4 / 2022-12-Stealth-Project-findings