hats-finance / Fenix--0x9d7765a7ebd5b6322a30797a44a5428531970d3d

0 stars 1 forks source link

Incorrect use of __gap[] #16

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

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

Github username: @lanrebayode Twitter username: lanrebayode1 Submission hash (on-chain): 0xbaed60c1f36333df78bfaeed3c7547a846ac52508787436b6339111332b6bd26 Severity: low

Description: Description\ Storage gap us used to prevent slot collision in upgradable contract as specified by openzeplin.

However it was not used correctly in https://github.com/Satsyxbt/Fenix/blob/353c8e8e24454336e805e5c0e11e4e9ae1491d03/contracts/core/VotingEscrowUpgradeableV1_2.sol#L1273 as specified in OZ docs

The array length plus the already used storage slot should add up to 50

https://docs.openzeppelin.com/contracts/3.x/upgradeable#:~:text=The%20size%20of%20the%20__gap%20array%20is%20calculated%20so%20that%20the%20amount%20of%20storage%20used%20by%20a%20contract%20always%20adds%20up%20to%20the%20same%20number%20(in%20this%20case%2050%20storage%20slots).

Attack Scenario\ Describe how the vulnerability can be exploited.

Attachments

  1. Proof of Concept (PoC) File

    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).

https://github.com/Satsyxbt/Fenix/blob/353c8e8e24454336e805e5c0e11e4e9ae1491d03/contracts/core/VotingEscrowUpgradeableV1_2.sol#L1273

  1. Revised Code File (Optional) the gap size should be updated with 50 - usedSlots
0xmahdirostami commented 2 months ago

OOS, Invalid

lanrebayode commented 2 months ago

@0xmahdirostami thanks for the feedback while the submission might have referenced an OOS contract, the issue persist in other part of the codebase that implements OZ upgradable contracat.

e.g: https://github.com/Satsyxbt/Fenix/blob/353c8e8e24454336e805e5c0e11e4e9ae1491d03/contracts/nest/SingelTokenVirtualRewarderUpgradeable.sol

0xmahdirostami commented 1 month ago

for now, I will judge these submissions as well (not being OOS)

non-issue, Best practice issue.

what is impact here?