Hawk High
First Flight #39
Beginner FriendlySolidity
100 EXP
View results
Previous Next
Submission Details
Impact: high
Likelihood: high
Invalid

Incompatible Storage Layouts in the Upgradable Contract Implementations Risk Data misinterpretation, implementation logic malfunctioning, data corruption.

dreamybird734

Summary

Changes made in the storage variables, affects the storage layout which leads to incompatibility between the old(levelOne.sol) and the new versions(levelTwo.sol) of the implementation logic which can lead to incorrect implementation of the contract's logic returning unintended values.

Vulnerability Details

The upgradable contract system, utilizing OpenZeppelin's proxy pattern, exhibits a critical vulnerability due to incompatible storage layouts between the current implementation contract (levelOne.sol) and the proposed upgrade implementation (levelTwo.sol). Specifically the storage layout of these two logic contracts do not align, posing a significant risk to data integrity and contract functionality upon upgrade.

In levelOne.sol, the state variables are declared as follows:

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; // 35%
uint256 public constant PRINCIPAL_WAGE = 5; // 5%
uint256 public constant PRECISION = 100

In the proposed upgrade levelTwo.sol, the storage layout has been modified:

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;

It is noted that the storage layout of these two implementation logic contract are not compatible which pose a risk on code integrity, specifically speaking, in line 3 of levelOne.sol code, a storage value uint256 schoolFees; was removed in levelTwo.sol leading to the reordering of the storage slot, and this results in the data returning not intended values causing the contract logic to malfunction, the storage variables are misinterpreted as the storage slots in levlOne.sol are now replaced or misaligned. In this case upon upgrade uint256 public busary variable will be reading from a different slot 2 causing the data logic to be incorrect because the data was initially stored in slot 3, in a case where the data is overwritten it can lead to the loss of funds. Because of the misaligned storage layouts data retuned upon upgrades will be misinterpreted. In Hawk-High contract the following variables were removed in the proposed upgrade implementation (levelTwo.sol). uint256 schoolFees

mapping(address => uint256) private reviewCount

mapping(address => uint256)lastReviewTime .


Impact.

This can lead to incorrect contract logic in the contract upgrade, because the functions will be reading and returning values incompatible and inconsistent with the contract logic and data integrity.

For example uint256 public busarywill return not intended values upon upgrade causing the contract logic to malfunction, leading to unexpected behaviour, teachers and the principal might not be able to receive their payments because of wrong contract logic, inadvertenly results in fund loss in case of data being overwritten. The rest of the storage variables will be misinterpreted upon upgrades as a result of different storage slot layout.

Tools Used

forge inspect tool was used to checkout the storage layout.

Recommendations.

  • Maintain the exact storage layout in the previous implementation.

  • Create a new implementation contract that restores a compatible storage layout with the original while incorporating the desired new logic.


Updates

Lead Judging Commences

yeahchibyke Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Lack of quality
yeahchibyke Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Lack of quality

Support

FAQs

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

Give us feedback!