hats-finance / Tapioca-0xe0b920d38a0900af3bab7ff0ca0af554129f54ad

4 stars 4 forks source link

`cumulative` is being inflated in some cases which would lead to the duration also being inflated #17

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

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

Github username: @bauchibred Twitter username: bauchibred Submission hash (on-chain): 0x9df352339572ff2c6821e84a50272f143f98010ef82033b8607fbd784c1ef574 Severity: medium

Description: Description

Take a look at how the new cummulative is set here via twTAP#participate() https://github.com/hats-finance/Tapioca-0xe0b920d38a0900af3bab7ff0ca0af554129f54ad/blob/dev/contracts/governance/twTAP.sol#L376

            // Compute and save new cumulative
            divergenceForce = _duration >= pool.cumulative;

From the whitepaper, however, the formula should be: https://www.tapioca.xyz/docs/twAML.pdf (see equation 5 from page 5)

Which showcases how this check should be strictly > and not >= when getting the divergenceForce, so in this case it should be _duration > pool.cumulative;.

That's to say when_duration = pool.cumulative, the effect should be a reduction, but the current implementation wrongly results in an increase instead.

This then leads to an overestimation of cumulative, allowing for a longer range of _duration which would cause for lock in longer time periods.

That's because Maximum Lock Time is 4x the current AML (Average Magnitude Lock). Therefore, if the current AML (consensus of user's locks) is one epoch (one week), the max lock time will be four epochs (four weeks). This was implemented in order to deter for example a user locking for for 10,000 epochs (191 years), which would steer the AML to an extremely high level which would effectively DoS (Denial of Service) twAML. Attack Scenario N/A Recommendation Apply these changes to participate()

+           divergenceForce = _duration > pool.cumulative;
-           divergenceForce = _duration >= pool.cumulative;

Attachments

  1. Proof of Concept (PoC) File

N/A, However for more info see this, which was not so accurately duplicated and whose fixed was missed in this pull request.

  1. Revised Code File (Optional)

N/A

0xRektora commented 5 months ago

As mentioned in the past audit, this is gonna be judged as a QA, which are not taken into account for this audit.