Lucene search

K
code423n4Code4renaCODE423N4:2022-08-RIGOR-FINDINGS-ISSUES-321
HistoryAug 06, 2022 - 12:00 a.m.

No storage gap for Upgradable contract might lead to storage slot collision

2022-08-0600:00:00
Code4rena
github.com
10
upgradable contract
storage gap
storage slot collision
contract malfunction
openzeppelin
vulnerability

Lines of code
<https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/ProjectFactory.sol#L19&gt;
<https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/HomeFiProxy.sol#L14&gt;
<https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/HomeFi.sol#L29-L31&gt;
<https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Disputes.sol#L19-L20&gt;
<https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Project.sol#L26-L27&gt;
<https://github.com/code-423n4/2022-08-rigor/blob/b17b2a11d04289f9e927c71703b42771dd7b86a4/contracts/Community.sol#L23-L25&gt;

Vulnerability details

Impact

For Upgradable 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.

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

Proof of Concept

Several contracts are intended to be upgradeable contracts in the code base, including

  • DebtToken
  • ProjectFactory
  • HomeFiProxy
  • HomeFi
  • Disputes
  • Project
  • Community

However, none of these contracts contain 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”. Refer to the bottom part of this article:

<https://docs.openzeppelin.com/contracts/3.x/upgradeable&gt;

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;

The text was updated successfully, but these errors were encountered:

👀 1 horsefacts reacted with eyes emoji

All reactions

  • 👀 1 reaction