search

hats-finance / Fenix--0x9d7765a7ebd5b6322a30797a44a5428531970d3d

0 stars 1 forks source link

Gap is implemented wrongly #18

Open hats-bug-reporter[bot] opened 2 months ago

hats-bug-reporter[bot] commented 2 months ago

Github username: @0x3b Twitter username: 0x3b33 Submission hash (on-chain): 0x201d52d94857f6c106e2e70b20a2a72129e2b84cebbafedeebeb9111778378df Severity: medium

Description: Description BaseManagedNFTStrategyUpgradeable's __gap variable is implemented wrongly and it does not follow the OZ standart.

Attack Scenario THe storage size of each contract must be 50. OZ has desribed in their docs - https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps

The size of the __gap array is calculated so that the amount of storage used by a contract always adds up to the same number (in this case 50 storage slots).

However the current storage size is more than 50 as we have a few veriables and then 

uint256[50] private __gap;

Adding any new variables may brick the storage of the contracts that inherit BaseManagedNFTStrategyUpgradeable

Recommendation Calculate the __gap by making sure the whole contract has 50 storage slots and the next contract (the one that is inheriting from BaseManagedNFTStrategyUpgradeable) storage slot starts at 51.

0xmahdirostami commented 2 months ago

Why did you label it as a medium-level issue?

Best practice issue.