hats-finance / Fenix--0x9d7765a7ebd5b6322a30797a44a5428531970d3d

0 stars 1 forks source link

revealed issues surrounding the adjustments of slopes based on the end times of locked periods for tokens. #39

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

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

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

Description: Description\

The exploitable vulnerability in the provided Solidity code exists in the _checkpoint function, specifically in the method of handling slope updates during overlapping locked periods for the same token (_tokenId). This involves incorrect updates to slope_changes when both old_locked.end and new_locked.end occur at the same future time point, but new_locked is not an extension but potentially shortening or modification of the locked period. This issue is a logical error that leads to incorrect governance weight calculations, which fits the condition of "Manipulation of governance voting results."

Attack Scenario\

  1. Initial Setup: Assume an attacker or a regular user locks some tokens by creating an old_locked with a non-zero end time (old_locked.end) that's set to some future time. Let's say the end is set to a particular future block timestamp and they get a certain governance weight (u_old.slope) due to this.

  2. Triggering the Vulnerability:

    • The user now modifies their locked tokens before old_locked.end using the new_locked with new_locked.end equal to old_locked.end but with different parameters (e.g., reduced amount, same or less locking period than originally locked). This action is intended to change the conditions initially set with old_locked.
  3. Incorrect Slope Adjustment:

    • As per the control flow in _checkpoint, the function neglects to correctly adjust the slope_changes for situations where new_locked.end matches old_locked.end but is not merely an extension (i.e., it alters parameters significantly).
    • The slopes for both new and old locked periods are read and potentially modified. However, since they are treated as an extension in the code (old_dslope -= u_new.slope; only if old_locked.end doesn't match new_locked.end), this leads to incorrect slope calculation where the new conditions aren't properly accounted for, maintaining higher weight erroneously.

Attachments

  1. Proof of Concept (PoC) File

  2. Revised Code File (Optional)

BohdanHrytsak commented 1 month ago

POC is needed, as there are inconsistencies and lack of clarity on the ultimate benefit of the user, how much more voting power will be, etc. and in which cases this will actually occur Also, there is incomprehensibility (e.g., reduced amount, same or less locking period than originally locked) - it can only increase the locking time or the amount or using permanent lock, but then neglects these things and calculations

0xmahdirostami commented 1 month ago

Please provide Poc