sherlock-audit / 2024-02-rubicon-finance-judging

5 stars 3 forks source link

4b - No storage gap for upgradeable contracts `GladiusReactor` and `RubiconFeeController` #49

Closed sherlock-admin2 closed 7 months ago

sherlock-admin2 commented 7 months ago

4b

medium

No storage gap for upgradeable contracts GladiusReactor and RubiconFeeController

Summary

The upgradeable contracts GladiusReactor and RubiconFeeController have no storage gaps which can lead to a storage overwrite during upgrade

Vulnerability Detail

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.

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

Impact

when GladiusReactor and RubiconFeeController do not contain any storage gaps, In future upgrades if an additional variable is added to these contracts, that new variable will overwrite the storage slot of the last variable in their respective child contracts

Code Snippet

No storage gaps in GladiusReactor https://github.com/sherlock-audit/2024-02-rubicon-finance/blob/main/gladius-contracts-internal/src/reactors/GladiusReactor.sol#L18

No storage gaps in RubiconFeeController https://github.com/sherlock-audit/2024-02-rubicon-finance/blob/main/gladius-contracts-internal/src/fee-controllers/RubiconFeeController.sol#L16

Tool used

Manual Review

Recommendation

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

sherlock-admin commented 7 months ago

3 comment(s) were left on this issue during the judging contest.

tsvetanovv commented:

Low. "Storage Slot Collision issue" is Low according to the Sherlock documentation

PNS commented:

Simple contracts with one of the parent contract not implementing storage gaps are considered low/informational.

0xAadi commented:

Invalid: OOS