hats-finance / Fenix-Finance-0x83dbe5aa378f3ce160ed084daf85f621289fb92f

0 stars 0 forks source link

Voter in VotingEscrowUpgradeable can permanently lock user tokens #38

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

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

Github username: -- Twitter username: -- Submission hash (on-chain): 0xf1f4a6ccadb68120fa9cd908ebd0a936bfd28fa7ba79d92d8f9748dacdba3b2f Severity: medium

Description: Context:\ VotingEscrowUpgradeable.sol#L1027 VotingEscrowUpgradeable.sol#L1037 VotingEscrowUpgradeable.sol#L1048 VotingEscrowUpgradeable.sol#L283 VotingEscrowUpgradeable.sol#L808

Description\ A malicious voter can arbitrarily increase the number of attachments or set the voted status of a token to true. This prevents the token from being withdrawn, merged or transfered thereby locking the tokens into the contract for as long as the voter would like.

I submitted this as a medium severity because it has external circumstances (a malicious voter) however has a very high impact if it does occur.

  1. Proof of Concept (PoC)
    • A user creates a lock for their token and deposits it into the VotingEscrowUpgradeable contract.
  1. Recommendation: Consider removing voting() and attach() altogether. On the other hand, keeping the voter not malicious will resolve the problem.
BohdanHrytsak commented 7 months ago

Thank you for the submission.

I admit that this line is puzzling: https://github.com/Satsyxbt/Fenix/blob/7b81d318fd9ef6107a528b6bd49bb9383e1b52ab/contracts/core/VotingEscrowUpgradeable.sol#L105C1-L106C1

but in fact, voter will be and should be VoterUpgradeable

voting(), attach() methods are needed to properly regulate the voting process and protect against certain issues

Setted the correct voter is a Centralized Risk that is OOS