code-423n4 / 2022-03-volt-findings

0 stars 0 forks source link

NonCustodialPSM can become insolvent as CPI index rises #83

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-03-volt/blob/main/contracts/peg/NonCustodialPSM.sol#L236-L248

Vulnerability details

Impact

NonCustodialPSM mints and redeems VOLT to a chosen stablecoin at the current market rate minus a fixed fee. It is assumed that the difference to be covered with pcvDeposit funds. That assumption is similar to one used in FEI protocol, but there no rate growth takes place as FEI to USD rate supposed to be stable, while VOLT to USD rate will rise over time.

VOLT market rate is tied to the off-chain published CPI index. The growth of this index can easily surpass the yield of the pcvDeposit used, so its interest cannot be guaranteed to be always greater than CPI index advancement. The contract can end up in the situation when no redeem be possible, i.e. NonCustodialPSM can become insolvent.

For example, let's say the stablecoin is USDC, and now investors are worried about inflation and buy/mint 100m VOLT for 100m USDC. Fast forward 1 year, and investors were generally right, as due to rise of the oil prices happening simultaneously with logistics issues the CPI index growth end up being 30% APR for the year.

Then, inflation fears abated and, say, stocks become stronger, and investors want their funds now to put them there and sell/redeem 100m VOLT expecting 125m USDC in return (for simplicity say 5m USDC goes to mint and redeem fees combined). USDC deposit strategy used in pcvDeposit yielded 10% APR for the year. The contract cannot redeem all the funds due as it is 125 - 100 * 1.1 = 15m USDC short.

Putting severity to high as the contract serves requests sequentially and the last investors' funds are lost this way, i.e. in the example above all the users, who came in to redeem when contract has 15m USDC in obligations and no funds, will lose their entire deposits.

Proof of Concept

Continuing the example, current low risk USDC deposit rates are circa 2.5 lower than US CPI:

AAVE: https://classic.aave.com/#/markets

Compound: https://compound.finance/markets/USDC

US CPI: https://www.bls.gov/cpi/

NonCustodialPSM.redeem uses current oracle price to determine what amount of stablecoins to be paid for 1 VOLT:

https://github.com/code-423n4/2022-03-volt/blob/main/contracts/peg/NonCustodialPSM.sol#L236

https://github.com/code-423n4/2022-03-volt/blob/main/contracts/peg/NonCustodialPSM.sol#L378-L390

NonCustodialPSM.mint does the same:

https://github.com/code-423n4/2022-03-volt/blob/main/contracts/peg/NonCustodialPSM.sol#L274

https://github.com/code-423n4/2022-03-volt/blob/main/contracts/peg/NonCustodialPSM.sol#L357-L365

For example, FEI protocol use a wide range of pcvDeposits, whose yields vary depending on the underlying strategy:

https://github.com/fei-protocol/fei-protocol-core/blob/develop/protocol-configuration/mainnetAddresses.ts#L164-L568

But there are no PCV deposits whose returns are linked to CPI of any country, so mismatch (basis) risk exists, which has to be addressed.

Recommended Mitigation Steps

Consider providing a way to directly inject funds from a separately held stability fund (possibly shared across all the stablecoins and corresponding pcvDeposits) in addition to pcvDeposit as its strategy alone cannot guarantee the returns needed.

Ideally, the redeem and mint fees collected should go to this stability fund as well, with the possibility to retrieve them when there is a total surplus big enough.

More important, consider limiting the redeem amount to total user's share of the pcvDeposit and its stability fund part, so the deficit be visible and shared between all the users. A user can then choose either to withdraw now, obtaining less than CPI index due to current liquidity situation, or to wait for stability fund to be filled up or for pcvDeposit yield to catch up. This way no user will lose the whole deposit.

ElliotFriedman commented 2 years ago

There is a risk of the system not being able to make everyone whole if the PCV grows too rapidly and onchain yields cannot keep up with or outcompete inflation, however this is not a code issue, so marking as invalid.

jack-the-pug commented 2 years ago

This is a valid concern. The warden explained why it's possible to happen and how the current implementation handled it improperly, which can make some of the users suffer an unexpected and unfair loss.

The recommended fix is reasonable.

I'll keep this as a High.

ElliotFriedman commented 2 years ago

There is a 10m FEI liquidity backstop provided by the Tribe DAO, so the system would need to be live for many years without earning any yield to finally become insolvent.

jack-the-pug commented 2 years ago

With the new information provided by the sponsor, I now agrees that the likelihood of the situation described by the warden is low. And therefore, I'm downgrading the issue from High to Medium.