Sparkn

Summary

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" -OpenZeppelin

Vulnerability Details

This upgradeability system consists of a proxy contract which users interact with directly and that is in charge of forwarding transactions to and from a second contract. This second contract contains the logic, commonly known as the implementation contract.

When using this particular upgradeability pattern, it is important to take into account any potential changes in the storage layout of a contract, as there can be storage collisions between different versions of the same implementation. Some possible scenarios are:

When changing the order of the variables in the contract
When removing the non-latest variable defined in the contract
When changing the type of a variable
When introducing a new variable before any existing one
In some cases, when adding a new field to a struct in the contract
There is no certainty that the storage layout will remain safe after an upgrade. Violating any of these storage layout restrictions will cause the upgraded version of the contract to have its storage values mixed up, and can lead to critical errors in the contracts.

Impact

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" -OpenZeppelin. 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, potentially causing loss of user fund or cause the contract to malfunction completely.

Further Reading : https://docs.openzeppelin.com/upgrades-plugins/1.x/writing-upgradeable

Tools Used

Manual Review

Recommendations

Consider adding a correct __gap variable to these contracts, or documenting a plan for managing storage collisions when upgrading