Beginner FriendlyFoundryDeFiOracle
100 EXP
View results
Submission Details
Severity: medium
Invalid

The updateExchangeRate function should not revert if the new exchange rate is equal to the prior exchange rate

Summary

It is not correct that the exchange rate should always go up. What should always go up (or at least stay flat) is the value of each share of the vault - or, to put it another way, the value of each interest-bearing token held by a depositor. This value will stay flat if no flash loans are made and will increase each time a flash loan is successfully completed. But that is different from the exchange rate, which represents how many interest-bearing tokens you get for each underlying token you deposit (and vice versa).

But updateExchangeRate reverts if the new exchange rate after a deposit or flash loan is equal to the old exchange rate. It shouldn't do this. If there has been no new flash loans taken out between deposits the exchange rate stays the same.

Vulnerability Details

updateExchangeRate reverts if the new exchange rate is equal to the existing exchange rate:

if (newExchangeRate <= s_exchangeRate) {
revert AssetToken__ExchangeRateCanOnlyIncrease(
s_exchangeRate,
newExchangeRate
);

Impact

This function may revert when it shouldn't. The calculation of the exchange rate in this function is also incorrect. Because there is also an error in the deposit function (it erroneously adds fees that didn't accrue), the deposit function will never revert due to failing this check but the check still isn't right. If one person deposits and then no flash loans are made and then the next person deposits, the second depositor will get the same rate as the first depositor (if the exchange rate is calculated correctly).

Tools Used

Manual review

Recommendations

Remove this check from the update exchange rate calculation. You could instead add a post check to the flashloan function that the exchange rate increased as a result of the flash loan (see changes below) - flash loaning is what is supposed to increase the exchange rate, so that is where you should check. However this isn't strictly necessary because the flash loan function already checks that the balance of underlying tokens in AssetToken.sol increased by the amount of the fee, which will have the impact of increasing the exchange rate, so you could argue the exchange rate post check is unnecessary.

If you want to add a post check to the flashloan function, here are the changes you could make:

At the beginning of the flash loan function add:

uint256 startingExchangeRate = assetToken.getExchangeRate();

At the end of the flashloan function, add:

if (assetToken.getExchangeRate() <= startingExchangeRate) {
revert ThunderLoan__ExchangeRateDidNotIncrease();}
Updates

Lead Judging Commences

0xnevi Lead Judge
over 1 year ago
0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.