Thunder Loan
AI First Flight #7
Beginner FriendlyFoundryDeFiOracle
EXP
View results
Submission Details
Impact: high
Likelihood: medium
Invalid

Deposit updates exchange rate and mints shares before token transfer allowing state manipulation.

lovelymoon554

In a standard lending protocol, when a user deposits underlying tokens, the contract first transfers the tokens into the protocol from the user, verifies the actual balance change, and then updates internal accounting and mints corresponding shares (AssetTokens).

In ThunderLoan.sol, the deposit function violates this pattern. It updates the exchange rate, calculates the mint amount, and mints AssetTokens to the user before executing the safeTransferFrom call to pull the underlying tokens into the vault. This creates a state mismatch where internal share accounting is modified prior to actual token custody transfer.

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);
}

Risk

Likelihood:

Attackers interact directly with the deposit entry point using custom tokens or leverage multi-step transaction vectors.

State variables and share calculations process prior to the completion of external ERC20 transfers.

Impact:

  • Protocol internal state inconsistency regarding total assets versus total minted shares.

  • Vulnerability to balance/exchange-rate manipulation vectors during deposit execution phases.

Proof of Concept

Explanation: To verify this vulnerability, an auditor can inspect the order of operations in deposit(). Because assetToken.mint() is called before token.safeTransferFrom(), any callback or re-entrant hook triggered by a non-standard ERC20 token (or a malicious receiver) will observe an updated token balance state and minted shares before the underlying assets have legally arrived in the assetToken contract.

function testDepositOrderOfOperations() public {
// 1. Initialize a mock or custom ERC20 token that triggers a callback on transferFrom.
// 2. Call deposit() with the malicious token.
// 3. Inside the callback, check assetToken total supply and underlying balances.
// 4. Observe that shares are minted and exchange rates updated before asset transfer completes.
}

Recommended Mitigation:

Explanation: To resolve this issue, strictly follow the Checks-Effects-Interactions pattern. Pull the underlying tokens into the protocol via safeTransferFrom first, ensuring the contract actually holds the funds, and only then perform internal accounting updates, share minting, and exchange rate modifications.

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);
+ token.safeTransferFrom(msg.sender, address(assetToken), amount);
assetToken.mint(msg.sender, mintAmount);
uint256 calculatedFee = getCalculatedFee(token, amount);
assetToken.updateExchangeRate(calculatedFee);
- token.safeTransferFrom(msg.sender, address(assetToken), amount);
}
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 2 hours ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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

Give us feedback!