Missing storage _gap may result in storage slot collision during upgrades

[code423n4]

Lines of code

https://github.com/code-423n4/2022-12-gogopool/blob/main/contracts/contract/tokens/upgradeable/ERC20Upgradeable.sol#L10
https://github.com/code-423n4/2022-12-gogopool/blob/aec9928d8bdce8a5a4efe45f54c39d4fc7313731/contracts/contract/tokens/upgradeable/ERC4626Upgradeable.sol#L11

Vulnerability details

Impact

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" (quote OpenZeppelin). 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, potentially causing loss of user fund or cause the contract to malfunction completely.

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

Proof of Concept

The ERC20Upgradeable and ERC4626Upgradeable contracts are intended to be upgradable, but they are lacking storage gaps. The storage gaps are essential because "It allows us to freely add new state variables in the future without compromising the storage compatibility with existing deployments".

Tools Used

Visual Studio Code, Open Zeppelin

Recommended Mitigation Steps

As explained in https://docs.openzeppelin.com/upgrades-plugins/1.x/writing-upgradeable#storage-gaps, add storage gaps to to the two upgradable files.

uint256[48] __gap;

[c4-judge]

GalloDaSballo marked the issue as duplicate of #300

[c4-judge]

GalloDaSballo marked the issue as not a duplicate

[c4-judge]

GalloDaSballo changed the severity to QA (Quality Assurance)

[GalloDaSballo]

L

[c4-judge]

GalloDaSballo marked the issue as grade-c