First Flight #3: Thunder Loan

Summary

The exchange rate function calculates the new exchange rate as the (s_exchangeRate * ((TotalSupply() (of asset tokens or shares in the vault) + fee) / TotalSupply())). This is not the correct way to calculate the exchange rate. The reason it is not correct to just add the fee to TotalSupply() is because TotalSupply() is in terms of asset tokens or shares in the vault, and the fee is in terms of underlying tokens, so you can't just add them.

The shares received by a depositor should be calculated as (assets deposited * TotalSupply)/(TotalAssets + 1). (TotalAssets is the amount of underlying token in the vault. +1 is to avoid dividing by 0 the first deposit.) Then the exchange rate = shares received / assets deposited.

But, for purposes of deposits and redemptions, you don't even need to calculate an exchange rate - as further explained below, it is more straightforward (and less can go wrong) to just calculate the asset token shares to be received upon a deposit or the underlying tokens to be received upon a redemption without tracking the exchange rate. You can do this based on just the amount of tokens/asset tokens of the deposit/redemptions and the amount of underlying tokens and asset tokens in AssetToken.sol.

If you want to calculate an exchange rate for purposes of invariant testing, you can still do that - you just don't need it for deposits and redemptions.

Vulnerability Details

Impact

The exchange rate is not calculated correctly. People will not receive the right amount of underlying tokens and asset tokens when they deposit and redeem.

Tools Used

Manual review

Recommendations

You don't need to calculate the exchange rate at all for purposes of deposits and redemptions. Instead you can do the following to just calculate the amount of shares to be received when a depositor puts tokens in and the amount of tokens to get back when the depositor redeems. Make the following changes:

Add a TotalAssets function to AssetToken.sol:

Add a function for shares to be received by depositors to AssetToken.sol - this will be used in the deposit function instead of the exchange rate:

Add a function for underlying tokens to be returned upon redemption to AssetToken.sol - this will be used in redeem instead of the exchange rate:

Then, in ThunderLoan.sol, you would change the deposit and redeem functions to remove the exchange rate function and call assetToken.sharesToBeReceived(amount) and assetToken.tokensToBeReceived(amountofAssetToken) instead:

You also don't need to call updateExchangeRate() in the flashloan function. The reason is that the flashloan function (via calling repay) sends the fees, consisting of underlying tokens, to AssetToken.sol. Therefore, they become part of TotalAssets() and will be picked up by the sharesToBeReceived() and tokensToBeReceived() calculations by virtue of being in AssetToken.sol.