sherlock-audit / 2024-03-zivoe-judging

8 stars 6 forks source link

asui - Contract assumes all the stablecoins are always 1:1 with USD which is not the case incase of a depeg event. #105

Closed sherlock-admin4 closed 5 months ago

sherlock-admin4 commented 6 months ago

asui

high

Contract assumes all the stablecoins are always 1:1 with USD which is not the case incase of a depeg event.

Summary

Because the contract assume stablecoins are always 1:1 with USD and directly uses the amount to mint zJTT/zSTT, an attacker can profit from it incase one of the stablecoin depegs. See past depeg events here: USDT, DAI , USDC.

Vulnerability Detail

Since the contract doesn't use oracles, in case one stablecoin depegs(say USDC) and goes down in value( < USD ), it could be utilized by an attacker to mint more tranche tokens(zJTT/zSTT) than he should and profit from it by redeeming to another stablecoin(USDT, DAI, FRAX). Since, initially the protocol is going to use uniswap pools to ack as an exit(redeem) for zJTT/zSTT tokens the attacker can use flash loans to increase the amount of funds under attack. And even when the protocol implements OCR contract to redeem tranche tokens, the attack is still possible but just not in one transaction(i.e. no flash loans).

Example: USDC is 50% down(for the sake of our example), Alice mints deposits 100 USDC and mints 100 zSTT(because amount is used directly instead of oracles) when she should be given only 50 zSTT. Alice redeems 100 zSTT in USDT(uniswap pool/OCR contract) and gets 100 USDT and profits for 50 dollars which is a loss for innocent users.

Impact

Incase one of the stablecoins used depegs(which had happened in the past) it can be used to mint trancheTokens(zJTT/zSTT) and redeem it with another stablecoin for profit.

Code Snippet

The code that can be exploited under this event are the depositJunior and depositSenior functions in both ZivoeITO and ZivoeTranches contracts.

Tool used

Manual Review

Recommendation

The simplest mitigation would be to add oracle checks in the standardize function in ZivoeGlobals because this is called by all the deposit functions, and revert incase the given stablecoin depegs. Implement this function:

function _checkStable(address asset) internal {
    if(asset == USDC){
        check USDC/USD oracle and revert if not 1:1 with USD
    }
    if(asset == USDT) {
        check USDT/USD oracle and revert if not 1:1 with USD
    }
    if(asset == DAI) {
        check DAI/USD oracle and revert if not 1:1 with USD
    }
    if(asset == FRAX) {
        check FRAX/USD oracle and revert if not 1:1 with USD
    }

    // check chainlink oracles and revert if the given asset is not 1:1 with USD 
}

and add this line in the standardize function:


+      _checkOracle(asset);
pseudonaut commented 6 months ago

Not relevant

sherlock-admin4 commented 5 months ago

1 comment(s) were left on this issue during the judging contest.

panprog commented:

borderline low/medium, this is the choice/design of the protocol to lock stables to 1. There is no real loss of funds in case of depeg. During ITO: this will simply airdrop token and give tranche tokens in proportion to stablecoin amounts, not USD-denominated amounts. After ITO admins can whitelist/remove from whitelist the stables to prevent this issue. Still, this can cause harm to protocol in depeg events, so it's best to implement this measure.

pseudonaut commented 5 months ago

If USDC, USDT, or DAI depegs and doesn't stabilize within a relatively short period of time there would indeed be serious consequences across the entire ecosystem - really not valid or something we are going to control for

panprog commented 5 months ago

Downgrading to Informational.

I believe this must be accounted for, because major stablecoin depegs have happened in the past, so there must be a mitigation for such events. However, the way I see, the only way to prevent abuse of depegs for this protocol is to temporarily (or permanently) stop accepting deposits in these depegged stablecoins, and such measure already exists (admin can change stablecoinWhitelist in globals). As such, this issue is informational for the admin to be aware of what to do and create mitigation plan for such events.

zarkk01 commented 5 months ago

Hi @panprog ,

You are right that this bug is fixed if the stablecoin is removed from the whitelist but this action can be done only by ZVL as we see in the modifier. As it is stated in the discord channel(and is logical) ZVL is a multi-sig of internal team members. Since it is multi-sig, It is safe to assume that in the case of a flash depeg ZVL is not gonna be able to remove the stablecoin from whitelist in time. Also, this can be assumed(not so surely tho, but probably will happen) in the case of a "normal" depeg which can happen in the span of some minutes. This, of course, may have serious impact on the protocol. For this reason, I believe this is valid medium.

Thanks.