code-423n4 / 2024-03-saltyio-mitigation-findings

0 stars 0 forks source link

Voting mechanism heavily disfavours the `Yes` voters due to transparency of each vote #93

Open c4-bot-5 opened 4 months ago

c4-bot-5 commented 4 months ago

Lines of code

https://github.com/othernet-global/salty-io/blob/main/src/dao/Proposals.sol#L410

Vulnerability details

Summary

The current proposal approval mechanism is:

This works heavily in favour of those who do not want to pass the proposal. They will simply NOT cast their vote unless they see that the Yes votes are nearing the required quorum count. If the Yes votes are not able to achieve quorum on their own, the proposal is going to fail anyway. It makes no sense for those against the proposal to help reach quorum by casting their votes in advance.

The current mechanism works nicely for a secret ballot system - like in the real world election process where each vote is kept secret until the final reveal. In some countries & forms of elections, if the total number of votes cast is less than 10 or 20% of the population in that constituency, then the election results are scrapped (akin to reaching a minimum required quorum). The current code logic works only for secret ballots OR a time-bound election with no minimum-quorum. The current protocol design is neither of those.

Details

The incentive for a voter to cast their vote is when they feel the other side may win. Anything they can do to put the other side at a disadvantage is helpful. Consider the above example of a real-world election. If everyone knows that a minimum 10% of the population needs to cast their vote AND THEN the new candidate has to get more votes in order to replace the current candidate AND that the election results will be declared AS SOON AS quorum is reached, then the following situation emerges -

This heavily favours those already in power and is not considered a fair election. This is one of the reasons, among others, why each vote is kept secret until the end. Due to the secrecy, everyone participates from the start in order to avoid the other side from winning, making it a fair & healthy election.

This very approach is missing in the code logic. This issue was present both in the old as well as new implementation. However it was a lot riskier for No voters to use this approach & abstain from voting because there existed the possibility that the quorum value may change suddenly if someone unstakes and hence resulting in a sudden win of Yes voters. In contrast, in the new implementation since requiredQuorum is saved right at the time of proposal creation, it makes it a lot more safer now to use this tactic.

Proof of Concept

Just some sample numbers to make the above description clearer -

Recommended Mitigation Steps

The following measures need to be taken:

Assessed type

Other

Picodes commented 4 months ago

This is usually mitigated by enforcing the quorum only on the number of "yes" votes.

Picodes commented 4 months ago

I understand how the design isn't optimal but on first read fail to see how this qualifies for Medium severity.

The implications of this are that if the number of proposal supporters is somewhere between 0.5 quorum and quorum you don't know for sure if the proposal will pass or not, which isn't really a security issue

t0x1cC0de commented 4 months ago

Hey @Picodes, This is not totally accurate:

The implications of this are that if the number of proposal supporters is somewhere between 0.5 quorum and quorum you don't know for sure if the proposal will pass or not, which isn't really a security issue

In reality, if the number of Yes is less than quorum (or as you put it, somewhere between 0.5 quorum and quorum), it's a given that they will lose. I won't use fancy terms like "game theory", but it's common sense for No voters to only vote when they believe Yes can reach quorum. Ideally, they will front-run the last transaction capable of pushing Yes over the quorum line in order to cast their No votes. Practically, one can expect that they would all vote quickly as soon as they see Yes being very close to the quorum number.

This becomes an issue now because the protocol is essentially asking Yes voters to reach quorum on their own if they wish to win. This is completely different from the intended goal.

othernet-global commented 4 months ago

Allowing the yes votes to determine quorum in the absence of all no votes is acceptable (although realistically not all NO votes would abstain). If the DAO sees NO votes not being cast regularly, it can reduce the quorum as needed.

c4-sponsor commented 4 months ago

othernet-global (sponsor) disputed

t0x1cC0de commented 4 months ago

Allowing the yes votes to determine quorum in the absence of all no votes is acceptable (although realistically not all NO votes would abstain). If the DAO sees NO votes not being cast regularly, it can reduce the quorum as needed.

Thank you for your explanation Daniel. The only thing I will point out before a decision is made, is that the protocol is not simply "Allowing the yes votes to determine quorum", but rather "forcing them to reach quite close to quorum on their own".

This isn't an edge case but rather the whole dynamics of deciding winners has changed markedly, very much against those in a proposal's favour.

Picodes commented 4 months ago

In reality, if the number of Yes is less than quorum (or as you put it, somewhere between 0.5 quorum and quorum), it's a given that they will lose.

From a game theory standpoint supporters of No shouldn't vote as you showed, but only if they have perfect information, which isn't the case here. So no it's not "a given that they will lose" even from a purely game theory standpoint.

Likely, some will still vote because, for example, they don't know for sure how many supporters of "No" are out there so may still want to cast their votes to show their intent and motivate other supporters of "No" to vote as well. They also have no way to guess in advance the real number of supporters of the "Yes" cause so they don't know if they can pass the quorum independently. So your reasoning isn't fully valid.

We could argue as well that from a game theory standpoint, no one should vote before the last moment to avoid revealing its intent but we know in practice people don't do this.

So the impact is that if the number of supporters of "Yes" is somewhere between 0.5 quorum and quorum the success of the proposal is "random" and depends on the reasonings of the supporters of "No".

Now, on the final impact on this, recall that the quorum is only here to ensure a minimum level of support for proposals that pass and to avoid proposals where nobody votes to pass. The risk is that the quorum is too high and everything is frozen because no proposal can make it. Here, this doesn't impact these 2 functionalities so I think Low severity is appropriate.

c4-judge commented 4 months ago

Picodes changed the severity to QA (Quality Assurance)