sherlock-audit / 2023-04-blueberry-judging

8 stars 5 forks source link

Bauer - Users can fail to closePositionFarm and lose their funds #64

Open sherlock-admin opened 1 year ago

sherlock-admin commented 1 year ago

Bauer

high

Users can fail to closePositionFarm and lose their funds

Summary

If self.is_killed in the curve pool contract becomes true, user may be unable to call the CurveSpell.closePositionFarm() function to repay his debt, resulting in his assets being liquidated.

Vulnerability Detail

The CurveSpell.closePositionFarm() function is used to unwind a position on a strategy that involves farming CRV rewards through staking LP tokens in a Curve pool. Inside the function, the protocol swaps the harvested CRV tokens to the debt token, and calculates the actual amount of LP tokens to remove from the Curve pool. It then removes the LP tokens using the remove_liquidity_one_coin function of the Curve pool.

   int128 tokenIndex;
            for (uint256 i = 0; i < tokens.length; i++) {
                if (tokens[i] == pos.debtToken) {
                    tokenIndex = int128(uint128(i));
                    break;
                }
            }

            ICurvePool(pool).remove_liquidity_one_coin(
                amountPosRemove,
                int128(tokenIndex),
                0
            );
        }

        // 5. Withdraw isolated collateral from Bank
        _doWithdraw(param.collToken, param.amountShareWithdraw);

        // 6. Repay
        {
            // Compute repay amount if MAX_INT is supplied (max debt)
            uint256 amountRepay = param.amountRepay;
            if (amountRepay == type(uint256).max) {
                amountRepay = bank.currentPositionDebt(bank.POSITION_ID());
            }
            _doRepay(param.borrowToken, amountRepay);
        }

        _validateMaxLTV(param.strategyId);

If self.is_killed in the curve pool contract becomes true, calling such remove_liquidity_one_coin() function would always revert. In this case, calling the CurveSpell.closePositionFarm() function reverts. When user's position is about to be liquidated, if the closePositionFarm() function is DOS'ed,user may be unable to repay his debt, resulting in the user losing their funds

def remove_liquidity_one_coin(
    _token_amount: uint256,
    i: int128,
    _min_amount: uint256
) -> uint256:
    """
    @notice Withdraw a single coin from the pool
    @param _token_amount Amount of LP tokens to burn in the withdrawal
    @param i Index value of the coin to withdraw
    @param _min_amount Minimum amount of coin to receive
    @return Amount of coin received
    """
    assert not self.is_killed  # dev: is killed

    dy: uint256 = 0
    dy_fee: uint256 = 0
    dy, dy_fee = self._calc_withdraw_one_coin(_token_amount, i)

Impact

If self.is_killed in the curve pool contract becomes true, user may be unable to repay his debt, resulting in his assets being liquidated.

Code Snippet

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

Tool used

Manual Review

Recommendation

Gornutz commented 1 year ago

https://github.com/Blueberryfi/blueberry-core/commit/bffe1167a2bc7038d64f23d6bd4301ed5c60dd29

IAm0x52 commented 1 year ago

Fix looks good. If pool is dead then it will remove as all tokens instead of a single token. No slippage protection for withdrawal has been given in this case but this is fine as pool is dead so getting sandwiched is impossible.