0xNirix - Exchange Rate is neglected in DS-RA Swaps causing Issues like Fund Loss

[sherlock-admin3]

0xNirix

Medium

Exchange Rate is neglected in DS-RA Swaps causing Issues like Fund Loss

Summary

Failure to account for DS+CT to RA exchange rate will cause an incorrect valuation of user tokens for swappers as the contract will incorrectly calculate swap amounts and flash loan repayments.

Root Cause

User Initiates Swap:

function swapDsforRa(Id reserveId, uint256 dsId, uint256 amount, uint256 amountOutMin) external returns (uint256 amountOut) {
    AssetPair storage assetPair = reserves[reserveId].ds[dsId];
    assetPair.ds.transferFrom(msg.sender, address(this), amount);
    amountOut = __swapDsforRa(assetPair, reserveId, dsId, amount, amountOutMin);
}

At this point, amount is the DS amount, but it's not adjusted for the exchange rate.

Core Swap Logic in __swapDsforRa:

function __swapDsforRa(AssetPair storage assetPair, Id reserveId, uint256 dsId, uint256 amount, uint256 amountOutMin) internal returns (uint256 amountOut) {
    (amountOut,) = assetPair.getAmountOutSellDS(amount);
    if (amountOut < amountOutMin) {
        revert InsufficientOutputAmount();
    }
    __flashSwap(assetPair, assetPair.pair, 0, amount, dsId, reserveId, false, amountOut);
}

getAmountOutSellDS doesn't consider the exchange rate, potentially undervaluing amountOut.

https://github.com/sherlock-audit/2024-08-cork-protocol/blob/main/Depeg-swap/contracts/libraries/DsFlashSwap.sol#L166-L170

    function getAmountOutSellDS(AssetPair storage assetPair, uint256 amount)
        internal
        view
        returns (uint256 amountOut, uint256 repaymentAmount)
    {
        (uint112 raReserve, uint112 ctReserve) = getReservesSorted(assetPair);
        // we calculate the repayment amount based on the imbalanced ct reserve since we borrow CT from the AMM
        repaymentAmount = MinimalUniswapV2Library.getAmountIn(amount, raReserve, ctReserve - amount);


        // the amountOut is essentially what the user receive, we can calculate this by simply subtracting the repayment amount
        // from the amount, since we're getting back the same RA amount as DS user buy, this works. to get the effective price per DS,
        // you would devide this by the DS amount user bought.
        // note that we subtract 1 to enforce uni v2 rules
        amountOut = amount - repaymentAmount;


        // enforce uni v2 rules, pay 1 wei more
        amountOut -= 1;
        repaymentAmount += 1;


        assert(amountOut + repaymentAmount == amount);

Flash Swap Initiation:
The __flashSwap function initiates the flash loan, borrowing the calculated amountOut of RA.
Flash Swap Callback (uniswapV2Call):

function uniswapV2Call(address sender, uint256 amount0, uint256 amount1, bytes calldata data) external {
    // ... (verification code)
    __afterFlashswapSell(self, amount, reserveId, dsId, caller, extraData);
}

After Flash Swap Sell (__afterFlashswapSell):

function __afterFlashswapSell(ReserveState storage self, uint256 ctAmount, Id reserveId, uint256 dsId, address caller, uint256 raAttributed) internal {
    AssetPair storage assetPair = self.ds[dsId];
    assetPair.ds.approve(owner(), ctAmount);
    assetPair.ct.approve(owner(), ctAmount);

    IPSMcore psm = IPSMcore(owner());
    (uint256 received,) = psm.redeemRaWithCtDs(reserveId, ctAmount);

    uint256 repaymentAmount = received - raAttributed;
    Asset ra = assetPair.ra;

    IERC20(ra).safeTransfer(caller, raAttributed);
    IERC20(ra).safeTransfer(msg.sender, repaymentAmount);
}

The redeemRaWithCtDs accounts for the exchange rate redeeming expected correct RA.
raAttributed (passed as extraData in the flash swap) is based on the incorrect amountOut calculated earlier.
The repayment amount is calculated based on this incorrect raAttributed, leading to an excess repayment to the flash loan provider.

A similar issue exists for the RA to DS swap process in getAmountOutBuyDS and afterFlashswapBuy.

While there is slippage protection via min amounts, but since preview methods also do the same computation, user provided min amounts will likely be wrong in the first place.

Internal pre-conditions

1. DS+CT to RA exchange rate needs to be greater than 1:1 which is expected as per spec.

External pre-conditions

No response

Attack Path

1. User calls swapDsforRa function with a certain amount of DS tokens
2. Contract calculates amountOut without considering the exchange rate
3. Contract initiates a flash loan for the undervalued amountOut of RA
4. In the callback, contract redeems RA from PSM without adjusting for exchange rate
5. Contract transfers the undervalued raAttributed amount to the user
6. Excess RA is used to repay the flash loan

Impact

The users suffer an approximate loss proportional to the difference between the actual exchange rate and 1:1 for DS for RA swap. Similar issue exist in RA for DS swap.

PoC

No response

Mitigation

No response

Duplicate of #119