Summary
When writing new versions of your contracts, either due to new features or bug fixing, there is an additional restriction to observe: you cannot change the order in which the contract state variables are declared, nor their type. Violating any of these storage layout restrictions will cause the upgraded version of the contract to have its storage values mixed up, and can lead to critical errors in your application.
Vulnerability Details
For example, in the upgraded contract, the s_flashLoanFee store is the same slot for s_feePrecision in the in the contract upgraded before.
STATE VARIABLES
mapping(IERC20 => AssetToken) public s_tokenToAssetToken;
uint256 private s_feePrecision;
uint256 private s_flashLoanFee;
STATE VARIABLES
mapping(IERC20 => AssetToken) public s_tokenToAssetToken;
uint256 private s_flashLoanFee;
Impact
The upgraded version of the contract to have its storage values mixed up, and can lead to critical errors in application.
POC
we show the value of s_flashLoanFee changed after upgraded.
pragma solidity 0.8.20;
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";
import {ThunderLoanUpgraded} from "../../src/upgradedProtocol/ThunderLoanUpgraded.sol";
contract ThunderLoanTestUpgraded 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));
}
function testUpgradeError() public{
uint256 feeUpgradedBefore = thunderLoan.getFee();
console.log(feeUpgradedBefore);
ThunderLoanUpgraded thunderLoanV2 = new ThunderLoanUpgraded();
thunderLoan.upgradeTo(address(thunderLoanV2));
uint256 feeUpgradedAfter = thunderLoan.getFee();
console.log(feeUpgradedAfter);
assertEq(feeUpgradedBefore,feeUpgradedAfter);
}
}
we run
forge test --match-contract ThunderLoanTestUpgraded -vv
we get
[FAIL. Reason: Assertion failed.] testUpgradeError() (gas: 3083993)
Logs:
3000000000000000
1000000000000000000
Error: a == b not satisfied [uint]
Left: 3000000000000000
Right: 1000000000000000000
Test result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 1.82ms
Ran 1 test suites: 0 tests passed, 1 failed, 0 skipped (1 total tests)
Tools Used
forge
Recommendations
Do not change the order in which the contract state variables are declared, nor their type when write upgraded version contract.
