sherlock-audit / 2024-02-optimism-2024-judging

6 stars 4 forks source link

jasonxiale - No Storage Gap for Upgradeable Contracts #187

Closed sherlock-admin4 closed 6 months ago

sherlock-admin4 commented 7 months ago

jasonxiale

medium

No Storage Gap for Upgradeable Contracts

Summary

For upgradeable contracts, there must be storage gap to "allow developers to freely add new state variables in the future without compromising the storage compatibility with existing deployments". Otherwise it may be very difficult to write new implementation code. Without storage gap, the variable in child contract might be overwritten by the upgraded base contract if new variables are added to the base contract. This could have unintended and very serious consequences to the child contracts.

Refer to the bottom part of this article: https://docs.openzeppelin.com/upgrades-plugins/1.x/writing-upgradeable

Vulnerability Detail

DelayedWETH.sol is an upgradeable contract and inherits from WETH98, and at the same time, there are storage variables defined in WETH98

 31     mapping(address => uint256) public balanceOf;
 32     mapping(address => mapping(address => uint256)) public allowance;

So storage gap should be included

Impact

Code Snippet

Tool used

Manual Review

Recommendation

nevillehuang commented 6 months ago

Invalid, based on sherlock rules

  1. Use of Storage gaps: Simple contracts with one of the parent contract not implementing storage gaps are considered low/informational.