code-423n4 / 2022-06-canto-v2-findings

0 stars 0 forks source link

A cap is needed on the amount of Note than can be borrowed #92

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/CNote.sol#L33 https://github.com/Plex-Engineer/lending-market-v2/blob/ea5840de72eab58bec837bb51986ac73712fcfde/contracts/Note.sol#L14

Vulnerability details

Impact

The fact that there is no cap on the amount of Note that can be borrowed makes the Oracle Extractable Value unlimited. But as you intend to rely on TWAP, you need to make sure the cost of oracle manipulation is lower than the Oracle Extractable Value.

Proof of Concept

By manipulating the TWAPs of the designated proxy used for Note (USDC ?) and its relative price to a given collateral(which would be highly costly), an attacker could borrow Note without limit, and empty all pools related to Note and all Note-related value, instantly killing the stablecoin.

The value extractable by Oracle Manipulations is usually easily computable as it is the size of the lending market, but here, it’s more difficult to evaluate as it could potentially be any value linked to Note. This makes risk management harder and increase significantly the risk of attack.

Therefore a cap on how many Notes can be borrowed needs to be added to mitigate this risk.

The attack would be:

Essentially as you have no cap on the amount of Note that could be borrowed in such a scenario, you cannot be sure that the potential attack profits are lower than the attack cost.

Recommended Mitigation Steps

The governance needs to set a limit on how much Note can be borrowed to mitigate risks, or add for example an “hourly” borrowing limit.

Easiest way to do this would be able to mint / burn from the accountant

GalloDaSballo commented 2 years ago

I don't think you can manipulate the price of cNOTE per this code

https://github.com/Plex-Engineer/lending-market-v2/blob/443a8c0fed3c5018e95f3881a31b81a555c42b2d/contracts/Stableswap/BaseV1-periphery.sol#L499-L502

        //set price statically to 1 when the Comptroller is retrieving Price
        else if (compareStrings(ctoken.symbol(), "cNOTE") && msg.sender == Comptroller) {
            return 1; // Note price is fixed to 1
        }

However, you can manipulate the price of another token against USDC

https://github.com/Plex-Engineer/lending-market-v2/blob/443a8c0fed3c5018e95f3881a31b81a555c42b2d/contracts/Stableswap/BaseV1-periphery.sol#L529-L533

        else {
            stablePair = (stable == 0) ? false : true;
            pair = IBaseV1Pair(pairFor(USDC, underlying, stablePair)); //get the pair for the USDC/underlying pool
            price = pair.quote(underlying, 1, 8); //how much USDC is this token redeemable for
        }

The attack outlined by the warden would require an imbalance in the price of an asset against the given above code.

It would also require oracle manipulation, which requires no external arbitrage nor intervention It would require some value to be extractable from the system

For those reasons, I think Medium Severity is more appropriate