search

code-423n4 / 2024-03-zksync-findings

2 stars 1 forks source link

Add storage gasp to avoid overwriting storage slots #99

Closed c4-bot-9 closed 7 months ago

c4-bot-9 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-03-zksync/blob/main/code/contracts/ethereum/contracts/state-transition/chain-deps/facets/ZkSyncStateTransitionBase.sol#L11 https://github.com/code-423n4/2024-03-zksync/blob/main/code/contracts/ethereum/contracts/upgrades/BaseZkSyncUpgrade.sol#L44 https://github.com/code-423n4/2024-03-zksync/blob/main/code/contracts/ethereum/contracts/upgrades/BaseZkSyncUpgradeGenesis.sol#L17

Vulnerability details

Impact

For the Upgradeable contracts, the inherited contract ZkSyncStateTransitionBase should contain storage gaps to avoid issues. If the Contracts inheriting BaseZkSyncUpgrade are upgraded with additional variables, it should not conflict with storage slots previously written on the proxy

Proof of Concept

See https://docs.openzeppelin.com/upgrades-plugins/1.x/writing-upgradeable#storage-gaps

Tools Used

Manual Review

Recommended Mitigation Steps

add storage gaps like below

uint256[49] __gap;

Assessed type

Upgradable

saxenism commented 8 months ago

Invalid.

c4-sponsor commented 8 months ago

saxenism (sponsor) disputed

alex-ppg commented 7 months ago

The Warden specifies that a contract expected to be upgradeable does not allocate a gap to accommodate for future variable inclusions, however, the contract in question is inherited by contracts that do not introduce any variables. In any case, this submission is considered a best practice recommendation rather than an actual vulnerability rendering it invalid for an HM classification.

c4-judge commented 7 months ago

alex-ppg marked the issue as unsatisfactory: Overinflated severity