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

4 stars 2 forks source link

Delayed Invocation of storeRatesAtExpiry Affects View Functions #103

Closed c4-bot-4 closed 8 months ago

c4-bot-4 commented 8 months ago

Lines of code

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

Vulnerability details

Impact

The PrincipalToken contract is designed to manage financial instruments that have a maturity date (expiry). At this point, the rates of the Principal Token (PT) and the Interest Bearing Token (IBT) should be fixed and no longer subject to market fluctuations. These rates are crucial for calculating the value of the PT and the yield that users are entitled to receive.

View functions in the contract, such as previewRedeem or maxWithdraw, provide users with information about the amount of underlying assets or IBTs they can receive for their PT shares. These calculations must use the correct rates to ensure accuracy.

However, if the storeRatesAtExpiry function is not invoked after the PT expires, the contract does not have the final rates stored and continues to use the last known market rates for these calculations. Since the market rates can fluctuate until they are fixed at expiry, any view functions called after expiry but before storeRatesAtExpiry is invoked may return values based on potentially outdated rates.

Affected Functions:

previewRedeem previewRedeemForIBT maxRedeem maxWithdraw maxWithdrawIBT convertToPrincipal convertToUnderlying totalAssets

Impact:

This can lead to inaccurate financial reporting on the user interface and potentially incorrect decision-making by users or external dependent contracts/systems.

Tools Used

Manual Review

Recommended Mitigation Steps

Implement an automated mechanism to call storeRatesAtExpiry at the time of expiry, or introduce additional checks within view functions to ensure they account for whether rates at expiry have been stored. Alternatively, provide clear user interface prompts or documentation to encourage users to call storeRatesAtExpiry as needed.

Assessed type

Other

c4-pre-sort commented 8 months ago

gzeon-c4 marked the issue as primary issue

c4-pre-sort commented 8 months ago

gzeon-c4 marked the issue as sufficient quality report

gzeon-c4 commented 8 months ago

low severity and would cost more gas

c4-sponsor commented 8 months ago

yanisepfl (sponsor) disputed

yanisepfl commented 8 months ago

In such a scenario, the view functions would use the current rates, which are the ones that would be stored and used if non-view functions were called. Hence everything is as expected there.

This issue is thus wrong and disputed.

c4-judge commented 8 months ago

JustDravee marked the issue as unsatisfactory: Invalid