First Flight #3: Thunder Loan

Summary

Difference between the order of storage variables in ThunderLoan and ThunderLoanUpgraded will cause storage collisions, leads to critical errors.

Vulnerability Details

Here is storage structure in ThunderLoan and ThunderLoanUpgraded contract.

Incorrect storage preservation in implementation:

In v0, the layout is as follows:

• s_tokenToAssetToken uses storage slot 0.

• s_feePrecision uses storage slot 1.

• s_flashLoanFee uses storage slot 2.

• s_currentlyFlashLoaning uses storage slot 3.

In v1, the layout is as follows:

• s_tokenToAssetToken uses storage slot 0.

• s_flashLoanFee uses storage slot 1.

• s_currentlyFlashLoaning uses storage slot 2.
  The storage collision occurs because the s_flashLoanFee in v1 is using the same storage slot (slot 1) that s_feePrecision used in v0, and s_currentlyFlashLoaning in v1 is using the same storage slot (slot 2) that s_flashLoanFee used in v0. This means that during the upgrade, the values stored in s_feePrecision and s_flashLoanFee in v0 will be overwritten by the values in s_flashLoanFee and s_currentlyFlashLoaning in v1, respectively.
  When contract is upgraded,

Impact

data will be corrupted which leads to unintended behaviour and loss of funds

Tools Used

Manual Review

Recommendations

Maintain the same storage stucture In new upgrades as well. Make sure that new variables are appended to the storage layout and that no existing slots are modified or reused.

storage preservation in implementation should look like this: