Hawk High
First Flight #39
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: medium
Valid

Inconsistent storage layout causes data corruption

mishoko

Summary

A critical storage collision vulnerability exists in the upgrade mechanism between LevelOne and LevelTwo contracts. The storage layout mismatch during the upgrade process leads to severe data corruption and potential loss of critical state variables.

Vulnerability Details

The vulnerability stems from inconsistent storage layouts between the two contracts. LevelOne contains several state variables that are missing in LevelTwo, including:

  • schoolFees

  • reviewTime

  • reviewCount

  • lastReviewTime

When upgrading from LevelOne to LevelTwo, the storage slots shift to fill these gaps, causing:

  1. Direct value corruption (e.g., bursary value changes from 15,000 USDC to 2419201)

  2. Mapping corruption (student scores become corrupted)

  3. Array data loss (student and teacher lists become empty)

  4. Constant value changes (teacher wage changes from 35% to 40%)

Impact

The storage collision has severe consequences:

  1. Financial Impact: Bursary amounts are corrupted, potentially leading to incorrect fund distribution

  2. Data Loss: All student and teacher records are lost

  3. Access Control: Teacher and student status mappings are corrupted

  4. Business Logic: Different constant values (TEACHER_WAGE vs TEACHER_WAGE_L2) lead to incorrect calculations

  5. Contract State: The entire contract state becomes unreliable after upgrade

Tools Used

  • Manual code review

  • Foundry for testing

PoC

// SPDX-License-Identifier: MIT
pragma solidity 0.8.26;
​
import {Test, console2} from "forge-std/Test.sol";
import {DeployLevelOne} from "../script/DeployLevelOne.s.sol";
import {LevelOne} from "../src/LevelOne.sol";
import {LevelTwo} from "../src/LevelTwo.sol";
import {MockUSDC} from "./mocks/MockUSDC.sol";
​
contract StorageCollisionPOC is Test {
DeployLevelOne deployBot;
LevelOne levelOneProxy;
LevelTwo levelTwoImplementation;
MockUSDC usdc;
address principal;
uint256 schoolFees;
​
function setUp() public {
deployBot = new DeployLevelOne();
address proxyAddress = deployBot.deployLevelOne();
levelOneProxy = LevelOne(proxyAddress);
usdc = deployBot.getUSDC();
principal = deployBot.principal();
schoolFees = deployBot.getSchoolFees();
}
​
function test_storage_collision_impact() public {
// 1. Setup initial state in LevelOne
vm.startPrank(principal);
address teacher1 = makeAddr("teacher1");
address teacher2 = makeAddr("teacher2");
levelOneProxy.addTeacher(teacher1);
levelOneProxy.addTeacher(teacher2);
vm.stopPrank();
​
// 2. Enroll multiple students
address[] memory students = new address[](3);
for (uint256 i = 0; i < 3; i++) {
students[i] = makeAddr(string(abi.encodePacked("student", i)));
vm.startPrank(students[i]);
usdc.mint(students[i], schoolFees);
usdc.approve(address(levelOneProxy), schoolFees);
levelOneProxy.enroll();
vm.stopPrank();
}
​
// 3. Start session
vm.prank(principal);
levelOneProxy.startSession(70);
​
// 4. Give reviews to students
vm.warp(block.timestamp + 1 weeks);
vm.startPrank(teacher1);
levelOneProxy.giveReview(students[0], true);
levelOneProxy.giveReview(students[1], false); // Give bad review to Student 1
vm.stopPrank();
​
// 5. Verify initial state before upgrade
console2.log("\n=== Initial State (LevelOne) ===");
console2.log("Student 0 Score:", levelOneProxy.studentScore(students[0]));
console2.log("Student 1 Score:", levelOneProxy.studentScore(students[1]));
console2.log("Student 2 Score:", levelOneProxy.studentScore(students[2]));
console2.log("Teacher Wage:", levelOneProxy.TEACHER_WAGE());
console2.log("Principal Wage:", levelOneProxy.PRINCIPAL_WAGE());
console2.log("Bursary:", levelOneProxy.bursary());
console2.log("Total Students:", levelOneProxy.getTotalStudents());
console2.log("Total Teachers:", levelOneProxy.getTotalTeachers());
​
// 6. Perform upgrade to LevelTwo
levelTwoImplementation = new LevelTwo();
bytes memory data = abi.encodeCall(LevelTwo.graduate, ());
vm.prank(principal);
levelOneProxy.upgradeToAndCall(address(levelTwoImplementation), data);
​
// 7. Cast proxy to LevelTwo
LevelTwo levelTwoProxy = LevelTwo(address(levelOneProxy));
​
// 8. Verify state after upgrade (should show storage collision)
console2.log("\n=== State After Upgrade (LevelTwo) ===");
console2.log("Student 0 Score:", levelTwoProxy.studentScore(students[0]));
console2.log("Student 1 Score:", levelTwoProxy.studentScore(students[1]));
console2.log("Student 2 Score:", levelTwoProxy.studentScore(students[2]));
console2.log("Teacher Wage L2:", levelTwoProxy.TEACHER_WAGE_L2());
console2.log("Principal Wage L2:", levelTwoProxy.PRINCIPAL_WAGE_L2());
console2.log("Bursary:", levelTwoProxy.bursary());
console2.log("Total Students:", levelTwoProxy.getTotalStudents());
console2.log("Total Teachers:", levelTwoProxy.getTotalTeachers());
}
}
​

results in

=== Initial State (LevelOne) ===
Student 0 Score: 100
Student 1 Score: 90
Student 2 Score: 100
Teacher Wage: 35
Principal Wage: 5
Bursary: 15000000000000000000000
Total Students: 3
Total Teachers: 2
=== State After Upgrade (LevelTwo) ===
Student 0 Score: 1
Student 1 Score: 1
Student 2 Score: 1
Teacher Wage L2: 40
Principal Wage L2: 5
Bursary: 2419201
Total Students: 0

Recommendations

  1. Maintain consistent storage layout between upgradeable contracts:

    • Keep all state variables from the previous version

    • Add new variables at the end of the storage layout

    • Use storage gaps to prevent collisions

  2. Add storage layout checks in the upgrade function to verify compatibility

  3. Consider implementing a migration function to properly transfer state between versions

  4. Add comprehensive tests that verify storage layout compatibility before allowing upgrades

Updates

Lead Judging Commences

yeahchibyke Lead Judge 7 months ago
Submission Judgement Published
Validated
Assigned finding tags:

storage collision

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!