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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Governance Voting Dis-proportionally Favours Users Who Stake And Vote After A Poll Has Been Created And Had Its Snapshot Taken #64

Open
code423n4 opened this issue Mar 9, 2022 · 2 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 disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-02-anchor/blob/main/contracts/anchor-token-contracts/contracts/gov/src/contract.rs#L543-L580
https://github.com/code-423n4/2022-02-anchor/blob/main/contracts/anchor-token-contracts/contracts/gov/src/contract.rs#L582-L665
https://github.com/code-423n4/2022-02-anchor/blob/main/contracts/anchor-token-contracts/contracts/gov/src/staking.rs#L15-L57
https://github.com/code-423n4/2022-02-anchor/blob/main/contracts/anchor-token-contracts/contracts/gov/src/contract.rs#L364-L455

Vulnerability details

Impact

Polls are created by targeting the receive_cw20 function which is queried whenever the contract receives tokens. By setting the hook message to Cw20HookMsg::CreatePoll, the sender is able to create a poll, assuming the amount sent satisfies the minimum deposit amount for poll creation. Users can also choose to call ExecuteMsg::SnapshotPoll or have it handled automatically when a user casts a vote on the newly created poll.

The snapshot simply sets a_poll.staked_amount, which represents the total staked amount within the governance contract at a given block. However, during the voting period, other users can stake tokens and effectively have an increasing influence over the outcome of a given poll. There are no check-pointed balances to ensure that a certain user had staked tokens at the time the poll had its snapshot taken.

This can be abused to skew poll results in favour of users who stake their Anchor tokens after a poll has had its snapshot taken.

Proof of Concept

Let's assume the share to token exchange rate is 1:1 such that if a user deposits 100 Anchor tokens, they receive 100 shares in return.

Consider the following scenario:

  • There are a total of 100 Anchor tokens in the Governance contract.
  • Alice creates a poll and executes ExecuteMsg::SnapshotPoll such that a_poll.staked_amount == 100.
  • Bob deposits 10 Anchor tokens through the Cw20HookMsg::StakeVotingTokens hook message which increases the contract's total balance to 110 and shares to 110 as the exchange rate is 1:1 upon minting and redeeming shares.
  • At this point, the target poll has a a_poll.staked_amount == 100, even though there are really 110 Anchor tokens staked.
  • As a result, if Bob votes on a poll, they have a 10% degree of influence on the outcome of the poll, even though they have less than 10% of the total staked tokens (i.e. 10/110).
  • Therefore, poll voters are actually incentivised to stake tokens after a poll has had its snapshot taken in order to maximise their voting power.

Tools Used

Manual code review.

Recommended Mitigation Steps

Consider implementing a check-pointing mechanism such that when a user casts a vote, the user's staked balance is checked at the block height upon which the snapshot was taken instead of checking its most up-to-date staked balance. This check-pointing behaviour is implemented on Ethereum which has a more restrictive block space. The mechanism will simply store the staker's balance on each stake/unstake action. When user's wish to vote, the protocol will check the balance at a specific block (i.e. the snapshotted block). An example implementation can be found here.

@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 Mar 9, 2022
code423n4 added a commit that referenced this issue Mar 9, 2022
@bitn8 bitn8 added the disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) label Apr 19, 2022
@bitn8
Copy link

bitn8 commented Apr 19, 2022

I wouldn't call this a critical bug. Nevertheless, this will be addressed with ve-ANC tokenomics where tokens have to lock for periods of time (curve lock tokenomics).

@GalloDaSballo GalloDaSballo added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Aug 6, 2022
@GalloDaSballo
Copy link
Collaborator

Loss of Yield = Med seems appropriate

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 disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Development

No branches or pull requests

3 participants