First Flight #3: Thunder Loan
First Flight #3
Beginner FriendlyFoundryDeFiOracle
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

Storage layout mismatch between `ThunderLoan` and `ThunderLoanUpgraded`

ciara

The storage slots of the flash loan fee and the fee precision have switched storage slots between Thunderloan and ThunderLoanUpgraded. This leads to the values being read and written to storage incorrectly.

Vulnerability details

The storage layout is stored in the proxy contract and cannot be changed once the proxy has been deployed. When the implementation is upgraded from ThunderLoan to ThunderLoanUpgraded, the storage variables need to map to the correct, initially set, storage slots or there is a mismatch between the storage layout and the storage variables.

In the ThunderLoan contract on line 96-97, the storage slots of the fee precision (s_feePrecisison) and the flash loan fee (s_flashLoanFee) are declared in storage:

uint256 private s_feePrecision;
uint256 private s_flashLoanFee;

Whereas, in the ThunderLoanUpgraded contract, on line 96-97, the storage variables are stated in the reverse:

uint256 private s_flashLoanFee;
uint256 public constant FEE_PRECISION = 1e18;

These variables are now pointing to the reverse storage slots. Hence, when FEE_PRECISION is modified or read, the flash loan fee storage is what is modified or read, and visa versa.

Impact

Since the flash loan fee and fee precision are now pointing to the incorrect storage slots, calculations will be performed incorrectly.

For fee precision, the initial value will be 3e15 rather than 1e18
Together this results in a fee that is much larger than expected. This is because the fee is calculated using the following function:

function getCalculatedFee(IERC20 token, uint256 amount) public view returns (uint256 fee) {
//slither-disable-next-line divide-before-multiply
// @audit price oracle manipulation
uint256 valueOfBorrowedToken = (amount * getPriceInWeth(address(token))) / s_feePrecision;
//slither-disable-next-line divide-before-multiply
fee = (valueOfBorrowedToken * s_flashLoanFee) / s_feePrecision;
}

This reduces into the following calculation:

function getCalculatedFee(IERC20 token, uint256 amount) public view returns (uint256 fee) {
//slither-disable-next-line divide-before-multiply
// @audit price oracle manipulation
uint256 valueOfBorrowedToken = (amount * getPriceInWeth(address(token))) / s_feePrecision;
//slither-disable-next-line divide-before-multiply
fee = (amount * s_flashLoanFee * getPriceInWeth(address(token))) / s_feePrecision;
}

Hence, if the flash loan fee is larger than expected, and the fee precision is smaller than expected, the value of fee will increase.
This means that flash loans will be more expensive initially than desired.

Storage slots being incorrectly ordered have downstream impacts and is a high-severity vulnerability

Proof of concept

Working test case

The following test proves that when the implementation contract is upgraded from ThunderLoan to ThunderLoanUpgraded, getFee() returns a flash loan fee with the value intended for the fee precision:

function test_poc_upgradeIncorrectStorageLayout() public {
vm.prank(thunderLoan.owner());
thunderLoan.upgradeToAndCall(address(thunderLoanUpgraded), abi.encodeWithSignature("getFee()"));
console.log("initial Fee: ", thunderLoan.getFee());
// The fee should be 3e15 but it is not due to the incorrect storage layout
assertTrue(thunderLoan.getFee() != 3e15);
}
$ forge test --mt test_poc_upgradeIncorrectStorageLayout -vvvv
​
// output
Running 1 test for test/unit/ThunderLoanTest.t.sol:ThunderLoanTest
[PASS] test_poc_upgradeIncorrectStorageLayout() (gas: 36816)
Logs:
initial Fee: 1000000000000000000
Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 2.74ms

The test passes, showing that the initial flash loan fee, s_flashLoanFee is 1e18 rather than 3e15 as desired. Hence, the storage variables are pointing to the incorrect storage slots.

Recommended mitigation

Re-order the storage variable declarations in ThuderLoanUpgraded:

+uint256 public constant FEE_PRECISION = 1e18;
uint256 private s_flashLoanFee;
-uint256 public constant FEE_PRECISION = 1e18;

Tools used

  • Forge

Updates

Lead Judging Commences

0xnevi Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

storage collision on upgrade

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.