code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-drips/blob/main/src/Managed.sol#L18

Vulnerability details

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”. Otherwise, it may be very difficult to write new implementation code. Without storage gap, the variable in the contract contract might be overwritten by the upgraded contract if new variables are added. This could have unintended and very serious consequences to the child contracts.

Impact

Without storage gap, the variable in the contract contract might be overwritten by the upgraded contract if new variables are added. This could have unintended and very serious consequences to the child contracts.

Proof of Concept

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”.

This contract are intended to be upgradeable contracts in the code base: Managed.sol

4: import {UUPSUpgradeable} from "openzeppelin-contracts/proxy/utils/UUPSUpgradeable.sol";
...
18: abstract contract Managed is UUPSUpgradeable {

Tools Used

Manual Review

Recommended Mitigation Steps

Recommend adding appropriate storage gap at the end of upgradeable storage contracts such as the below. Please reference OpenZeppelin upgradeable contract templates. uint256[50] private __gap;

GalloDaSballo commented 1 year ago

QA - Low severity per https://github.com/code-423n4/org/issues/55

c4-judge commented 1 year ago

GalloDaSballo changed the severity to QA (Quality Assurance)