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

precision mismatch between liquidity token to and from AssetToken and fee determination leads to loss of funds for both the liquidity providers and wrong fee determination.

0xspryon

Summary

Wrong precision scaling on ThunderLoan::deposit::mintAmount & ThunderLoanUpgraded::deposit::mintAmount & ThunderLoan::deposit::redeem & ThunderLoanUpgraded::deposit::redeem & AssetToken::updateExchangeRate because we're using both the liquidity token and the asset token and the fee and the totalSupply as though they had the same decimals value which isn't forcibly the case for all erc20 tokens on ethereum.

This problem of lack of precision scaling can be found at multiple locations in the codebase.

  1. ThunderLoan::deposit::mintAmount when converting from liquidity token to asset token

  2. ThunderLoanUpgraded::deposit::mintAmount when converting from liquidity token to asset token

  3. ThunderLoan::redeem::amountUnderlying when converting from asset token to liquidity token

  4. ThunderLoanUpgraded::redeem::amountUnderlying when converting from asset token to liquidity token

  5. AssetToken::updateExchangeRate::fee when determining the new exchange rate. The fee being in a different precision is added onto the totalSupply as though they both were of the same decimal precision. Which they aren't for certain tokens like say USDC.

Vulnerability Details

For ThunderLoan::deposit/ThunderLoanUpgraded::deposit

mintAmount, the amount of assetToken to be minted for the amount of liquidity token deposited is derived from the expression

uint256 mintAmount = (amount * assetToken.EXCHANGE_RATE_PRECISION()) / exchangeRate

The exaplanation on AssetToken::s_exchangeRate says that, s_exchangeRate is the underlying per asset exchange rate.
ie: s_exchangeRate = 2
means 1 asset token is worth 2 underlying tokens.

Using the initial values of both:
AssetToken::s_exchangeRate = 1e18 and AssetToken::EXCHANGE_RATE_PRECISION = 1e18
this effectively means that, if we deposit 1 unit of liquidity token ( ie amount = 1 ) we expect to be minted 1 unit of assetToken. There exist ERC20 tokens with different decimal values. For example:
USDT on eth having a decimal value of 6
USDC on eth having a decimal value of 6
ZIL on eth having a decimal value of 12

Using one such token as the liquidity token means that, the mintAmount calculated above will be wrong as can be seen in the below calculatioon.

Expection

using the above s_exchangeRate and EXCHANGE_RATE_PRECISION values. If I deposit 1 USDC ( decimals = 1e6 ), then I expect to receive 1 assetToken (decimals = 1e18)

reality

uint256 mintAmount = (1e6 * 1e18)/1e18;
ergo,
uint256 mintAmount = 1e6; // < 1e18
We can clearly see that, the LP will be minted less tokens than expected.

POC

test/unit/ThunderLoanTest.t.sol:testDepositMintsAssetAndUpdatesBalance clearly shows us that, the LP is minted the same numerical value of liquidity he provided regardless of the decimals of the liquidity token he's providing. Herein lies the problem.

For ThunderLoan::redeem/ThunderLoanUpgraded::redeem

Applying a logic similar to the above for the redeem functions, we notice that, amountUnderlying has a precision of 18 decimals but it's being transfered on the liquidity token as though the liquidity token has 18 decimals of precision as well. Which we've already established, it doesn't always have 18 decimals of precision. This effectively means that, if the liquidity token is say USDC, we'll be sending the LP more tokens than he initially wanted to redeem and at the expense of the other liquidity providers.

For AssetToken::updateExchangeRate::fee

Applying a logic similar to the above for the updateExchangeRate, we notice that, newExchangeRate is smaller or greater than should have been because we're taking fee which can of a precision greater than or lesser than that of AssetToken and adding to the totalSupply() of AssetToken as though they both have the same precisions. Using USDC as our liquidity token, then newExchangeRate will be less that what is expected.

Impact

For tokens with a precision greater than that of assetToken
LP is minted more assetToken than expected for the amount of liquidity he deposits.
During redemption, LP is provided with lesser liquidity tokens than was supposed to.
During updateExchangeRate, the fee is greater than expected

For tokens with a precision lower than that of assetToken
LP is minted less assetToken than expected for the amount of liquidity he deposits.
During redemption, LP is provided with more liquidity tokens than was supposed to.
During updateExchangeRate, the fee is lesser than expected

Tools Used

Manual review

Recommendations

We should scale the precision of mintAmount to that of the assetToken before doing the minting.

ThunderLoan/ThunderLoanUpgraded::redeem
function redeem(
IERC20 token,
uint256 amountOfAssetToken
)
external
revertIfZero(amountOfAssetToken)
revertIfNotAllowedToken(token)
{
AssetToken assetToken = s_tokenToAssetToken[token];
uint256 exchangeRate = assetToken.getExchangeRate();
if (amountOfAssetToken == type(uint256).max) {
amountOfAssetToken = assetToken.balanceOf(msg.sender);
}
- uint256 amountUnderlying = (amountOfAssetToken * exchangeRate) / assetToken.EXCHANGE_RATE_PRECISION();
+ uint256 scaledAmountUnderlying = amountUnderlying * token.decimals() / assetToken.decimals();
emit Redeemed(msg.sender, token, amountOfAssetToken, amountUnderlying);
assetToken.burn(msg.sender, amountOfAssetToken);
- assetToken.transferUnderlyingTo(msg.sender, amountUnderlying);
+ assetToken.transferUnderlyingTo(msg.sender, scaledAmountUnderlying);
}
ThunderLoan/ThunderLoan::deposit
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;
+ uint256 mintAmount = (amount * 10**assetToken.decimals() * assetToken.EXCHANGE_RATE_PRECISION()) / (exchangeRate * 10**token.decimals());
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);
}
`test/unit/ThunderLoan.t.sol::testDepositMintsAssetAndUpdatesBalance 
This test assumes `s_precisionFee == s_exchangeRate == 1e18`
function testDepositMintsAssetAndUpdatesBalance() public setAllowedToken {
tokenA.mint(liquidityProvider, AMOUNT);
​
vm.startPrank(liquidityProvider);
tokenA.approve(address(thunderLoan), AMOUNT);
thunderLoan.deposit(tokenA, AMOUNT);
vm.stopPrank();
​
AssetToken asset = thunderLoan.getAssetFromToken(tokenA);
+
+ uint mintAmount = (AMOUNT * 10**(asset.decimals() - 1)) / 10**(tokenA.decimals() - 1);
+
assertEq(tokenA.balanceOf(address(asset)), AMOUNT);
- assertEq(asset.balanceOf(liquidityProvider), AMOUNT);
+ assertEq(asset.balanceOf(liquidityProvider), mintAmount);
}
AssetToken::updateExchangeRate
- function updateExchangeRate(uint256 fee) external onlyThunderLoan {
+ function updateExchangeRate(uint256 fee, uint256 feePrecision) external onlyThunderLoan {
// 1. Get the current exchange rate
// 2. How big the fee is should be divided by the total supply
// 3. So if the fee is 1e18, and the total supply is 2e18, the exchange rate be multiplied by 1.5
// if the fee is 0.5 ETH, and the total supply is 4, the exchange rate should be multiplied by 1.125
// it should always go up, never down
// newExchangeRate = oldExchangeRate * (totalSupply + fee) / totalSupply
// newExchangeRate = 1 (4 + 0.5) / 4
// newExchangeRate = 1.125
+ uint scaledFee = fee * decimals() / feePrecision;
- uint256 newExchangeRate = s_exchangeRate * (totalSupply() + fee) / totalSupply();
+ uint256 newExchangeRate = s_exchangeRate * (totalSupply() + scaledFee) / totalSupply();
​
if (newExchangeRate <= s_exchangeRate) {
revert AssetToken__ExhangeRateCanOnlyIncrease(s_exchangeRate, newExchangeRate);
}
s_exchangeRate = newExchangeRate;
emit ExchangeRateUpdated(s_exchangeRate);
}
Updates

Lead Judging Commences

0xnevi Lead Judge
almost 2 years ago
0xnevi Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Other

Support

FAQs

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