Thunder Loan

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

Reentrancy in `flashloan()` Function

Root + Impact

Description

The `flashloan()` function has reentrancy concerns identified by Slither. The exchange rate is updated before external calls, and state variables are modified after external calls. While the current implementation may be safe due to the checks at the end, the order of operations creates potential reentrancy risks that could be exploited if combined with other vulnerabilities.


The `flashloan()` function updates state and makes external calls in an order that creates reentrancy concerns:

```solidity

function flashloan(address receiverAddress, IERC20 token, uint256 amount, bytes calldata params) external {

// ... checks ...

uint256 fee = getCalculatedFee(token, amount);

assetToken.updateExchangeRate(fee);  *// @> State updated (exchange rate)*

emit FlashLoan(receiverAddress, token, amount, fee, params);

s_currentlyFlashLoaning[token] = true; // @> State updated

assetToken.transferUnderlyingTo(receiverAddress, amount);  *// @> External call*

receiverAddress.functionCall(...);  *// @> External call (can reenter)*

uint256 endingBalance = token.balanceOf(address(assetToken));

if (endingBalance < startingBalance + fee) {

revert ThunderLoan__NotPaidBack(startingBalance + fee, endingBalance);

}

s_currentlyFlashLoaning[token] = false; // @> State updated after external calls

}

```

The function:

1. Updates exchange rate before external calls

2. Makes external calls to the receiver

3. Updates state after external calls

If the receiver can reenter `flashloan()` before the first call completes, it could potentially exploit the updated exchange rate or the `s_currentlyFlashLoaning` flag.

Risk

Likelihood:

  • * This occurs on every flash loan transaction

    * While the current checks prevent most reentrancy issues, the order of operations is not following the checks-effects-interactions pattern

    * If combined with other vulnerabilities (like the exchange rate update issues), could be exploited

Impact:

  • * Potential for reentrancy attacks if combined with other vulnerabilities

    * Exchange rate could be manipulated through reentrant calls

    * Could lead to incorrect accounting or loss of funds

    * The `s_currentlyFlashLoaning` flag might not properly prevent reentrancy in all cases

Proof of Concept

```solidity
// Scenario (theoretical, requires other vulnerabilities):
// 1. Attacker initiates flash loan
// 2. Exchange rate updated
// 3. Tokens transferred to attacker's contract
// 4. Attacker's executeOperation() reenters flashloan() before first call completes
// 5. Second flash loan sees updated exchange rate and s_currentlyFlashLoaning[token] = true
// 6. Could potentially exploit the state inconsistencies
```

Recommended Mitigation

Follow the checks-effects-interactions pattern and use reentrancy guards:
```diff
+import { ReentrancyGuardUpgradeable } from "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
-contract ThunderLoan is Initializable, OwnableUpgradeable, UUPSUpgradeable, OracleUpgradeable {
+contract ThunderLoan is Initializable, OwnableUpgradeable, UUPSUpgradeable, OracleUpgradeable, ReentrancyGuardUpgradeable {
function initialize(address tswapAddress) external initializer {
__Ownable_init();
__UUPSUpgradeable_init();
+ __ReentrancyGuard_init();
__Oracle_init(tswapAddress);
s_feePrecision = 1e18;
s_flashLoanFee = 3e15; // 0.3% ETH fee
}
- function flashloan(address receiverAddress, IERC20 token, uint256 amount, bytes calldata params) external {
+ function flashloan(address receiverAddress, IERC20 token, uint256 amount, bytes calldata params) external nonReentrant {
AssetToken assetToken = s_tokenToAssetToken[token];
uint256 startingBalance = IERC20(token).balanceOf(address(assetToken));
if (amount > startingBalance) {
revert ThunderLoan__NotEnoughTokenBalance(startingBalance, amount);
}
if (!receiverAddress.isContract()) {
revert ThunderLoan__CallerIsNotContract();
}
uint256 fee = getCalculatedFee(token, amount);
- // slither-disable-next-line reentrancy-vulnerabilities-2 reentrancy-vulnerabilities-3
- assetToken.updateExchangeRate(fee);
emit FlashLoan(receiverAddress, token, amount, fee, params);
s_currentlyFlashLoaning[token] = true;
assetToken.transferUnderlyingTo(receiverAddress, amount);
// slither-disable-next-line unused-return reentrancy-vulnerabilities-2
receiverAddress.functionCall(
abi.encodeWithSignature(
"executeOperation(address,uint256,uint256,address,bytes)",
address(token),
amount,
fee,
msg.sender,
params
)
);
uint256 endingBalance = token.balanceOf(address(assetToken));
if (endingBalance < startingBalance + fee) {
revert ThunderLoan__NotPaidBack(startingBalance + fee, endingBalance);
}
+ // Update exchange rate AFTER fee is confirmed collected
+ assetToken.updateExchangeRate(fee);
s_currentlyFlashLoaning[token] = false;
}
```
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge 16 days 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!