Submission Details
Severity:
Doing updateExchangeRate in deposit function will cause the protocol have to pay more amount to the depositers
Summary
The ThunderLoan contract assume that if the contract gets the fee from the FlashLoan action, The fee will share to all liquidity providers. In deposit function, there is no fee get yet, so it is error for Doing updateExchangeRate action.
Vulnerability Details
uint256 calculatedFee = getCalculatedFee(token, amount);
assetToken.updateExchangeRate(calculatedFee);
The updateExchangeRate action in deposit function will cause s_exchangeRate getting bigger and bigger. Then depositers will get more asset than they deposit in the protocol, even though there is no FlashLoan action.
function updateExchangeRate(uint256 fee) external onlyThunderLoan {
// 1. Get the current exchange rate
// 2. How big the fee is should be divided by the total supply
// 3. So if the fee is 1e18, and the total supply is 2e18, the exchange rate be multiplied by 1.5
// if the fee is 0.5 ETH, and the total supply is 4, the exchange rate should be multiplied by 1.125
// it should always go up, never down
// newExchangeRate = oldExchangeRate * (totalSupply + fee) / totalSupply
// newExchangeRate = 1 (4 + 0.5) / 4
// newExchangeRate = 1.125
uint256 newExchangeRate = s_exchangeRate * (totalSupply() + fee) / totalSupply();
if (newExchangeRate <= s_exchangeRate) {
revert AssetToken__ExhangeRateCanOnlyIncrease(s_exchangeRate, newExchangeRate);
}
s_exchangeRate = newExchangeRate;
emit ExchangeRateUpdated(s_exchangeRate);
}
Impact
The depositers can get more assets if they redeem firstly. Some of depositers can not get their asserts back forever.
POC
// SPDX-License-Identifier: MIT
import { Test, console } from "forge-std/Test.sol";
import { BaseTest, ThunderLoan } from "./BaseTest.t.sol";
import { AssetToken } from "../../src/protocol/AssetToken.sol";
import { MockFlashLoanReceiver } from "../mocks/MockFlashLoanReceiver.sol";
contract ThunderLoanTestBugs is BaseTest {
uint256 constant AMOUNT = 10e18;
uint256 constant DEPOSIT_AMOUNT = AMOUNT * 100;
address liquidityProvider = address(123);
address liquidityProvider2 = address(124);
address user = address(456);
MockFlashLoanReceiver mockFlashLoanReceiver;
function setUp() public override {
super.setUp();
vm.prank(user);
mockFlashLoanReceiver = new MockFlashLoanReceiver(address(thunderLoan));
}
modifier setAllowedToken() {
vm.prank(thunderLoan.owner());
thunderLoan.setAllowedToken(tokenA, true);
_;
}
function testDepositMintsAssetAndRedeem() public setAllowedToken {
tokenA.mint(liquidityProvider, AMOUNT);
vm.startPrank(liquidityProvider);
tokenA.approve(address(thunderLoan), AMOUNT);
thunderLoan.deposit(tokenA, AMOUNT);
vm.expectRevert(bytes("ERC20: transfer amount exceeds balance"));
thunderLoan.redeem(tokenA, AMOUNT);
vm.stopPrank();
}
function testTwoUsersDepositMintsAssetAndRedeem() public setAllowedToken {
tokenA.mint(liquidityProvider, AMOUNT);
tokenA.mint(liquidityProvider2, AMOUNT);
vm.startPrank(liquidityProvider);
tokenA.approve(address(thunderLoan), AMOUNT);
thunderLoan.deposit(tokenA, AMOUNT);
vm.stopPrank();
vm.startPrank(liquidityProvider2);
tokenA.approve(address(thunderLoan), AMOUNT);
thunderLoan.deposit(tokenA, AMOUNT);
vm.stopPrank();
vm.startPrank(liquidityProvider);
thunderLoan.redeem(tokenA, AMOUNT);
vm.stopPrank();
AssetToken asset = thunderLoan.getAssetFromToken(tokenA);
uint256 exchangeRate = asset.getExchangeRate();
uint256 moreAMOUNT = (AMOUNT * exchangeRate) / asset.EXCHANGE_RATE_PRECISION();
//liquidityProvider get more tokenA
assertGt(moreAMOUNT, AMOUNT);
assertEq(tokenA.balanceOf(liquidityProvider), moreAMOUNT);
assertEq(asset.balanceOf(liquidityProvider), 0);
//there is less than AMOUNT tokenA for liquidityProvider2 to getback
assertEq(tokenA.balanceOf(address(asset)), 2*AMOUNT-moreAMOUNT);
assertLt(tokenA.balanceOf(address(asset)), AMOUNT);
}
modifier hasDeposits() {
vm.startPrank(liquidityProvider);
tokenA.mint(liquidityProvider, DEPOSIT_AMOUNT);
tokenA.approve(address(thunderLoan), DEPOSIT_AMOUNT);
thunderLoan.deposit(tokenA, DEPOSIT_AMOUNT);
vm.stopPrank();
_;
}
}
Run the test:
forge test --match-contract ThunderLoanTestBugs
Running 2 tests for test/unit/ThunderLoanTestBugs.t.sol:ThunderLoanTestBugs
[PASS] testDepositMintsAssetAndRedeem() (gas: 1163074)
[PASS] testTwoUsersDepositMintsAssetAndRedeem() (gas: 1230156)
Test result: ok. 2 passed; 0 failed; 0 skipped; finished in 3.85ms
Ran 1 test suites: 2 tests passed, 0 failed, 0 skipped (2 total tests)
Tools Used
forge
Recommendations
delete the updateExchangeRate action in deposit function
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 over 1 year ago
Submission Judgement Published Assigned finding tags:
can't redeem because of the update exchange rate
Rewards breakdown
nSLOC
387This contest has a flat reward structure with the following payouts:
100 EXP
20 EXP
2 EXP
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.