KupiaSec - The `FlashSwapRouter` mistakenly transfers certain `RA` tokens to `DS` buyers, resulting in financial losses for `lv` holders

[sherlock-admin2]

KupiaSec

High

The FlashSwapRouter mistakenly transfers certain RA tokens to DS buyers, resulting in financial losses for lv holders

Summary

In the _swapRaforDs() function, the process begins by swapping a specified amount of DS (denoted as amountSellFromReserve) for RA. This generated RA is intended to be treated as a fee. However, instead of being processed as a fee, the generated RA is sent directly to the buyer, while the fees are deducted from the ModuleCore contract.

Vulnerability Detail

In the _swapRaforDs() function, the following steps occur:

1. It swaps a specified amount of DS (denoted as amountSellFromReserve) for RA using the __swapDsforRa() function (line 124).
2. The generated RA is intended to be charged as a fee (line 125).

At line 124, the return value vaultRa represents the amount of RA generated in step 1. However, the __swapDsforRa() function has already sent this generated RA to the caller (in this case, the buyer). As a result, the fee charging in step 2 utilizes RA sourced from the IVault(owner())(the ModuleCore contract). This leads to a loss of funds for lv holders.

    function _swapRaforDs(
        ReserveState storage self,
        AssetPair storage assetPair,
        Id reserveId,
        uint256 dsId,
        uint256 amount,
        uint256 amountOutMin
    ) internal returns (uint256 amountOut) {
        uint256 borrowedAmount;

        // calculate the amount of DS tokens attributed
        (amountOut, borrowedAmount,) = assetPair.getAmountOutBuyDS(amount);

        // calculate the amount of DS tokens that will be sold from LV reserve
        uint256 amountSellFromReserve =
            amountOut - MathHelper.calculatePrecentageFee(self.reserveSellPressurePrecentage, amountOut);

        // sell all tokens if the sell amount is higher than the available reserve
        amountSellFromReserve = assetPair.reserve < amountSellFromReserve ? assetPair.reserve : amountSellFromReserve;

        // sell the DS tokens from the reserve if there's any
        if (amountSellFromReserve != 0) {
            // decrement reserve
            assetPair.reserve -= amountSellFromReserve;

            // sell the DS tokens from the reserve and accrue value to LV holders
124         uint256 vaultRa = __swapDsforRa(assetPair, reserveId, dsId, amountSellFromReserve, 0);
125         IVault(owner()).provideLiquidityWithFlashSwapFee(reserveId, vaultRa);

            // recalculate the amount of DS tokens attributed, since we sold some from the reserve
            (amountOut, borrowedAmount,) = assetPair.getAmountOutBuyDS(amount);
        }

        if (amountOut < amountOutMin) {
            revert InsufficientOutputAmount();
        }

        // trigger flash swaps and send the attributed DS tokens to the user

        __flashSwap(assetPair, assetPair.pair, borrowedAmount, 0, dsId, reserveId, true, amountOut);
    }

This issue arises because the __swapDsforRa() function has already dispatched the generated RA to the buyer. You can trace this by following the steps of the __swapDsforRa() function:

__swapDsforRa() -> __flashSwap() -> univ2Pair.swap() -> uniswapV2Call() -> __afterFlashswapSell() -> sending to caller

    function __afterFlashswapSell(
        ...

        // send caller their RA
404     IERC20(ra).safeTransfer(caller, raAttributed);
        ...

Impact

Loss of RA for lv holders.

Code Snippet

https://github.com/sherlock-audit/2024-08-cork-protocol/tree/main/Depeg-swap/contracts/core/flash-swaps/FlashSwapRouter.sol#L98-L138

https://github.com/sherlock-audit/2024-08-cork-protocol/tree/main/Depeg-swap/contracts/core/flash-swaps/FlashSwapRouter.sol#L380-L407

Tool used

Manual Review

Recommendation

It is recommended not to send RA directly to users within the __afterFlashswapSell() function. Instead, the generated RA should be processed after the swap is completed.

Duplicate of #161