If user provides more tokens as was expected while adding liquidity, no more liquidity minted to him

[code423n4]

Lines of code

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

Vulnerability details

Impact

When calling the swap function below, the ISwapCallback(msg.sender).swapCallback(amountIn, amountOut, data) is called. It's possible that contract will send more input tokens amount than required to the pool. The check allows this. When this happens, the output token amount corresponding to the extra input token amount will not be transferred from the pool to the recipient after the swap. As a result, the user does not receive any output tokens to compensate input tokens he sent.

Proof of Concept

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

    function swap(address recipient, uint256 amount, bool tokenAIn, bool exactOutput, uint256 sqrtPriceLimit, bytes calldata data) external returns (uint256 amountIn, uint256 amountOut) {
        State memory currentState = checkReentrancy(false, true);
        // ************************ reentrancy ************************

        Delta.Instance memory delta;

        int32 startingTick = currentState.activeTick;
        int32 lastTwa = twa.floor();

        {
            delta.excess = (!exactOutput) ? Math.fromScale(amount, tokenAIn ? tokenAScale : tokenBScale) : Math.fromScale(amount, !tokenAIn ? tokenAScale : tokenBScale);

            delta.tokenAIn = tokenAIn;
            delta.exactOutput = exactOutput;
            delta.sqrtPriceLimit = sqrtPriceLimit;
        }

        while (delta.excess > 0) {
            Delta.Instance memory newDelta;
            newDelta = _swapTick(currentState, delta);
            delta.combine(newDelta);
        }

        amountIn = Math.toScale(delta.deltaInErc, tokenAIn ? tokenAScale : tokenBScale, exactOutput || delta.swappedToMaxPrice);
        amountOut = Math.toScale(delta.deltaOutErc, tokenAIn ? tokenBScale : tokenAScale, false);

        if (amountOut != 0) {
            if (tokenAIn) {
                binBalanceA += delta.deltaInBinInternal.toUint128();
                binBalanceB = Math.clip128(binBalanceB, delta.deltaOutErc.toUint128());
            } else {
                binBalanceB += delta.deltaInBinInternal.toUint128();
                binBalanceA = Math.clip128(binBalanceA, delta.deltaOutErc.toUint128());
            }
            twa.updateValue(currentState.activeTick * PRBMathSD59x18.SCALE + int256(Math.clip(delta.endSqrtPrice, delta.sqrtLowerTickPrice).div(delta.sqrtUpperTickPrice - delta.sqrtLowerTickPrice)));
        }

        state.activeTick = currentState.activeTick;

        uint256 previousBalance;
        SafeERC20.safeTransfer(IERC20(tokenAIn ? tokenB : tokenA), recipient, amountOut);
        previousBalance = tokenAIn ? _tokenABalance() : _tokenBBalance();
        ISwapCallback(msg.sender).swapCallback(amountIn, amountOut, data);
        require(previousBalance + amountIn <= (tokenAIn ? _tokenABalance() : _tokenBBalance()), "S");

        emit Swap(msg.sender, recipient, tokenAIn, exactOutput, amountIn, amountOut, currentState.activeTick);
        _moveBins(currentState.activeTick, startingTick, lastTwa);

        // ************************ reentrancy ************************
        state.status = NO_EMERGENCY_UNLOCKED;
    }

After the swap there is a check that previous balance + swapped amount is less than current balance. require(previousBalance + amountIn <= (tokenAIn ? _tokenABalance() : _tokenBBalance()), "S");

But if user paid more input tokens, then no extra output tokens were sent to user.

Tools Used

VsCode

Recommended Mitigation Steps

Do not allow to pay more tokens than expected.

[c4-judge]

kirk-baird marked the issue as primary issue

[gte620v]

This does not seem to be a contract bug at all. It is similar to saying a user can send unexpected funds to a contract and not get anything in return. That is the expected behavior.

The user should program their callback to not send more tokens than are required. Or the user can use the router.

[c4-sponsor]

gte620v marked the issue as sponsor disputed

[kirk-baird]

I agree with the warden that there is risk in allowing the user to accidentally transfer more funds than required. However, this requires a significant user error as the quantity required for transfer is specified in the external call swapCallback().

Since this issue requires significant user error I consider it to be LOW and therefore downgrade it to QA.

[c4-judge]

Duplicate of https://github.com/code-423n4/2022-12-Stealth-Project-findings/issues/24

[c4-judge]

kirk-baird marked the issue as grade-b