The storage layout in *StorageV1 may prevent the contract from being upgraded

[code423n4]

Lines of code

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/auction/storage/AuctionStorageV1.sol#L10-L20

Vulnerability details

Impact

In the *StorageV1 contract, the Settings structure variable is at the top of the storage, which causes a storage collision if the Settings structure needs to be extended later.

contract TokenStorageV1 is TokenTypesV1 {
    /// @notice The token settings
    Settings internal settings;

    /// @notice The vesting details of a founder
    /// @dev Founder id => Founder
    mapping(uint256 => Founder) internal founder;

    /// @notice The recipient of a token
    /// @dev ERC-721 token id => Founder
    mapping(uint256 => Founder) internal tokenRecipient;
}

Also, a storage gap should be added to the *StorageV1 contract to allow new variables to be added when the contract is upgraded.

Proof of Concept

https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/auction/storage/AuctionStorageV1.sol#L10-L20
https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/governor/storage/GovernorStorageV1.sol#L9-L20
https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/governance/treasury/storage/TreasuryStorageV1.sol#L9-L16
https://github.com/code-423n4/2022-09-nouns-builder/blob/7e9fddbbacdd7d7812e912a369cfd862ee67dc03/src/token/storage/TokenStorageV1.sol#L9-L20

Tools Used

None

Recommended Mitigation Steps

Consider moving the Settings structure to the end of the *StorageV1 contract and adding a storage gap at the end of the *StorageV1 contract.

uint256[50] private __gap;

[GalloDaSballo]

I think the report has some validity but ultimately slot math has to be performed on each upgrade and no perfect solution exists

Even adding a gap can still cause issues if not handled properly

[GalloDaSballo]

Dup of #358