CurveVolatileCollateral Collateral status can be manipulated by flashloan attack

[code423n4]

Lines of code

https://github.com/reserve-protocol/protocol/blob/9ee60f142f9f5c1fe8bc50eef915cf33124a534f/contracts/plugins/assets/curve/CurveVolatileCollateral.sol#L32-L65

Vulnerability details

Impact

Attacker can make the CurveVolatileCollateral enter the status of IFFY/DISABLED . It will cause the basket to rebalance and sell off all the CurveVolatileCollateral.

Proof of Concept

The CurveVolatileCollateral overrides the _anyDepeggedInPool function to check if the distribution of capital is balanced. If the any part of underlying token exceeds the expected more than _defaultThreshold, return true, which means the volatile pool has been depeg:

uint192 expected = FIX_ONE.divu(nTokens); // {1}
for (uint8 i = 0; i < nTokens; i++) {
    uint192 observed = divuu(vals[i], valSum); // {1}
    if (observed > expected) {
        if (observed - expected > _defaultThreshold) return true;
    }
}

And the coll status will be updated in the super class CurveStableCollateral.refresh():

if (low == 0 || _anyDepeggedInPool() || _anyDepeggedOutsidePool()) {
    markStatus(CollateralStatus.IFFY);
}

The attack process is as follows:

1. Assumption: There is a CurveVolatileCollateral bases on a TriCrypto ETH/WBTC/USDT, and the vaule of them should be 1:1:1, and the _defaultThreshold of the CurveVolatileCollateral is 5%. And at first, there are 1000 USDT in the pool and the pool is balanced.

2. The attacker uses flash loan to deposit 500 USDT to the pool. Now, the USDT distribution is 1500/(1500+1000+1000) = 42.86% .

3. Attacker refresh the CurveVolatileCollateral. Because the USDT distribution - expected = 42.86% - 33.33% = 9.53% > 5% _defaultThreshold . So CurveVolatileCollateral will be marked as IFFY.

4. The attacker withdraw from the pool and repay the USDT.

5. Just wait delayUntilDefault, the collateral will be marked as defaulted by the alreadyDefaulted function.

   function alreadyDefaulted() internal view returns (bool) {
       return _whenDefault <= block.timestamp;
   }

Tools Used

Manual review

Recommended Mitigation Steps

I think the depegged status in the volatile pool may be unimportant. It will be temporary and have little impact on the price of outside lp tokens. After all, override the _anyDepeggedOutsidePool to check the lp price might be a good idea.

Assessed type

Context

[c4-judge]

thereksfour marked the issue as primary issue

[c4-sponsor]

tbrent marked the issue as sponsor confirmed

[c4-judge]

thereksfour marked the issue as satisfactory

[c4-judge]

thereksfour marked the issue as selected for report