First Flight #3: Thunder Loan

Vulnerability Details

A storage collision issue is identified between ThunderLoan.sol and ThunderLoanUpgraded.sol. In the original contract (ThunderLoan.sol), two private state variables s_feePrecision and s_flashLoanFee are declared:

In the upgraded contract (ThunderLoanUpgraded.sol), the s_feePrecision variable has been removed, and a new constant FEE_PRECISION is introduced:

This modification can lead to a storage collision because the layout of the state variables in the upgraded contract does not match the layout in the original contract. The s_flashLoanFee in the upgraded contract may occupy the storage slot that was originally for s_feePrecision in the original contract.

Impact

If not managed properly, a storage collision can lead to unexpected behavior when upgrading to a new contract. The values of the state variables may be incorrect, leading to logical errors in the contract's execution. This can result in loss of funds or compromised contract integrity, severely affecting the protocol's functionality and user trust.

Recommendations

Before deploying the upgraded contract, it's crucial to ensure that the storage layout matches the original contract's layout. Any new variables should be appended to the end of the storage layout, and if variables are removed, their slots should either be left empty or replaced with variables of the same type and size to avoid collisions.

In this specific case, the upgraded contract should maintain the s_feePrecision variable (even if it is no longer used) to preserve the storage layout, or explicitly specify the storage slot to be used for s_flashLoanFee using the EIP-1967 standard to ensure it does not overlap with any existing storage.

For future upgrades, consider using storage gap patterns, where a reserved space (array of uint256) is placed in the storage layout to allow for future variable additions without affecting existing storage.

Here's a suggested code modification to maintain the storage layout:

Alternatively, explicitly specify the storage slot for the new variable: