Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Attackers can increase voting power by incentivizing #479

Open
code423n4 opened this issue Sep 15, 2022 · 4 comments
Open

Attackers can increase voting power by incentivizing #479

code423n4 opened this issue Sep 15, 2022 · 4 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working edited-by-warden sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue

Comments

@code423n4
Copy link
Contributor

code423n4 commented Sep 15, 2022

Lines of code

https://github.com/code-423n4/2022-09-nouns-builder/blob/main/src/governance/governor/Governor.sol#L248-L297

Vulnerability details

馃帹聽Category

Governance

馃挜聽Impact

If the benefit to be gained from the outcome of the vote is less than the cost of obtaining the right to vote, the outcome of the vote is influenced

馃摑聽Proof of Concept

https://github.com/code-423n4/2022-09-nouns-builder/blob/main/src/governance/governor/Governor.sol#L248-L297
Vampire Attack Explained
https://finematics.com/vampire-attack-sushiswap-explained/

馃殰聽Tools Used

Manual

鉁吢燫ecommended Mitigation Steps

  • Add blacklist check for _castVote function.
if(isBlacklisted(msg.sender) revert BLACKLISTED();
  • Create a function to delete the voting power and add the address to the blacklist.
function addBlackList(address _blackList) external onlyOwner {
		isBlackListed[_blackList] = true;
		emit Blacklisted(_blackList);
}

function deleteVotingPower(address _blackListed,uint _weight, uint _support) external onlyOwner {
    if (_support > 2) revert INVALID_VOTE();
    require(hasVoted[_proposalId][_blackListed],"NO VOTING BY BLACKLISTED");
    if (_support == 0) {
      proposal.againstVotes -= _weight;
  } else if (_support == 1) {
      proposal.forVotes -= _weight;
  } else if (_support == 2) {
      // Update the total number of votes abstaining
      proposal.abstainVotes -= _weight;
  }
}

馃懍聽Similar Issue

code-423n4/2022-05-aura-findings#278

@code423n4 code423n4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Sep 15, 2022
code423n4 added a commit that referenced this issue Sep 15, 2022
@GalloDaSballo
Copy link
Collaborator

Nice idea, not sure if there's any way to avoid this

Allowlist would cause centralization issue, and would be defeated by the bribes as well

@GalloDaSballo GalloDaSballo added the sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons label Sep 19, 2022
@GalloDaSballo
Copy link
Collaborator

While I think the finding can be further developed. I have to concede that via bribes and incentives, governance can be skewed to act against the interest of the founders

Whether this is good or bad per see, is not that relevant, however the system does allow delegation to a "malicious party" which would be able to obtain enough votes to do whatever they want.

Notice that the veto system prevents this as well, so we could argue that removing Vetoing allows this, which is effectively another take on a 51% attack.

Leaving as unique for the thought-provoking idea

@kulkarohan
Copy link
Collaborator

This is why veto power is granted to founders. Not an issue IMO and would introduce needless complexity + centralization going the blacklist route.

@kulkarohan kulkarohan added the sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue label Sep 28, 2022
@GalloDaSballo
Copy link
Collaborator

Per the discussion above, taking into consideration that CodeArena is a end-user facing project, meaning our reports could be used to determine if a project is safe or not;

Given the recent example of large scale bribes to influence a voting outcome

While disagreeing as well with the solution of offering a Blocklist to prevent malicious actor, and agreeing that the finding should have been better written.

I have to rationally concede that the ability to bribe governance can be used to extract value from the Treasury, a Vetoer may or may not offer protection at that time due to second order social consequences.

Because of that, given the rules (Loss of Value conditional on externalities), given the precedents (one linked above, I'm sure there's many other), I still believe that this is a valid Medium Severity finding.

Per the comment by the sponsor, a properly aligned Vetoer will be able to prevent most of these attacks, however I don't think that's sufficient to make the finding invalid

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working edited-by-warden sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue
Projects
None yet
Development

No branches or pull requests

3 participants