Description
Normal behavior
The original ThunderLoan.deposit() increases the AssetToken exchange rate by calling updateExchangeRate(calculatedFee), allowing existing LPs to earn a share of deposit fees.
Specific issue
ThunderLoanUpgraded.deposit() removes the updateExchangeRate(calculatedFee) call entirely. After the upgrade, deposits no longer contribute to LP yield via the exchange rate, even though the protocol still charges an implicit fee logic in the original design.
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);
token.safeTransferFrom(msg.sender, address(assetToken), amount);
}
function deposit(IERC20 token, uint256 amount) external ... {
uint256 calculatedFee = getCalculatedFee(token, amount);
@> assetToken.updateExchangeRate(calculatedFee);
token.safeTransferFrom(msg.sender, address(assetToken), amount);
}
Risk
Likelihood
Impact
Existing LPs lose deposit-side fee yield.
The protocol's tokenomics change silently without a documented migration plan.
Combined with F-03 storage collision, the upgrade makes the protocol strictly worse for LPs.
Proof of Concept
The PoC upgrades the proxy to ThunderLoanUpgraded, deposits tokens, and shows that the exchange rate does not increase, whereas the original implementation would have increased it.
pragma solidity 0.8.20;
import { ThunderLoan } from "src/protocol/ThunderLoan.sol";
import { ThunderLoanUpgraded } from "src/upgradedProtocol/ThunderLoanUpgraded.sol";
import { UUPSUpgradeable } from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
import { IERC20 } from "@openzeppelin/contracts/token/ERC20/IERC20.sol";
contract UpgradeRegressionTest {
ThunderLoan thunderLoan;
IERC20 token;
address attacker = address(0x1);
function _upgrade() internal {
ThunderLoanUpgraded impl = new ThunderLoanUpgraded();
vm.prank(thunderLoan.owner());
UUPSUpgradeable(address(thunderLoan)).upgradeTo(address(impl));
}
function test_H9_PoC_upgradedDeposit_skipsExchangeRateUpdate() public {
_upgrade();
ThunderLoanUpgraded upgraded = ThunderLoanUpgraded(address(thunderLoan));
uint256 rateBefore = thunderLoan.getAssetFromToken(token).getExchangeRate();
token.mint(attacker, 10e18);
vm.startPrank(attacker);
token.approve(address(upgraded), 10e18);
upgraded.deposit(token, 10e18);
vm.stopPrank();
uint256 rateAfter = thunderLoan.getAssetFromToken(token).getExchangeRate();
@> assertEq(rateAfter, rateBefore, "upgraded deposit no longer calls updateExchangeRate(calculatedFee)");
}
}
Recommended Mitigation
Either restore the deposit-side fee accrual in ThunderLoanUpgraded.deposit() or document the intentional removal with a migration plan that compensates LPs. If the fee is restored, also fix F-05 so that the fee is denominated in underlying tokens, not WETH.
// Restore with underlying-token-denominated fee
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);
// Accrue fee in underlying token units, not WETH
uint256 calculatedFee = (amount * s_flashLoanFee) / FEE_PRECISION;
assetToken.updateExchangeRate(calculatedFee);
token.safeTransferFrom(msg.sender, address(assetToken), amount);
}
