Skip to content
This repository has been archived by the owner on May 26, 2023. It is now read-only.

Tomo - [Tomo-M6] Attackers can increase voting power by incentivizing #74

Closed
github-actions bot opened this issue Nov 16, 2022 · 5 comments
Closed

Comments

@github-actions
Copy link

Tomo

medium

[Tomo-M6] Attackers can increase voting power by incentivizing

Summary

Attackers can increase voting power by incentivizing

Vulnerability Detail

There are a lot of cases of used bribes and incentives, governance to skewed to act against the interest of the founders

This is a good example of this issue

https://tribe.fei.money/t/fully-repay-fuse-bad-debt/4528

https://dialectic.ch/editorial/nouns-governance-attack-2

https://halborn.com/explained-the-beanstalk-hack-april-2022/

It is true that a properly aligned Vetoer will be able to prevent most of these attacks.

However, if attackers get a lot of votes, it is possible to vote not to let all proposals be executed, and the real governance system collapses

Impact

The proposal decision depends on the attacker

Code Snippet

https://github.com/sherlock-audit/2022-11-frankendao/blob/main/src/Staking.sol#L266-L281

/// @notice Delegate votes to another address
/// @param _delegatee The address you wish to delegate to
/// @dev Refunds gas if delegatingRefund is true and hasn't been used by this user in the past 24 hours
function delegate(address _delegatee) public {
  if (_delegatee == address(0)) _delegatee = msg.sender;
  
  // Refunds gas if delegatingRefund is true and hasn't been used by this user in the past 24 hours
  if (delegatingRefund && lastDelegatingRefund[msg.sender] + refundCooldown <= block.timestamp) {
    uint256 startGas = gasleft();
    _delegate(msg.sender, _delegatee);
    lastDelegatingRefund[msg.sender] = block.timestamp;
    _refundGas(startGas);
  } else {
    _delegate(msg.sender, _delegatee);
  }
}

https://github.com/sherlock-audit/2022-11-frankendao/blob/main/src/Staking.sol#L495-L501

/// @notice Get the total voting power (token + community) for an account
/// @param _account The address of the account to get voting power for
/// @return The total voting power for the account
/// @dev This is used by governance to calculate the voting power of an account
function getVotes(address _account) public view returns (uint) {
    return tokenVotingPower[_account] + getCommunityVotingPower(_account);
}

https://github.com/sherlock-audit/2022-11-frankendao/blob/main/src/Governance.sol#L586-L600

/// @notice Cast a vote for a proposal
/// @param _proposalId The id of the proposal to vote on
/// @param _support The support value for the vote (0=against, 1=for, 2=abstain)
function castVote(uint256 _proposalId, uint8 _support) external {
    // Refunds gas if votingRefund is true
    if (votingRefund) {
        uint256 startGas = gasleft();
        uint votes = _castVote(msg.sender, _proposalId, _support);
        emit VoteCast( msg.sender, _proposalId, _support, votes);
        _refundGas(startGas);
    } else {
        uint votes = _castVote(msg.sender, _proposalId, _support);
        emit VoteCast( msg.sender, _proposalId, _support, votes);
    }
}

Tool used

Manual Review

Recommendation

Implement the following to solve this problem

  • For a certain period of time, the return value of getVote() and getCommunityVotingPower() would be zero for the blacklist accounts.
  • For a certain period of time, the transaction of stake() and delegate() would return revert for the blacklist tokenId.
@zobront
Copy link

zobront commented Nov 17, 2022

We implemented veto power to ensure none of these attacks are issues.

@Evert0x Evert0x closed this as completed Nov 17, 2022
@Tomosuke0930
Copy link

Escalate for 1 USDC

It is true that a properly aligned Vetoer will be able to prevent most of these attacks.
However, if attackers get a lot of votes, it is possible to vote not to let all proposals be executed, and the real governance system collapses

Ref: Vulnerability Detail above

I understand what you said but, you can't be any denial about the fact that the attacker will cause all proposals to fail, effectively destroying the voting system.

Therefore, I offered a way to prevent that attack.

Also, this is an example from Code4rena, but I think it's enough things to prove this bug is a valid one.
code-423n4/2022-09-nouns-builder-findings#479 (comment)

@sherlock-admin
Copy link
Contributor

Escalate for 1 USDC

It is true that a properly aligned Vetoer will be able to prevent most of these attacks.
However, if attackers get a lot of votes, it is possible to vote not to let all proposals be executed, and the real governance system collapses

Ref: Vulnerability Detail above

I understand what you said but, you can't be any denial about the fact that the attacker will cause all proposals to fail, effectively destroying the voting system.

Therefore, I offered a way to prevent that attack.

Also, this is an example from Code4rena, but I think it's enough things to prove this bug is a valid one.
code-423n4/2022-09-nouns-builder-findings#479 (comment)

You've created a valid escalation for 1 USDC!

To remove the escalation from consideration: Delete your comment.
To change the amount you've staked on this escalation: Edit your comment (do not create a new comment).

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

@Evert0x
Copy link
Contributor

Evert0x commented Nov 28, 2022

Escalation rejected.

This is a known risk of token voting.

@sherlock-admin
Copy link
Contributor

Escalation rejected.

This is a known risk of token voting.

This issue's escalations have been rejected!

Watsons who escalated this issue will have their escalation amount deducted from their next payout.

@Evert0x Evert0x removed the Escalated label Nov 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants