sherlock-audit / 2024-08-cork-protocol-judging

2 stars 2 forks source link

Melodic Vermilion Coyote - Underflow risk in `swapDsforRa` and `swapRaforDs`in `DsFlashSwap.sol :getAmountOutSellDs` #304

Closed sherlock-admin3 closed 2 months ago

sherlock-admin3 commented 2 months ago

Melodic Vermilion Coyote

Low/Info

Underflow risk in swapDsforRa and swapRaforDsin DsFlashSwap.sol :getAmountOutSellDs

Summary

The getAmountOutSellDS function in the DsFlashSwap https://github.com/sherlock-audit/2024-08-cork-protocol/blob/main/Depeg-swap/contracts/libraries/DsFlashSwap.sol#L170 library has a potential underflow vulnerability. This occurs when the repaymentAmount exceeds the amount parameter, leading to a panic error when attempting to compute amountOut. This situation can arise during the calculation of repayment amounts based on the reserves, particularly when the reserves are imbalanced.

Root Cause

Conditions for repaymentAmount > amount

  1. Insufficient Reserves: If the reserves of raReserve and ctReserve are imbalanced, particularly if ctReserve is significantly lower than amount, the calculation may yield a high repaymentAmount.
  2. High Amount Input: If the amount (the CT amount being sold) is large relative to the available reserves, it can lead to a situation where the required repayment amount exceeds the input amount.
  3. Market Conditions: If the market conditions (e.g., price slippage, liquidity) are such that the amount of RA that needs to be borrowed to cover the CT being sold is disproportionately high, this can also lead to a higher repaymentAmount.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

No response

PoC

No response

Mitigation

To mitigate this risk, the function should include a check to ensure that repaymentAmount does not exceed amount before performing the subtraction. If the check fails, the function should revert with an appropriate error message. Here’s a suggested modification:

function getAmountOutSellDS(
    AssetPair storage assetPair,
    uint256 amount
) internal view returns (uint256 amountOut, uint256 repaymentAmount) {
    (uint112 raReserve, uint112 ctReserve) = getReservesSorted(assetPair);

    repaymentAmount = MinimalUniswapV2Library.getAmountIn(
        amount,
        raReserve,
        ctReserve - amount
    );

    // Check to prevent underflow
    require(repaymentAmount <= amount, "Repayment amount exceeds the amount");

    amountOut = amount - repaymentAmount;
}

This modification ensures that the function will revert if an underflow condition is detected, thus preventing potential exploitation and maintaining the integrity of the contract.

sherlock-admin2 commented 1 month ago

The protocol team fixed this issue in the following PRs/commits: https://github.com/Cork-Technology/Depeg-swap/pull/72