PositionAction.sol#decreaseLever with EXACT_IN 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#L488

Vulnerability details

Impact

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

Bug Description

Note: This is a new issue that was introduced by the latest code diff (Doesn't exist in the 2024-07 Loopfi contest).

When performing decreaseLever, in the flashloan fallback onCreditFlashLoan(), we need to payoff the flashloan along with the fees.

For EXACT_IN primary swaps, if extra underlying tokens are swapped out, they are sent back to the vault as payed off debt. However, the issue here is when calculating the residual amount, it does not take flashloan fees into account.

The buggy line is uint256 residualAmount = swapAmountOut - subDebt;, where subDebt is the amount of token taken from flashloan, but fees also need to be subtracted here.

    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) {
            leverParams.primarySwap.amount = withdrawnCollateral;

            bytes memory swapData = _delegateCall(
                address(swapAction),
                abi.encodeWithSelector(swapAction.swap.selector, leverParams.primarySwap)
            );

            uint256 swapAmountOut = abi.decode(swapData, (uint256));
            
            // @audit-bug: FEES ARE NOT TAKEN INTO ACCOUNT.
@>          uint256 residualAmount = swapAmountOut - subDebt;

            // sub collateral and debt
            if (residualAmount > 0) {
                underlyingToken.forceApprove(address(leverParams.vault), residualAmount);
                ICDPVault(leverParams.vault).modifyCollateralAndDebt(
                    leverParams.position,
                    address(this),
                    address(this),
                    0,
                    -toInt256(residualAmount)
                );
            }
        } else {
            ...
        }

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

Proof of Concept

N/A

Tools Used

Manual Review

Recommended Mitigation Steps

Change to uint256 residualAmount = swapAmountOut - subDebt - fee;

Assessed type

DoS

[c4-judge]

koolexcrypto marked the issue as duplicate of #27

[c4-judge]

koolexcrypto marked the issue as satisfactory