code423n4 commented 1 year ago

Lines of code

https://github.com/nounsDAO/nouns-monorepo/blob/718211e063d511eeda1084710f6a682955e80dcb/packages/nouns-contracts/contracts/governance/fork/newdao/token/base/ERC721CheckpointableUpgradeable.sol#L50

Vulnerability details

Impact

The current ERC721CheckpointableUpgradeable contract doesn't have any reserved storage gap. However, any logic contract that serves as a foundational contract and is expected to be inherited by other upgradable children should preserve a reasonable amount of storage gap. This is necessary to accommodate new state variables that might be introduced during future upgrades.

Proof of Concept

The contract ERC721CheckpointableUpgradeable is intended to be upgradable and used as a base contract, however there is no storage gap in this contract, which will limit the ability to upgrade.

Tools Used

Recommended Mitigation Steps

It is better to have a storage gap preserved in base contract ERC721CheckpointableUpgradeable in case that new state variables are introduced in future upgrades. For more information, please refer to: https://docs.openzeppelin.com/contracts/3.x/upgradeable#storage_gaps.