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

[H-6] 'deposit' function Re-entrancy

Dutch

Summary

In ThunderLoan.sol:85, there is a reentrancy vulnerability due to the ordering of operations in the deposit function. The function performs an ERC20 token transfer to deposit funds after minting new asset tokens, which should be the other way around to prevent reentrancy attacks.

Vulnerability Details

Vulnerable code:

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);
// @audit-issue [H-8] Reentrancy, this should be called before the mint is called
token.safeTransferFrom(msg.sender, address(assetToken), amount);
}

Impact

  1. Reentrancy can allow an attacker to repeatedly call the deposit function, enabling them to withdraw more funds than they deposited before the original transaction is completed. This could result in the depletion of the contract's funds and potentially render the contract insolvent.

Tools Used

  1. Manual Review

  2. Vs Code

Recommendations

  1. Update State Before Transfers: The contract should change its state (mint function call in this context) after performing the safeTransferFrom function to ensure funds are received before any new asset tokens are minted.

  2. Reentrancy Guard: Introduce a reentrancy guard by using a mutex or the Checks-Effects-Interactions pattern. This pattern recommends that you should make all state changes before calling external contracts.

Possible fix:
Transfer the tokens before minting any new tokens.

function deposit(IERC20 token, uint256 amount) external revertIfZero(amount) revertIfNotAllowedToken(token) {
// Transfer tokens first to prevent reentrancy attacks
token.safeTransferFrom(msg.sender, address(this), amount);
​
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);
}
Updates

Lead Judging Commences

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

Support

FAQs

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