cToken funds are locked if Compound's exchange rate is 0

[code423n4]

Handle

pmerkleplant

Vulnerability details

Impact

If Compound's current rate, returned by Compound itself, is 0, then no cToken funds can be withdrawn as a panic error division by 0 would occur.

In case of such a fatal error in Compound the cTokens could maybe be exchanged in a compensation program and users would therefore still need the ability to withdraw them.

Proof of Concept

In case of a user's redeem of tokens, the TempusPool::burnShares() function is called (case 1, 2).

This function calls the updateInterestRate() function implemented in a protocol specific Tempus Pool, e.g. CompoundTempusPool.sol (link).

In case of Compound, the interest rate is returned as the result of the cTokens exchangeRateCurrent() function (link).

The newly fetched interest rate is then forwarded to TempusPool::getRedemptionAmounts() and from there to the protocol specific Tempus Pool's numYieldTokensPerAsset() function (link).

In case of the CompoundTempusPool implementation, this would result in an expression like (backingTokens * precision) / rate (link).

In case the Compound's specific cToken's rate is 0, this would lead to a panic error division by 0 and would therefore disable the ability to withdraw cToken funds.

Note that this issue is independent of the pool's maturity.

Recommended Mitigation Steps

Probably except the risk. If not, return redeemableBackingTokens = principalAmount and reedemableYieldBearingTokens = 0 in TempusPool::getRedemptionAmounts() if currentRate == 0.

[mijovic]

Well, this is a valid point. However, it is related to the underlying protocol being hacked for all the liquidity that it has. Considering that, I can't say what should be severity here, as it is mostly what the risk of protocol is (in this case Compound).

However, we will implement a way to unlock these yield bearing tokens in case of 0 exchange rate. Probably we will allow owner to unlock these tokens to some address (which can be contract, or governance, or something else).

[RedFox20]

I was working on seeing if this should be fixed and even implemented a potential workaround. However, now I'm not even sure if this is even possible on Compound: exchangeRate = (totalCash + totalBorrows - totalReserves) / totalSupply

Someone would need to hack the cToken, drain all totalCash, somehow also reset totalBorrows by repaying all debts and then ALSO drain all reserves by hacking the admin account. Furthermore, if there is a Math error, then exchangeRateStored() is reverted, so setting fallback rates would not work in that case.

I mean, if there's a nuclear holocaust then I think the funds are locked also, right now it looks just as much of an edge case.

If I added a function that would recover all YBT, it would also introduce a weak point into the pool itself and reduce trust if owner of the pool can drain all YBT, so that's not a good decision.

So essentially the only way here is to use fallback rates, but most likely if there's an issue with the pool, it will revert anyway, so there's no solution here.

[0xean]

Going to downgrade to 0 - non critical. The risk here is basically the risk of integrating with an external protocol and having that external protocol have a hard failure. It isn't unique to this contract set or Tempus but anything within the Compound ecosystems.