LevelOne and LevelTwo storage layout do not match
Summary
The storage layout of the LevelTwo contract does not match the storage layout of the LevelOne contract. This means that after the upgrade, the state variables will clash and the contract will not work as expected.
Vulnerability Details
LevelOne storage layout:
| Name | Type | Slot | Offset | Bytes | Contract |
|---|---|---|---|---|---|
| principal | address | 0 | 0 | 20 | src/LevelOne.sol:LevelOne |
| inSession | bool | 0 | 20 | 1 | src/LevelOne.sol:LevelOne |
| schoolFees | uint256 | 1 | 0 | 32 | src/LevelOne.sol:LevelOne |
| sessionEnd | uint256 | 2 | 0 | 32 | src/LevelOne.sol:LevelOne |
| bursary | uint256 | 3 | 0 | 32 | src/LevelOne.sol:LevelOne |
| cutOffScore | uint256 | 4 | 0 | 32 | src/LevelOne.sol:LevelOne |
| isTeacher | mapping(address => bool) | 5 | 0 | 32 | src/LevelOne.sol:LevelOne |
| isStudent | mapping(address => bool) | 6 | 0 | 32 | src/LevelOne.sol:LevelOne |
| studentScore | mapping(address => uint256) | 7 | 0 | 32 | src/LevelOne.sol:LevelOne |
| reviewCount | mapping(address => uint256) | 8 | 0 | 32 | src/LevelOne.sol:LevelOne |
| lastReviewTime | mapping(address => uint256) | 9 | 0 | 32 | src/LevelOne.sol:LevelOne |
| listOfStudents | address[] | 10 | 0 | 32 | src/LevelOne.sol:LevelOne |
| listOfTeachers | address[] | 11 | 0 | 32 | src/LevelOne.sol:LevelOne |
| usdc | contract IERC20 | 12 | 0 | 20 | src/LevelOne.sol:LevelOne |
LevelTwo storage layout:
| Name | Type | Slot | Offset | Bytes | Contract |
|---|---|---|---|---|---|
| principal | address | 0 | 0 | 20 | src/LevelTwo.sol:LevelTwo |
| inSession | bool | 0 | 20 | 1 | src/LevelTwo.sol:LevelTwo |
| sessionEnd | uint256 | 1 | 0 | 32 | src/LevelTwo.sol:LevelTwo |
| bursary | uint256 | 2 | 0 | 32 | src/LevelTwo.sol:LevelTwo |
| cutOffScore | uint256 | 3 | 0 | 32 | src/LevelTwo.sol:LevelTwo |
| isTeacher | mapping(address => bool) | 4 | 0 | 32 | src/LevelTwo.sol:LevelTwo |
| isStudent | mapping(address => bool) | 5 | 0 | 32 | src/LevelTwo.sol:LevelTwo |
| studentScore | mapping(address => uint256) | 6 | 0 | 32 | src/LevelTwo.sol:LevelTwo |
| listOfStudents | address[] | 7 | 0 | 32 | src/LevelTwo.sol:LevelTwo |
| listOfTeachers | address[] | 8 | 0 | 32 | src/LevelTwo.sol:LevelTwo |
| usdc | contract IERC20 | 9 | 0 | 20 | src/LevelTwo.sol:LevelTwo |
The first mismatch is schoolFees that is missing in LevelTwo. Other mismatches are present between both but since one missing variable is enough to cause all other values to be shifted incorrectly, I will not go into details about the other mismatches. You can see for example that the slot for the usdc variable is 12 in LevelOne and 9 in LevelTwo. This means that the usdc variable will contain the value of the lastReviewTime variable after the upgrade.
Impact
After the upgrade, the state variables will clash and the contract will not work as expected. Variables will be missing or contain wrong values. This can lead to unexpected results and most likely break the contract.
Tools Used
Manually reviewed the code.
Recommendations
Make sure that the storage layout of the LevelTwo contract matches the storage layout of the LevelOne contract. This can be done by adding the missing variables to the LevelTwo contract and making sure that the order of the variables is the same as in the LevelOne contract.
Rewards breakdown
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.