Use wrong reserve values in `Pool.addLiquidity()`

[code423n4]

Lines of code

https://github.com/code-423n4/2022-12-Stealth-Project/blob/main/maverick-v1/contracts/models/Pool.sol#L113-L176

Vulnerability details

Impact

When doing Pool.addLiquidity, it would call bin.addLiquidity to add liquidity to the bins. And the calculation in bin.addLiquidity should be based on the amount of new tokens and the bin’s existing reserves.

However, Pool.addLiquidity uses temp.deltaA and temp.deltaB as the bin’s existing reserves. But temp.deltaA and temp.deltaB are calculated from _firstKindAtTickLiquidity(currentState.activeTick);. Which is not the bin’s existing reserves. This issue would cause bin.addLiquidity to calculate inaccurate results.

Proof of Concept

Pool.addLiquidity uses temp.deltaA and temp.deltaB as the bin’s existing reserves. (temp.reserveA, temp.reserveB) = _firstKindAtTickLiquidity(currentState.activeTick); And uses them as the bin’s existing reserves.

https://github.com/code-423n4/2022-12-Stealth-Project/blob/main/maverick-v1/contracts/models/Pool.sol#L113-L176

    function addLiquidity(
        uint256 tokenId,
        AddLiquidityParams[] calldata params,
        bytes calldata data
    ) external override returns (uint256 tokenAAmount, uint256 tokenBAmount, BinDelta[] memory binDeltas) {
        …

        AddLiquidityTemp memory temp;
        (temp.reserveA, temp.reserveB) = _firstKindAtTickLiquidity(currentState.activeTick);
        for (uint256 i; i < params.length; i++) {
            AddLiquidityParams memory param = params[i];
            require(param.kind < NUMBER_OF_KINDS);

            (uint128 binId, Bin.Instance storage bin) = _getOrCreateBin(currentState, param.kind, param.pos, param.isDelta);

            (temp.deltaA, temp.deltaB, temp.deltaLpBalance) = bin.addLiquidity(
                tokenId,
                Math.fromScale(param.deltaA, tokenAScale),
                Math.fromScale(param.deltaB, tokenBScale),
                currentState.activeTick,
                temp.reserveA,
                temp.reserveB
            );

            tokenAAmount += temp.deltaA;
            tokenBAmount += temp.deltaB;

            binDeltas[i] = BinDelta({
                binId: binId,
                deltaA: temp.deltaA.toUint128(),
                deltaB: temp.deltaB.toUint128(),
                kind: param.kind,
                isActive: true,
                lowerTick: bin.state.lowerTick,
                deltaLpBalance: temp.deltaLpBalance
            });
        }

        …
    }

Tools Used

None

Recommended Mitigation Steps

Use the bin’s existing reserves.

    function addLiquidity(
        uint256 tokenId,
        AddLiquidityParams[] calldata params,
        bytes calldata data
    ) external override returns (uint256 tokenAAmount, uint256 tokenBAmount, BinDelta[] memory binDeltas) {
        …        
        AddLiquidityTemp memory temp;
-       (temp.reserveA, temp.reserveB) = _firstKindAtTickLiquidity(currentState.activeTick);
        for (uint256 i; i < params.length; i++) {

            AddLiquidityParams memory param = params[i];
            require(param.kind < NUMBER_OF_KINDS);
+           (temp.reserveA, temp.reserveB) = _firstKindAtTickLiquidity(param.isDelta ? currentState.activeTick + param.pos : param.pos);
            (uint128 binId, Bin.Instance storage bin) = _getOrCreateBin(currentState, param.kind, param.pos, param.isDelta);

            (temp.deltaA, temp.deltaB, temp.deltaLpBalance) = bin.addLiquidity(
                tokenId,
                Math.fromScale(param.deltaA, tokenAScale),
                Math.fromScale(param.deltaB, tokenBScale),
                currentState.activeTick,
                temp.reserveA,
                temp.reserveB
            );

            tokenAAmount += temp.deltaA;
            tokenBAmount += temp.deltaB;

            binDeltas[i] = BinDelta({
                binId: binId,
                deltaA: temp.deltaA.toUint128(),
                deltaB: temp.deltaB.toUint128(),
                kind: param.kind,
                isActive: true,
                lowerTick: bin.state.lowerTick,
                deltaLpBalance: temp.deltaLpBalance
            });
        }
        …
    }

[gte620v]

This is not an issue. For all other ticks besides the active tick, the bin.addLiquidity computes the reserve ratio: https://github.com/code-423n4/2022-12-Stealth-Project/blob/main/maverick-v1/contracts/libraries/Bin.sol#L35-L40

We use the fact that if we aren't in the active tick, then the bin is either all A or all B, so we don't care about the existing reserves in those cases.

This finding is somewhat related to https://github.com/code-423n4/2022-12-Stealth-Project-findings/issues/103#issuecomment-1354109865

[c4-sponsor]

gte620v marked the issue as sponsor disputed

[kirk-baird]

As the sponsor has stated, the reserve ratio is the same for all bins in the active tick.

For bins outside the active tick the value must be calculated since one side is always zero.

[c4-judge]

kirk-baird marked the issue as unsatisfactory: Invalid