PositionAction.sol#decreaseLever with EXACT_OUT primary swaps would always brick if swapFee is non-zero.

[c4-bot-9]

Lines of code

https://github.com/code-423n4/2024-10-loopfi/blob/main/src/proxy/PositionAction.sol#L377

Vulnerability details

Impact

PositionAction.sol#decreaseLever with EXACT_OUT primary swaps would always brick if swapFee is non-zero.

Bug Description

Note: This is based on the 2024-07 Loopfi contest code-423n4/2024-07-loopfi-findings#191 issue. This protocol team applied a fix, but the fix is incomplete.

When performing decreaseLever, if the primary swap is EXACT_OUT, this means we are swapping collateral token an exact amount of debt token.

However, the output amount leverParams.primarySwap.amount is always equal to the flashLoan amount. This means we can only repay the original flashloan amount. But if fees is non-zero, the repayment will always fail.

Note that the team made the change where fee is approved in onCreditFlashLoan, but since the amount of tokens doesn't include fees, it would still not work.

    function decreaseLever(
        LeverParams memory leverParams,
        uint256 subCollateral,
        address residualRecipient
    ) external onlyDelegatecall onlyRegisteredVault(leverParams.vault) {
        // validate the primary swap
        if (leverParams.primarySwap.recipient != self) revert PositionAction__decreaseLever_invalidPrimarySwap();

        IPermission(leverParams.vault).modifyPermission(leverParams.position, self, true);

        if (leverParams.primarySwap.swapType == SwapType.EXACT_OUT) {
            uint256 totalDebt = ICDPVault(leverParams.vault).virtualDebt(leverParams.position);
            leverParams.primarySwap.amount = min(totalDebt, leverParams.primarySwap.amount);

            // residual recipient is required if the primary swap is an exact out swap
            if (residualRecipient == address(0)) {
                revert PositionAction__decreaseLever_invalidResidualRecipient();
            }
        }

        // take out credit flash loan
        flashlender.creditFlashLoan(
            ICreditFlashBorrower(self),
@>          leverParams.primarySwap.amount,
            abi.encode(leverParams, subCollateral, residualRecipient)
        );

        IPermission(leverParams.vault).modifyPermission(leverParams.position, self, false);
    }

    function onCreditFlashLoan(
        address /*initiator*/,
        uint256 /*amount*/,
        uint256 fee,
        bytes calldata data
    ) external returns (bytes32) {
        if (msg.sender != address(flashlender)) revert PositionAction__onCreditFlashLoan__invalidSender();
        (LeverParams memory leverParams, uint256 subCollateral, address residualRecipient) = abi.decode(
            data,
            (LeverParams, uint256, address)
        );

        uint256 subDebt = leverParams.primarySwap.amount;
        underlyingToken.forceApprove(address(leverParams.vault), subDebt + fee);
        // sub collateral and debt
        ICDPVault(leverParams.vault).modifyCollateralAndDebt(
            leverParams.position,
            address(this),
            address(this),
            0,
            -toInt256(subDebt)
        );
        
        // withdraw collateral and handle any CDP specific actions
        uint256 withdrawnCollateral = _onDecreaseLever(leverParams, subCollateral);

        if (leverParams.primarySwap.swapType == SwapType.EXACT_IN) {
            ...
        } else {
@>          bytes memory swapData = _delegateCall(
                address(swapAction),
                abi.encodeWithSelector(swapAction.swap.selector, leverParams.primarySwap)
            );
            uint256 swapAmountIn = abi.decode(swapData, (uint256));

            // swap collateral to stablecoin and calculate the amount leftover
            uint256 residualAmount = withdrawnCollateral - swapAmountIn;

            // send left over collateral that was not needed to payback the flash loan to `residualRecipient`
            if (residualAmount > 0) {
                // perform swap from collateral to arbitrary token if necessary
                if (leverParams.auxSwap.assetIn != address(0) && leverParams.auxSwap.swapType == SwapType.EXACT_IN) {
                    leverParams.auxSwap.amount = residualAmount;
                    _delegateCall(
                        address(swapAction),
                        abi.encodeWithSelector(swapAction.swap.selector, leverParams.auxSwap)
                    );
                } else {
                    // otherwise just send the collateral to `residualRecipient`
                    IERC20(leverParams.primarySwap.assetIn).safeTransfer(residualRecipient, residualAmount);
                }
            }
        }

@>      underlyingToken.forceApprove(address(flashlender), subDebt + fee);
        return CALLBACK_SUCCESS_CREDIT;
    }

Proof of Concept

N/A

Tools Used

Manual Review

Recommended Mitigation Steps

Also add swapFees to EXACT_OUT swap output, preferrably inside the onCreditFlashLoan() function.

Assessed type

DoS

[c4-judge]

koolexcrypto marked the issue as duplicate of #27

[c4-judge]

koolexcrypto marked the issue as satisfactory