Beginner FriendlyFoundryDeFiOracle
100 EXP
View results
Submission Details
Severity: high
Valid

The Logic of the Contract's Variable Declaration Order and Type should always be maintained

Summary

Maintaining the order and type of state variables is crucial for ensuring the compatibility and integrity of the storage layout across different contract versions. The protocol fails to do it.

Vulnerability Details

ThunderLoan.sol :

uint256 private s_feePrecision;
uint256 private s_flashLoanFee; // 0.3% ETH fee

while ThunderLoanUpgraded.sol has the following:

uint256 private s_flashLoanFee; // 0.3% ETH fee
uint256 public constant FEE_PRECISION = 1e18;

As we can see there will be a collision between the TunderLoan's s_feePrecision and s_flashLoanFee from ThunderLoanUpgraded.

Impact

Performing the upgrade will disrupt the functionality of the contract.

Tools Used

Manual review.

Recommendations

Ensure that s_flashLoanFee remains in slot 1 of the upgraded contract. Even if it's not that gas efficient, keep both the initial variables, in the way they were declared in the ThunderLoan contract and declare the constant at the end (this constan't doesn't affect storage anyway because it's baked directly into the bytecode of the contract when it is compiled)

In ThunderLoanUpgraded.sol

++ uint256 private s_feePrecision;
uint256 private s_flashLoanFee; // 0.3% ETH fee
uint256 public constant FEE_PRECISION = 1e18;
Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year 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.