Contracts without GAP can lead a upgrade disaster

[code423n4]

Lines of code

https://github.com/code-423n4/2022-10-thegraph/blob/309a188f7215fa42c745b136357702400f91b4ff/contracts/l2/token/GraphTokenUpgradeable.sol#L28

Vulnerability details

Impact

Some contracts do not implement good upgradeable logic.

Proof of Concept

Upgrading a contract will need a __gap storage in order to avoid storage collisions.

Proxied contracts MUST include an empty reserved space in storage, usually named __gap, in all the inherited contracts.

For example, the contract Governed or GraphUpgradeable are inherited by upgradeable contracts, but these contracts don't have a __gap storage, so if a new variable is created it could lead to storage collisions that will produce a contract disaster, including loss of funds.

Reference:

• https://docs.openzeppelin.com/contracts/3.x/upgradeable#storage_gaps
• https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable/blob/2cb8996b777060e658e2b8c9b1630313aedb04c0/contracts/token/ERC20/ERC20Upgradeable.sol#L394

Affected source code:

• GraphTokenUpgradeable.sol:28

Recommended Mitigation Steps

• Add uint256[50] private __gap; to all contracts.

[trust1995]

Dup of #306

[0xean]

dupe of #185 - wardens QA