Sudden increases in OsToken minting makes the APY go out of sync with protocol state

[hats-bug-reporter[bot]]

Github username: @akshaysrivastav Submission hash (on-chain): 0xc6425cdc35a38c7403fa5ad340244d374d02e7399cca687fe470e7d91123d698 Severity: high

Description:

Description

In VaultOsToken._redeemOsToken the OsToken debt of users is determined by the cumulativeFeePerShare value ratio.

_syncPositionFee():

                                            ⎛     cumulativeFeePerShare    ⎞
        position.shares = position.shares ⋅ ⎜──────────────────────────────⎟
                                            ⎝position.cumulativeFeePerShare⎠

In OsToken, the cumulativeFeePerShare is increased by the ratio of treasuryShares to _totalShares.

    _cumulativeFeePerShare += SafeCast.toUint192(Math.mulDiv(treasuryShares, _wad, totalShares));

In a scenario where large number of OsTokens gets minted just after the avgRewardPerSecond change, the sudden minting of OsTokens will rapidly increase the _cumulativeFeePerShare value. As the avgRewardPerSecond will still be pointing to the old apy value but the new _totalAssets amount will be much larger, the profit amount Math.mulDiv(avgRewardPerSecond * _totalAssets, timeElapsed, _wad) will also increase rapidly and incorrectly.

Moreover, the KeeperRewards contract won't be able to correct the avgRewardPerSecond due to the 12 hours rewardsDelay.

In this scenario, the debt value of users will grow rapidly in a matter of hours. Users who were sitting on a healthy LTV of 80% few hours ago will now be at the verge of liquidation. If their LTV crosses the 92% threshold they will get liquidated.

It should be noted that this scenario can become a reality due to:

• Sudden but natural market movement. New/existing users start minting OsTokens. In the past bull cycle many projects have seen multifold TVL growth overnight, hence this is not a rare scenario.
• An attacker artificially inflates the _totalAssets value by minting OsTokens repeatedly. As long as the profit amount avgRewardPerSecond * _totalAssets * timeElapsed comes out to be a big value, this attack scenario will work.

The core issue arises because there is a delay of 12 hours between two KeeperRewards.updateRewards txns and in that interval the totalAssets (and profits) of OsToken can grow significantly. Due to which the avgRewardPerSecond will go out of sync wrt the current OsToken contract state, which in end leads to rapid increase in user's LTV ratios and thus rapid liquidations.

Attack Scenario

• Initially X amount of OsTokens are minted.
• According to the state of validators the oracles set 15% as the avgRewardPerSecond value.
• After the avgRewardPerSecond is updated, sufficiently large number of OsTokens gets minted. Let's assume 10X.
• Now the contract will consider the apy of 15% on 10X OsTokens.
• This will result in large amount of profits in _unclaimedAssets function, rapid increase in _cumulativeFeePerShare, hence rapid increase in user's position.share value.
• Likely the LTV of user's will cross the 92% liquidation threshold.
• Then
  • If the scenario occured naturally then anyone can liquidate the users.
  • If the scenario was artificially created by an attacker, the attacker will perform liquidations and generate profits. After the attack he can simply burn the minted OsTokens and leave the system.

It should be noted that all of this occured within a span of few hours (< 12 hours). So most users won't get the chance to make their positions healthy and prevent liquidations.

Mitigation

Instead of using fixed intervals for updating avgRewardPerSecond, consider implementing a threshold based update mechanism. Eg, as soon as the avgRewardPerSecond changes by 0.5% the KeeperRewards.updateRewards can be invoked.

[tsudmi]

During every avgRewardPerSecond update the _unclaimedAssets is synced and timestamp is updated, so right after settings apy to 15%, the _unclaimedAssets will be 0 as timeElapsed becomes 0. Oracles can't set avgRewardPerSecond to very high value as it's limited with _maxAvgRewardPerSecond in KeeperRewards contract

[akshaysrivastav]

Hey @tsudmi

I am aware that timeElapsed becomes 0 after reward update. But the issue is not limited to the next instant after the reward update, instead the issue spans over 12 hours (reward delay period).

To state it again: The core issue arises because there is a delay of 12 hours between two KeeperRewards.updateRewards txns and in that interval the totalAssets (and profits) of OsToken can grow significantly. Due to which the avgRewardPerSecond will go out of sync wrt the current OsToken contract state, which in end leads to rapid increase in user's LTV ratios and thus rapid liquidations.

In short, as long as avgRewardPerSecond * _totalAssets * timeElapsed turns out to be a non zero value the reported issue is realised.

Also the maxAvgRewardPerSecond is 20% so a 15% apy is well under the apy limit.

[tsudmi]

Hey @akshaysrivastav

Lets assume I have 10 ETH staked and I've minted 9 ETH worth of osETH. Lets also assume that APY of my vault is 0% and APY of osETH is 15%. My current LTV is 90% (9 / 10 100) and liquidation happens with LTV equal to 92%. In 24 hours I will accumulate fee equal to ~0.0037 ETH ( 9 0.15 / 365) and my LTV will become 90.03% (9.0037 / 10 * 100). As you can see after 1 day the position is very far from the liquidation.