hats-finance / Smooth-0x64bc275b37e62eec81a00ecaecd2b9567058f990

Dappnode's MEV Smoothing Pool
0 stars 2 forks source link

Unchecked Quorum Reduction Fails to Consider Current State #18

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

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

Github username: @0xmahdirostami Submission hash (on-chain): 0xa6592499766f7ab911916c2e19b65c141099104b707c98d3820285734b24966e Severity: medium

Description: Description\ In the governance system, there's a way to decrease the quorum using the updateQuorum function. However, this function has a vulnerability. It doesn't check the current state of the VotedReportHash.

Scenario\ Consider the following scenario to understand the issue:

Impact\ I've described one scenario, but there could be more. The impact of this vulnerability is that decreasing the quorum doesn't lead to an update of the state root, even if the current VotedReportHash.votes is higher than the new quorum.

Attachments\ Revised Code File (Optional)

To address this vulnerability, a new variable, let's call it currentReportHash, needs to be introduced. We should also check its vote count in the updateQuorum function. If the new quorum is lower than the current quorum and the vote count is sufficient, then update the state root.

This change ensures that the state gets updated when the current vote count permits it.

invocamanman commented 10 months ago

I implemented this way because:

Since the current system can handle this situation, is a very edge case, and it adds complexity and gas cost to all calls i think that the current implementation is better.

This issue tho, does not block the system in any way, or steal funds, for instance, i would lower it to an informational