First Flight #3: Thunder Loan

Summary

ThunderLoanUpgraded contract make a change in the contract storage layout which will undesired effect.

Vulnerability Details

ThunderLoan contract storage contract is:

ThunderLoanUpgraded contract storage contract is:

because s_feePrecision which was a uint256 private variable become FEE_PRECISION which is uint256 public constant The Solidity compiler does not reserve a storage slot for constant and instead replaces every occurrence of these variables with their assigned value in the contract’s bytecode.

The elements in mappings are not stored sequentially, the slot in which the mapping is declared doesn’t hold any information, because mappings have no length. The formula to determine the slot in which each element in a mapping will be stored is the following:

and since the SLOT_MAPPING_DECLARATION change, all the underlying memory slot will change.
In old contract SLOT_MAPPING_DECLARATION=3 and in the upgraded contract SLOT_MAPPING_DECLARATION=2

Impact

Huge impact because all the memory become mess up

Tools Used

reading the code

Recommendations

If you want to upgrade a proxy implementation contract, you need to keep the exact same storage layout to avoid this type of issue.