Summary

Storage collision vulnerability identified in a potential upgrade from the LevelOne contract to the LevelTwo contract. This collision can lead to data corruption and unpredictable contract behavior.

Vulnerability Details

The LevelOne and LevelTwo contracts define their state variables with differing orders and present different sets of variables. When a contract is upgraded using a proxy pattern (like UUPS), the new implementation's code operates on the storage of the proxy, which was initially laid out according to the LevelOne contract. If the variable declarations in LevelTwo do not perfectly match the storage layout of LevelOne, a storage collision will occur. Specifically, variables in LevelTwo will be written to storage slots that were previously used by different variables in LevelOne.

In this case, the following differences exist:

• Reordered Variables: The order of the first few variables is different. LevelOne starts with address principal, bool inSession, uint256 schoolFees, uint256 public immutable reviewTime = 1 weeks uint256 public sessionEnd etc while LevelTwo starts with address principal, bool inSession, uint256 sessionEnd. etc

• Missing Variables: LevelTwo is missing several variables present in LevelOne, including schoolFees, reviewTime, reviewCount, lastReviewTime, TEACHER_WAGE, and PRINCIPAL_WAGE.

These discrepancies will cause LevelTwo to read and write to incorrect storage slots, corrupting the data previously stored by LevelOne.

Impact

• Data Corruption: Existing data stored by the proxy contract will be overwritten or misinterpreted by LevelTwo, leading to loss of data integrity.

• Unpredictable Contract Behavior: The contract may function incorrectly, exhibit unexpected behavior, or revert due to accessing corrupted data.

Tools Used

• Manual code review and analysis of the LevelOne and LevelTwo contract source code.

Recommendations

To mitigate this storage collision vulnerability, the following recommendations should be followed:

1. Ensure Storage Layout Compatibility: When upgrading to a new implementation contract, the order and types of existing state variables at the beginning of the contract must be preserved. New variables should be appended to the end of the variable list.

2. Use Storage Gaps: Employ storage gap variables (e.g., uint256[50] private __gap;) in the contract to reserve space in the proxy's storage. This allows for the addition of new state variables in future implementations without overwriting existing ones. These gaps should be placed at the end of the variable declarations.

By following these recommendations, the risk of storage collisions during contract upgrades can be significantly reduced, ensuring data integrity and preventing unpredictable contract behavior.