First Flight #3: Thunder Loan
First Flight #3
Beginner FriendlyFoundryDeFiOracle
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

The deposit function is incorrectly calculating the amount of asset tokens a depositor receives

happyformerlawyer

Summary

The deposit function calls getCalculatedFee on the depositor's inputs of token and amount and then calls assetToken.getExchangeRate(calculatedFee). This is acting as if the depositor is being charged a fee that is then accruing to assetToken, but in fact the only people who are supposed to be charged fees are the people taking flash loans. The depositor does not pay a fee. Therefore, this is throwing off the calculation of the exchange rate.

Vulnerability Details

The deposit function calculates a fee using the depositor's inputs and then updates the exchange rate using that calculated fee:

function deposit(
IERC20 token,
uint256 amount
) external revertIfZero(amount) revertIfNotAllowedToken(token) {
AssetToken assetToken = s_tokenToAssetToken[token];
uint256 exchangeRate = assetToken.getExchangeRate();
uint256 mintAmount = (amount * assetToken.EXCHANGE_RATE_PRECISION()) /
exchangeRate;
emit Deposit(msg.sender, token, amount);
assetToken.mint(msg.sender, mintAmount);
uint256 calculatedFee = getCalculatedFee(token, amount);
assetToken.updateExchangeRate(calculatedFee);
token.safeTransferFrom(msg.sender, address(assetToken), amount);
}

Impact

This incorrectly drives up the exchange rate by acting as if fees are earned from deposits when they are not. This can result in depositors getting more or less tokens than they should when they deposit and redeem.

Tools Used

Manual review

Test

I wrote this test to show that the exchange rate will increase upon a deposit even if no flash loans have been made, which makes no sense because no extra value is accruing to each asset token share. Because the exchange rate is driven artificially high, it can mean that future depositors/redeemers can end up with more or less tokens than they deserve.

function testDepositIncreasesExchangeRateEvenWithoutFlashLoans()
public
setAllowedToken
{
AssetToken asset = thunderLoan.getAssetFromToken(tokenA);
uint256 initialExchangeRate = asset.getExchangeRate();
console.log("exchange rate before:", initialExchangeRate);
vm.startPrank(liquidityProvider);
tokenA.mint(liquidityProvider, DEPOSIT_AMOUNT);
tokenA.approve(address(thunderLoan), DEPOSIT_AMOUNT);
thunderLoan.deposit(tokenA, DEPOSIT_AMOUNT);
vm.stopPrank();
uint256 currentExchangeRate = asset.getExchangeRate();
console.log("exchange rate after:", currentExchangeRate);
assertEq(initialExchangeRate, currentExchangeRate);
}

Recommendations

The deposit function shouldn't call getCalculatedFee, so I recommend removing the line. Also, I recommend removing the call to updateExchangeRate as well. I explained in another finding why I recommend not using the exchange rate concept for purposes of calculating how many tokens/asset tokens depositors get for deposits and redemptions.

Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

can't redeem because of the update exchange rate

Support

FAQs

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