Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

0xpetern - No storage gap for upgradeable contract could lead to storage slot collision #324

Open
sherlock-admin2 opened this issue Oct 29, 2024 · 0 comments

Comments

@sherlock-admin2
Copy link
Contributor

sherlock-admin2 commented Oct 29, 2024

0xpetern

Medium

No storage gap for upgradeable contract could lead to storage slot collision

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 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.
Refer to the bottom part of this article: https://docs.openzeppelin.com/upgrades-plugins/1.x/writing-upgradeable

Upgradeable contracts should have storage gaps to avoid storage slot collision. The absence of storage gap in upgradeable contracts of the protocol can lead to storage slot collision.

Root Cause

https://github.com/sherlock-audit/2024-10-axion/blob/main/liquidity-amo/contracts/BoostStablecoin.sol#L19

https://github.com/sherlock-audit/2024-10-axion/blob/main/liquidity-amo/contracts/MasterAMO.sol#L22
https://github.com/sherlock-audit/2024-10-axion/blob/main/liquidity-amo/contracts/Minter.sol#L12

Internal pre-conditions

No storage ga.

The contract doesn't contain a storage gap. 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". See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps for a description of this storage variable. While some contracts may not currently be sub-classed, adding the variable now protects against forgetting to add it in the future.

External pre-conditions

No response

Attack Path

No response

Impact

Leads to storage slot collision.

PoC

No response

Mitigation

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

@sherlock-admin2 sherlock-admin2 changed the title Creamy Sandstone Ram - No storage gap for upgradeable contract could lead to storage slot collision 0xpetern - No storage gap for upgradeable contract could lead to storage slot collision Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant