sherlock-audit / 2024-02-rio-network-core-protocol-judging

4 stars 4 forks source link

MatricksDeCoder - Lack of storage gap for upgradeable contracts #355

Closed sherlock-admin3 closed 8 months ago

sherlock-admin3 commented 8 months ago

MatricksDeCoder

medium

Lack of storage gap for upgradeable contracts

Summary

Upgradeable contracts are missing _gap variable

Vulnerability Detail

"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" (quote OpenZeppelin).

https://github.com/sherlock-audit/2024-02-rio-network-core-protocol/blob/4f01e065c1ed346875cf5b05d2b43e0bcdb4c849/rio-sherlock-audit/contracts/restaking/RioLRT.sol#L15

https://github.com/sherlock-audit/2024-02-rio-network-core-protocol/blob/4f01e065c1ed346875cf5b05d2b43e0bcdb4c849/rio-sherlock-audit/contracts/restaking/RioLRTAVSRegistry.sol#L9

https://github.com/sherlock-audit/2024-02-rio-network-core-protocol/blob/4f01e065c1ed346875cf5b05d2b43e0bcdb4c849/rio-sherlock-audit/contracts/restaking/RioLRTCoordinator.sol#L17

https://github.com/sherlock-audit/2024-02-rio-network-core-protocol/blob/4f01e065c1ed346875cf5b05d2b43e0bcdb4c849/rio-sherlock-audit/contracts/restaking/RioLRTDepositPool.sol#L21

https://github.com/sherlock-audit/2024-02-rio-network-core-protocol/blob/4f01e065c1ed346875cf5b05d2b43e0bcdb4c849/rio-sherlock-audit/contracts/restaking/RioLRTIssuer.sol#L22

https://github.com/sherlock-audit/2024-02-rio-network-core-protocol/blob/4f01e065c1ed346875cf5b05d2b43e0bcdb4c849/rio-sherlock-audit/contracts/restaking/RioLRTOperatorRegistry.sol#L23

https://github.com/sherlock-audit/2024-02-rio-network-core-protocol/blob/4f01e065c1ed346875cf5b05d2b43e0bcdb4c849/rio-sherlock-audit/contracts/restaking/RioLRTWithdrawalQueue.sol#L16

All the above Upgradeable contracts do not implement a __gap variable which puts contracts at risk

Impact

Without this variable, it is possible add new variables to the inherited contracts causing storage slot issues, storage collisions. The __gap provides a safeguard against this for upgradeable contracts. 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 funds or cause the contracts to malfunction completely.

Code Snippet

None of these contracts contain storage gap e.g

uint256[50] private __gap;

Tool used

Manual Review

Recommendation

It is recommended to add appropriate storage gap at the end of upgradeable contracts such as the below.

uint256[50] private __gap;

Duplicate of #346