Summary

No storage gap for Upgradeable Contracts might lead to storage slot collision.

Vulnerability Details

For upgradeable contracts, there must be a 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 complicated to write new implementation code. Without a storage gap, the variable in the child contract might be overwritten by the upgraded base contract if new variables are added to the base contract. This could have unintended and severe consequences to the child contracts, potentially causing loss of user funds or causing the contract to malfunction completely.

Refer to the bottom part of this article: https://docs.openzeppelin.com/upgrades-plugins/1.x/writing-upgradeable

Proof of Concept

Several contracts are intended to be upgradeable contracts in the code base, including

• SDLPool.sol

• SDLPoolPrimary.sol

• SDLPoolSecondary.sol

However, none of these contracts contain a storage gap. The storage gap is essential for upgradeable contracts because "It allows us to freely add new state variables in the future without compromising the storage compatibility with existing deployments". Refer to the bottom part of this article:

https://docs.openzeppelin.com/contracts/3.x/upgradeable

As an example, SDLPool is intended to act as the base contract in the project. If the contract inheriting the base contract contains an additional variable, then the base contract cannot be upgraded to include any additional variable, because it would overwrite the variable declared in its child contract. This greatly limits contract upgradeability.

Impact

Not including a variable for storage gaps can lead to restricted future upgrades and the risk of overwriting variables.

Tools Used

VsCode

Recommendations

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