No storage gap for upgradeable contract might lead to storage slot collision

[code423n4]

Lines of code

https://github.com/code-423n4/2022-11-paraspace/tree/main/paraspace-core/contracts/protocol/configuration/PoolAddressesProvider.sol#L20

Vulnerability details

Description

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

Impact

See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps for a description of this storage variable. While some contracts may not currently be sub-classed, adding the variable now protects against forgetting to add it in the future.

Proof Of Concept

PoolAddressesProvider uses the following upgradeability libraries: ../libraries/paraspace-upgradeability/InitializableImmutableAdminUpgradeabilityProxy.sol and ../libraries/paraspace-upgradeability/ParaProxy.sol. It is intended to be an upgradeable contract in the code base

However, the contract doesn't contain a storage gap. The storage gap is essential for upgradeable contract because “It allows us to freely add new state variables in the future without compromising the storage compatibility with existing deployments”. See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps for a description of this storage variable. While some contracts may not currently be sub-classed, adding the variable now protects against forgetting to add it in the future.

https://github.com/code-423n4/2022-11-paraspace/tree/main/paraspace-core/contracts/protocol/configuration/PoolAddressesProvider.sol#L20

Recommended Mitigation Steps

Recommend adding appropriate storage gap at the end of upgradeable contracts such as the below. Please reference OpenZeppelin upgradeable contract templates.

    uint256[50] private __gap;

[dmvt]

Storage gaps are considered QA.

[c4-judge]

dmvt marked the issue as unsatisfactory:
Overinflated severity