stake.link

Summary

For upgradeable contracts as per Openzeppelin’s docs storage gaps are a convention for reserving storage slots in a base contract, allowing future versions of that contract to use up those slots without affecting the storage layout of child contracts.
Otherwise, it may be very difficult to write new implementation code.

Vulnerability Details

The SDLPool inherits RewardsPoolController that utilizes UUPSUpgradeable and ERC721Upgradeable.
The SDLPool is then inherited by SDLPoolPrimary and SDLPoolSecondary contracts.
The issue lies as there is no storage gap defined for these contract.
This means that if an upgrade to SDLPool occurs in near future then it cannot be upgraded with any additional variable, because it would overwrite already declared variables.
This will create unexpected behaviour towards overall protocol to deploy and implementing proxies.

Impact

Without storage gap, state variable have high possibilities to be overwritten by the upgraded base contract if new variables are added to the base contract.
This could have unintended and very serious consequences on the project if any of the variable is overwritten with unexpected value.

Tools Used

OZ’s docs

Recommendations

The recommendation is made in area of adding storage gap at the end of all upgradeable contracts i.e., RewardsPoolController , SDLPool, SDLPoolPrimary and SDLPoolSecondary as referenced by OpenZeppelin’s upgradeable contract guideline and mentioned below: