Removing s_feePrecision in ThunderLoanUpgraded Shifts s_flashLoanFee to the Wrong Storage Slot, Setting Flash Loan Fee to 100% After Upgrade
Description
ThunderLoan declares s_feePrecision at slot 203 (value 1e18) and s_flashLoanFee at slot 204 (value 3e15). The UUPS proxy stores state in these fixed slots
regardless of which implementation is active.
ThunderLoanUpgraded removes s_feePrecision and replaces it with a constant (which consumes no storage slot). This shifts every subsequent variable one slot
earlier. After the upgrade, s_flashLoanFee in the new implementation now lives at slot 203 — where the old s_feePrecision value of 1e18 is stored — instead of
slot 204 where 3e15 lives.
// ThunderLoan.sol — original storage layout
mapping(IERC20 => AssetToken) public s_tokenToAssetToken; // slot 202
// @> slot 203 — value written by initialize(): 1e18
uint256 private s_feePrecision;
// @> slot 204 — value written by initialize(): 3e15
uint256 private s_flashLoanFee;
// ThunderLoanUpgraded.sol — upgraded storage layout
mapping(IERC20 => AssetToken) public s_tokenToAssetToken; // slot 202
// @> slot 203 — s_feePrecision is GONE; s_flashLoanFee shifts here
// @> reads proxy's slot 203 which still holds 1e18 from old s_feePrecision
uint256 private s_flashLoanFee;
// @> FEE_PRECISION is a constant — occupies NO storage slot
uint256 public constant FEE_PRECISION = 1e18;
Risk
Likelihood:
The upgrade is described as in-scope and planned — the owner will execute upgradeTo() in production
No reinitializer exists to reset s_flashLoanFee after the upgrade, so the wrong value persists immediately and permanently
Impact:
getCalculatedFee() returns a fee equal to 100% of the borrowed token's value — every flash loan costs the full borrowed amount
The protocol is immediately and permanently broken for all flash loan users post-upgrade until a corrective redeployment occurs
Proof of Concept
function testUpgradeBreaksFlashLoanFee() public {
vm.prank(thunderLoan.owner());
thunderLoan.setAllowedToken(tokenA, true);
vm.startPrank(liquidityProvider);
tokenA.mint(liquidityProvider, DEPOSIT_AMOUNT);
tokenA.approve(address(thunderLoan), DEPOSIT_AMOUNT);
thunderLoan.deposit(tokenA, DEPOSIT_AMOUNT);
vm.stopPrank();
uint256 feeBefore = thunderLoan.getCalculatedFee(tokenA, AMOUNT);
// feeBefore = 30000000000000000 (3e16, ~0.3%)
ThunderLoanUpgraded upgraded = new ThunderLoanUpgraded();
vm.prank(thunderLoan.owner());
thunderLoan.upgradeTo(address(upgraded));
ThunderLoanUpgraded upgradedProxy = ThunderLoanUpgraded(address(thunderLoan));
uint256 feeAfter = upgradedProxy.getCalculatedFee(tokenA, AMOUNT);
// feeAfter = 10000000000000000000 (1e19, 100% fee)
assertGt(feeAfter / feeBefore, 300); // ~333x bigger — PASSES
}
Output:
Fee BEFORE upgrade: 30000000000000000
Fee AFTER upgrade: 10000000000000000000
Recommended Mitigation
Preserve the slot layout by keeping a placeholder variable so s_flashLoanFee stays at slot 204:
// ThunderLoanUpgraded.sol
mapping(IERC20 => AssetToken) public s_tokenToAssetToken;
uint256 private s_feePrecision_deprecated; // slot 203 reserved — do not remove
uint256 private s_flashLoanFee; // slot 204 — matches original
uint256 public constant FEE_PRECISION = 1e18;
Lead Judging Commences
[H-01] Storage Collision during upgrade
## Description The thunderloanupgrade.sol storage layout is not compatible with the storage layout of thunderloan.sol which will cause storage collision and mismatch of variable to different data. ## Vulnerability Details Thunderloan.sol at slot 1,2 and 3 holds s_feePrecision, s_flashLoanFee and s_currentlyFlashLoaning, respectively, but the ThunderLoanUpgraded at slot 1 and 2 holds s_flashLoanFee, s_currentlyFlashLoaning respectively. the s_feePrecision from the thunderloan.sol was changed to a constant variable which will no longer be assessed from the state variable. This will cause the location at which the upgraded version will be pointing to for some significant state variables like s_flashLoanFee to be wrong because s_flashLoanFee is now pointing to the slot of the s_feePrecision in the thunderloan.sol and when this fee is used to compute the fee for flashloan it will return a fee amount greater than the intention of the developer. s_currentlyFlashLoaning might not really be affected as it is back to default when a flashloan is completed but still to be noted that the value at that slot can be cleared to be on a safer side. ## Impact 1. Fee is miscalculated for flashloan 1. users pay same amount of what they borrowed as fee ## POC 2 ``` function testFlashLoanAfterUpgrade() public setAllowedToken hasDeposits { //upgrade thunderloan upgradeThunderloan(); uint256 amountToBorrow = AMOUNT * 10; console.log("amount flashloaned", amountToBorrow); uint256 calculatedFee = thunderLoan.getCalculatedFee( tokenA, amountToBorrow ); AssetToken assetToken = thunderLoan.getAssetFromToken(tokenA); vm.startPrank(user); tokenA.mint(address(mockFlashLoanReceiver), amountToBorrow); thunderLoan.flashloan( address(mockFlashLoanReceiver), tokenA, amountToBorrow, "" ); vm.stopPrank(); console.log("feepaid", calculatedFee); assertEq(amountToBorrow, calculatedFee); } ``` Add the code above to thunderloantest.t.sol and run `forge test --mt testFlashLoanAfterUpgrade -vv` to test for the second poc ## Recommendations The team should should make sure the the fee is pointing to the correct location as intended by the developer: a suggestion recommendation is for the team to get the feeValue from the previous implementation, clear the values that will not be needed again and after upgrade reset the fee back to its previous value from the implementation. ##POC for recommendation ``` // function upgradeThunderloanFixed() internal { thunderLoanUpgraded = new ThunderLoanUpgraded(); //getting the current fee; uint fee = thunderLoan.getFee(); // clear the fee as thunderLoan.updateFlashLoanFee(0); // upgrade to the new implementation thunderLoan.upgradeTo(address(thunderLoanUpgraded)); //wrapped the abi thunderLoanUpgraded = ThunderLoanUpgraded(address(proxy)); // set the fee back to the correct value thunderLoanUpgraded.updateFlashLoanFee(fee); } function testSlotValuesFixedfterUpgrade() public setAllowedToken { AssetToken asset = thunderLoan.getAssetFromToken(tokenA); uint precision = thunderLoan.getFeePrecision(); uint fee = thunderLoan.getFee(); bool isflanshloaning = thunderLoan.isCurrentlyFlashLoaning(tokenA); /// 4 slots before upgrade console.log("????SLOTS VALUE BEFORE UPGRADE????"); console.log("slot 0 for s_tokenToAssetToken =>", address(asset)); console.log("slot 1 for s_feePrecision =>", precision); console.log("slot 2 for s_flashLoanFee =>", fee); console.log("slot 3 for s_currentlyFlashLoaning =>", isflanshloaning); //upgrade function upgradeThunderloanFixed(); //// after upgrade they are only 3 valid slot left because precision is now set to constant AssetToken assetUpgrade = thunderLoan.getAssetFromToken(tokenA); uint feeUpgrade = thunderLoan.getFee(); bool isflanshloaningUpgrade = thunderLoan.isCurrentlyFlashLoaning( tokenA ); console.log("????SLOTS VALUE After UPGRADE????"); console.log("slot 0 for s_tokenToAssetToken =>", address(assetUpgrade)); console.log("slot 1 for s_flashLoanFee =>", feeUpgrade); console.log( "slot 2 for s_currentlyFlashLoaning =>", isflanshloaningUpgrade ); assertEq(address(asset), address(assetUpgrade)); //asserting precision value before upgrade to be what fee takes after upgrades assertEq(fee, feeUpgrade); // #POC assertEq(isflanshloaning, isflanshloaningUpgrade); } ``` Add the code above to thunderloantest.t.sol and run with `forge test --mt testSlotValuesFixedfterUpgrade -vv`. it can also be tested with `testFlashLoanAfterUpgrade function` and see the fee properly calculated for flashloan
Rewards breakdown
Live
The contest is live. Earn rewards by submitting a finding.
Judging
Submissions are being reviewed by our AI judge. Results will be available in a few minutes.
View all submissionsRewards Distribution
The contest is complete and the rewards are being distributed.