sherlock-audit / 2023-04-blueberry-judging

8 stars 5 forks source link

Bauer - Lack of deadline for uniswap AMM #65

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Bauer

medium

Lack of deadline for uniswap AMM

Summary

The CurveSpell.closePositionFarm() function lacks a deadline check, making it vulnerable to sandwich attacks that can result in users losing their assets

Vulnerability Detail

The CurveSpell.closePositionFarm() params does not include a deadline currently. Inside the CurveSpell.closePositionFarm() function swaps are executed through the swapRouter.

 function closePositionFarm(
        ClosePosParam calldata param,
        IUniswapV2Router02 swapRouter,
        address[] calldata swapPath
    )
        external
        existingStrategy(param.strategyId)
        existingCollateral(param.strategyId, param.collToken)
    {
        address crvLp = strategies[param.strategyId].vault;
        IBank.Position memory pos = bank.getCurrentPositionInfo();
        if (pos.collToken != address(wCurveGauge))
            revert Errors.INCORRECT_COLTOKEN(pos.collToken);
        if (wCurveGauge.getUnderlyingToken(pos.collId) != crvLp)
            revert Errors.INCORRECT_UNDERLYING(crvLp);

        // 1. Take out collateral - Burn wrapped tokens, receive crv lp tokens and harvest CRV
        bank.takeCollateral(param.amountPosRemove);
        wCurveGauge.burn(pos.collId, param.amountPosRemove);

        {
            // 2. Swap rewards tokens to debt token
            uint256 rewards = _doCutRewardsFee(CRV);
            _ensureApprove(CRV, address(swapRouter), rewards);
            swapRouter.swapExactTokensForTokens(
                rewards,
                0,
                swapPath,
                address(this),
                type(uint256).max
            );
        }

Because Front-running is a key aspect of AMM design, deadline is a useful tool to ensure that your tx cannot be “saved for later”.

Due to the removal of the check, it may be more profitable for a validator to deny the transaction from being added until the transaction incurs the maximum amount of slippage.

Impact

Sandwich attacks cause users to lose assets

Code Snippet

https://github.com/sherlock-audit/2023-04-blueberry/blob/main/blueberry-core/contracts/spell/CurveSpell.sol#L143-L174

Tool used

Manual Review

Recommendation

The CurveSpell.closePositionFarm() function should accept a user-input deadline param

Duplicate of #145