bitsurfer - AuraSpell close position open for slippage issue due to `minAmountsOut` is 0, no deadline check and the ClosePosParam's `amountOutMin` value is ignored

[sherlock-admin2]

bitsurfer

high

AuraSpell close position open for slippage issue due to minAmountsOut is 0, no deadline check and the ClosePosParam's amountOutMin value is ignored

Summary

AuraSpell close position open for slippage issues due to minAmountsOut is 0, no deadline check and the ClosePosParam's amountOutMin value is ignored

Vulnerability Detail

AuraSpell doesn't check the amountOutMin and it will open for slippage issue. The ClosePosParam in closePositionFarm indeed contains amountOutMin to handle slippage, but this value is not being checked anywhere in AuraSpell.

Moreover, if we analyze the _getExitPoolParams function, the minAmountsOut returned is set with a default value 0.

File: BasicSpell.sol
65:     /// @dev Defines parameters required for closing a position.
66:     /// @param strategyId Identifier for the strategy to close.
67:     /// @param collToken Address of the isolated collateral token.
68:     /// @param borrowToken Address of the token representing the debt.
69:     /// @param amountRepay Amount of debt to repay.
70:     /// @param amountPosRemove Amount of position to withdraw.
71:     /// @param amountShareWithdraw Amount of isolated collateral tokens to withdraw.
72:     /// @param amountOutMin Minimum amount to receive after the operation (used to handle slippage).
73:     struct ClosePosParam {
74:         uint256 strategyId;
75:         address collToken;
76:         address borrowToken;
77:         uint256 amountRepay;
78:         uint256 amountPosRemove;
79:         uint256 amountShareWithdraw;
80:         uint256 amountOutMin;
81:     }

File: AuraSpell.sol
354:     function _getExitPoolParams(
355:         address borrowToken,
356:         address lpToken
357:     ) internal view returns (uint256[] memory, address[] memory, uint256) {
358:         (address[] memory tokens, , ) = wAuraPools.getPoolTokens(lpToken);
359:
360:         uint256 length = tokens.length;
361:         uint256[] memory minAmountsOut = new uint256[](length);
362:         uint256 exitTokenIndex;
363:
364:         for (uint256 i; i != length; ) {
365:             if (tokens[i] == borrowToken) break;
366:
367:             if (tokens[i] != lpToken) ++exitTokenIndex;
368:
369:             unchecked {
370:                 ++i;
371:             }
372:         }
373:
374:         return (minAmountsOut, tokens, exitTokenIndex);
375:     }

Furthermore, AuraSpell's closePositionFarm doesn't have deadline check unlike CurveSpell's closePositionFarm. CurveSpell have deadline and the minAmountsOut is applied on remove_liquidity_one_coin, but it's not the case on AuraSpell.

Lack of deadline check, minAmountsOut is 0, ignoring the ClosePosParam's amountOutMin exposes a potential vulnerability in the AuraSpell implementation which could result in unfavorable slippage outcomes during various operations, like sandwich attack or delayed or stale transaction.

The deadline check ensure that the transaction can be executed on time and the expired transaction revert.

While the ClosePosParam within closePositionFarm acknowledges the importance of amountOutMin to mitigate slippage risks, the crucial step of verifying this parameter is omitted within the context of AuraSpell.

Impact

With a minAmountsOut set to 0, and doesn't have a deadline check, this AuraSpell close position open for slippage, due to a sandwich attack or any bad price (position) returned from stale/stuck transactions, thus it may cause a loss of funds.

Code Snippet

https://github.com/sherlock-audit/2023-07-blueberry/blob/main/blueberry-core/contracts/spell/AuraSpell.sol#L361

Tool used

Manual Review

Recommendation

Implement deadline check and also apply the correct amountOutMin on exitPool when user is trying to close position

Duplicate of #102