Description: Variables LevelOne:schoolFees, LevelOne:reviewCount and LevelOne:lastReviewTime at storage slot 2, 9 & 10 respectively, are not included in LevelTwo contract variable which cause mixing up of variable.
Vulnerability Details: four variables that are in LevelOne contract but not included in LevelTwo contract
contract LevelTwo is Initializable{
using SafeERC20 for IERC20;
address principal;
bool inSession;
@>
uint256 public sessionEnd;
uint256 public bursary;
uint256 public cutOffScore;
mapping(address => bool) public isTeacher;
mapping(address => bool) public isStudent;
mapping(address => uint256) public studentScore;
@>
@>
address[] listOfStudents;
address[] listOfTeachers;
uint256 public constant TEACHER_WAGE_L2 = 40;
uint256 public constant PRINCIPAL_WAGE_L2 = 5;
uint256 public constant PRECISION = 100;
IERC20 usdc;
contract LevelOne is Initializable, UUPSUpgradeable {
using SafeERC20 for IERC20;
address principal;
bool inSession;
@> uint256 schoolFees;
@> uint256 public immutable reviewTime = 1 weeks;
uint256 public sessionEnd;
uint256 public bursary;
uint256 public cutOffScore;
mapping(address => bool) public isTeacher;
mapping(address => bool) public isStudent;
mapping(address => uint256) public studentScore;
@> mapping(address => uint256) private reviewCount;
@> mapping(address => uint256) private lastReviewTime;
address[] listOfStudents;
address[] listOfTeachers;
uint256 public constant TEACHER_WAGE = 35;
uint256 public constant PRINCIPAL_WAGE = 5;
uint256 public constant PRECISION = 100;
IERC20 usdc;
Impact: storage collision
Tools Used: Manual Review
Proof of Concept: For this to work add UUPSUpgradable to LevelTwo contract
Add this test to your LeveOnelAndGraduateTest.t.sol
First We add teachers and enroll students set value for bursary at slot 5
Upgrade the protocol to level two
storage slot of bursary is at 3
when we try to get bursary at slot 3 we actually get value of reviewTime of value 1 weeks at slot3 in LevelOne contract
Proof of Code
function test_StorageCollision() public {
vm.startPrank(principal);
levelOneProxy.addTeacher(alice);
levelOneProxy.addTeacher(bob);
vm.stopPrank();
vm.startPrank(clara);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
vm.startPrank(dan);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
vm.startPrank(eli);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
vm.startPrank(fin);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
vm.startPrank(grey);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
vm.startPrank(harriet);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
uint256 cuttOffScroe = 70;
vm.prank(principal);
levelOneProxy.startSession(cuttOffScroe);
bool isSessionOver = block.timestamp > levelOneProxy.getSessionEnd();
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
uint256 sessionEndOfLevelOne = levelOneProxy.sessionEnd();
vm.startPrank(principal);
levelOneProxy.graduateAndUpgrade(levelTwoImplementationAddress, data);
levelOneProxy.upgradeToAndCall(levelTwoImplementationAddress, data);
vm.stopPrank();
uint256 bursaryOfLevelTwo = LevelTwo(proxyAddress).bursary();
assert(usdc.balanceOf(principal) == 1500 ether);
assert(LevelTwo(proxyAddress).TEACHER_WAGE_L2() == 40);
assert(!isSessionOver);
assert(bursaryOfLevelTwo == sessionEndOfLevelOne);
}
Recommendations: Add the missing variables that were not included in LevelTwo contract
contract LevelTwo is Initializable, UUPSUpgradeable {
using SafeERC20 for IERC20;
address principal;
bool inSession;
+ uint256 schoolFees;
uint256 public sessionEnd;
uint256 public bursary;
uint256 public cutOffScore;
mapping(address => bool) public isTeacher;
mapping(address => bool) public isStudent;
mapping(address => uint256) public studentScore;
+ mapping(address => uint256) private reviewCount;
+ mapping(address => uint256) private lastReviewTime;
address[] listOfStudents;
address[] listOfTeachers;
