code-423n4 / 2024-02-spectra-findings

4 stars 2 forks source link

The post-expiration price rate can be frontran/sandwiched, permanently setting manipulated price #252

Closed c4-bot-8 closed 8 months ago

c4-bot-8 commented 9 months ago

Lines of code

https://github.com/code-423n4/2024-02-spectra/blob/main/src/tokens/PrincipalToken.sol#L409 https://github.com/code-423n4/2024-02-spectra/blob/main/src/tokens/PrincipalToken.sol#L901

Vulnerability details

Impact

The first call to storeRatesAtExpiry after expiration sets the rate of ptRate, ibtRate permanently. However, the vault price at this point can be manipulated e.g. with a sandwich attack, by providing a lot of underlying liquidity right before the update, and withdrawing it right after, however, the rate in the Spectra protocol e.g. for ibt will remain like this forever after the expiration period. This will freeze the price at unreal level, allowing attacker to either get discount for future redeems, or in a reverse case slightly increasing the rate, causing other users to get unfavourable redeems onwards.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

Following sequence of attack is possible:

uint256 currentIBTRate = IERC4626(ibt).previewRedeem(ibtUnit).toRay(_assetDecimals);

will return less value (due to vault having much fewer assets inside) than normally. currentIBTRate wil be then permanently lowered

shares = _withdrawShares(ibts, receiver, owner, _ptRate, _ibtRate);

and in _withdrawShares

shares = _ibts.mulDiv(_ibtRate, _ptRate, Math.Rounding.Ceil);

which translates to:

shares = (_ibts * _ibtRate) / _ptRate

The formula shows that attacker can profit off permanently increased _ibtRate, by increasing the numerator and giving him more shares than he should receive. The final result is, the last user to withdraw/redeem might not be able to do so due to lack of liquidity - the last user(s) to redeem withdraw will be then those who lose funds.

Tools Used

Manual approach.

Recommended Mitigation Steps

It might still be good idea to use underlying vault for calculating the price after expiration. Otherwise, use some TWAP-like rates accumulated over time, but ultimately do not rely on one-time price update to set the permanent price.

Assessed type

Timing

c4-pre-sort commented 9 months ago

gzeon-c4 marked the issue as primary issue

c4-pre-sort commented 9 months ago

gzeon-c4 marked the issue as sufficient quality report

gzeon-c4 commented 9 months ago

does not seems possible to manipulate, the sandwich would be depositing and withdrawing at the same rate

c4-sponsor commented 9 months ago

yanisepfl (sponsor) disputed

yanisepfl commented 9 months ago

This requires the IBT rate to be easily manipulated using deposits and withdraws. It is up to the users to be careful where to put their funds into.

Additionally, when rates are stored, users are all allocated an IBT balance to them, which they are able to withdraw/redeem and claim. Hence, if right before expiry the attacker somehow increased the IBT rate by depositing assets in the IBT vault, it would mean for the other users that they earned a portion of the attacker's funds. Now, let us imagine the users did the opposite and somehow managed to decrease the IBT worth in asset before expiry, then the impact for our users that hold PT is the same as for the IBT holders. And as already stated, it is up to the users to carefully investigate the IBTs they put their funds into.

We therefore dispute this issue.

c4-judge commented 8 months ago

JustDravee marked the issue as unsatisfactory: Invalid