Skip to content

Latest commit

 

History

History
113 lines (85 loc) · 4.89 KB

064.md

File metadata and controls

113 lines (85 loc) · 4.89 KB

Low Tangerine Cod

High

Accounting of totalCdsDepositedAmount will broken after CDS user withdraw

Summary

Unused liquidation amount is deducted from the deposited amount instead of the used liquidation amount.

Root Cause

Whenever user who opted in for liquidation will withdraw in params.cdsDepositDetails.liquidationAmount will be amount unused in liquidation, due to the fact that protocol deduction here

                for (uint128 i = (liquidationIndexAtDeposit + 1); i <= params.omniChainData.noOfLiquidations; i++) {
                    uint128 liquidationAmount = params.cdsDepositDetails.liquidationAmount;
                    // If the user available liquidation is non zero
                    if (liquidationAmount > 0) {
                        CDSInterface.LiquidationInfo memory liquidationData = omniChainCDSLiqIndexToInfo[i];
                        // Calculate the share by taking ratio between
                        // User's available liquidation amount and total available liquidation amount
                        uint128 share = (liquidationAmount * 1e10) / uint128(liquidationData.availableLiquidationAmount);
                        // Update users available liquidation amount
->                        params.cdsDepositDetails.liquidationAmount -= getUserShare(liquidationData.liquidationAmount, share);
                        // Based on the collateral type calculate the liquidated collateral to give to user
                        if (liquidationData.assetName == IBorrowing.AssetName.ETH) {
                            // increment eth amount
                            params.ethAmount += getUserShare(liquidationData.collateralAmount, share);
                        } else if (liquidationData.assetName == IBorrowing.AssetName.WeETH) {
                            // increment weeth amount and weth amount value
                            weETHAmount += getUserShare(liquidationData.collateralAmount, share);
                            weETHAmountInETHValue += getUserShare(liquidationData.collateralAmountInETHValue, share);
                        } else if (liquidationData.assetName == IBorrowing.AssetName.WrsETH) {
                            // increment rseth amount and rseth amount value
                            rsETHAmount += getUserShare(liquidationData.collateralAmount, share);
                            rsETHAmountInETHValue += getUserShare(liquidationData.collateralAmountInETHValue, share);
                        }
                    }
                }

contracts/lib/CDSLib.sol#L648

Lets look how omniChainData.totalCdsDepositedAmount - X will changed after liquidation + withdrawal. Lets assume cdsProfits is 0. After liquidation:

        omniChainData.totalCdsDepositedAmount -= liquidationAmountNeeded - cdsProfits; //

$$ X -= usedLiquidity $$

after withdrawal

$$ X -= deposit - unusedLiquidity $$

Now, sum them together which should be in total deposit due to the fact that this is amount user initially deposited

$$ X -= deposit - unusedLiquidity + usedLiquidity $$

At the end after cds deposit+liquidation+wtidhraw:

$$ X += deposit - (deposit - unusedLiquidity + usedLiquidity) $$

$$ X += - unusedLiquidity + usedLiquidity) $$

What does it mean for protocol? User's funds at some point will be stuck if calculation above less than 0 due to underflow. Also cumulative rate depends on this value so it will be incorrect which means deposits/withdrawals will not be tracked properly as well

Internal pre-conditions

none

External pre-conditions

none

Attack Path

always happening

Impact

Worst case

  1. Funds will be stuck at some point.
  2. deposits and withdraw will be tracked incorrectly in favour of users or protocol

PoC

Mitigation

                } else {
                    // update totalCdsDepositedAmount
+                    params.cdsDepositDetails.liquidationAmount = params.cdsDepositDetails.initialLiquidationAmount - params.cdsDepositDetails.liquidationAmount 
                    totalCdsDepositedAmount -= (params.cdsDepositDetails.depositedAmount - params.cdsDepositDetails.liquidationAmount);
                    params.omniChainData.totalCdsDepositedAmount -= (params.cdsDepositDetails.depositedAmount - params.cdsDepositDetails.liquidationAmount);
                    // update totalCdsDepositedAmountWithOptionFees
                    totalCdsDepositedAmountWithOptionFees -= (params.cdsDepositDetails.depositedAmount - params.cdsDepositDetails.liquidationAmount + params.optionsFeesToGetFromOtherChain);
                    params.omniChainData.totalCdsDepositedAmountWithOptionFees -= (
                        params.cdsDepositDetails.depositedAmount -
                        params.cdsDepositDetails.liquidationAmount +
                        params.optionFees);
                }