code-423n4 / 2022-09-party-findings

2 stars 0 forks source link

[NAZ-M1] No Storage Gap for Upgradeable Contract Might Lead to Storage Slot Collision #281

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/PartyDAO/party-contracts-c4/blob/main/contracts/proposals/ProposalStorage.sol#L1 https://github.com/PartyDAO/party-contracts-c4/blob/main/contracts/globals/Globals.sol#L1

Vulnerability details

Impact

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" (quote OpenZeppelin). 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, potentially causing loss of user fund or cause the contract to malfunction completely.

Proof of Concept

Refer to the bottom part of this article and the bottom part of this article.

Tools Used

Manual Review

Recommended Mitigation Steps

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;
merklejerk commented 1 year ago

Duplicate of #96

HardlyDifficult commented 1 year ago

See dupe for context. Closing as invalid.