search

code-423n4 / 2024-07-reserve-findings

5 stars 4 forks source link

Delegation to the `zero address` can lead to permanent loss of the user's voting power #98

Closed howlbot-integration[bot] closed 3 months ago

howlbot-integration[bot] commented 3 months ago

Lines of code

https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/StRSRVotes.sol#L166-L183 https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/StRSRVotes.sol#L230-L254 https://github.com/code-423n4/2024-07-reserve/blob/3f133997e186465f4904553b0f8e86ecb7bbacbf/contracts/p1/StRSRVotes.sol#L211-L218

Vulnerability details

Summary

Do Note that this vulnerability was highly inspired by this recent finding -

The StRSRVotes contract contains a similar vulnerability to the one described in the Nouns NFT scenario. The issue lies in the _moveVotingPower function, which is called during token transfers and delegations.

We have identified a critical vulnerability in the StRSRP1Votes contract. When delegateBySig is called, potential delegation to the zero address can lead to permanent loss of the user's voting power.

Users are no longer able to participate in the voting decisions When this function is used to delegate to address zero, the delegator wallet loses all its voting power and can no longer transfer their votes out of that wallet.

Vulnerability Details

The_moveVotingPower function allows delegation to address(0), which can lead to permanent loss of voting power.

The issue occurs when an StRSR owner delegates their voting power to an EOA. If the delegate, whether intentionally or unintentionally, uses the delegateBySig function to delegate to the zero address, one unit of voting power is permanently burned. This results in the associated losing its voting power and becoming non-transferrable. The transfer attempt will revert due to an underflow error when the contract attempts to reduce the sender's already zero voting power. When delegating to address(0), voting power is subtracted from the source but not added to any destination, effectively burning it.

The _afterTokenTransfer function calls _moveVotingPower, potentially causing issues during transfers if a user's voting power has been compromised. The stakeAndDelegate function allows users to stake and delegate in one transaction, potentially exacerbating the issue if misused.

Impact:

Voting Power Loss: The affected user loses its voting power permanently, reducing its utility in governance processes.

Proof of Concept:

  1. Setup: Alice owns StRSR 1. Bob owns StRSR 2.

  2. Delegation: Alice delegates her voting power to Bob (Alice votes: 0; Bob votes: 2).

  3. Faulty Delegation: Bob uses delegateBySig to delegate his voting power to the zero address (0x0000...). One vote is permanently burned (Alice votes: 0; Bob votes: 1).

  4. Transfer: Bob transfers StRSR 2 to a new wallet, Bobby (Alice votes: 0; Bob votes: 0; Bobby votes: 1).

  5. Zombie Noun: Alice's StRSR 1 becomes a zombie: it has no voting power and cannot be transferred out of Alice's wallet due to an underflow error.

Root Cause: The root cause lies in the StRSRP1Votes contract's _moveDelegates function, which handles vote delegation and transfer operations. The function fails to account for the scenario where voting power is delegated to the zero address. Specifically, when delegateBySig is used to delegate to the zero address, the function deducts voting power from the sender but does not reassign it to any recipient, leading to an irrecoverable loss of voting power. Additionally, this causes an underflow error during subsequent transfer attempts.

Recommended Mitigation:

Assessed type

call/delegatecall

c4-judge commented 2 months ago

thereksfour marked the issue as unsatisfactory: Invalid

Joshuajee commented 2 months ago

This issue is valid delegating votes to address zero will lead to a permanent loss of voting power, this is a similar finding. https://code4rena.com/reports/2022-08-nounsdao#h-01-erc721checkpointable-delegatebysig-allows-the-user-to-vote-to-address-0-which-causes-the-user-to-permanently-lose-his-vote-and-cannot-transfer-his-nft

This is the POC, add it to ZZstRSR.test.ts.

it.only('Delegate to user2 then user2 delegate to address 0', async function () {
      // Should perform basic validations on stake
      const amount1: BigNumber = bn('50e18')
      await rsr.connect(addr1).approve(stRSRVotes.address, amount1)
      await stRSRVotes.connect(addr1).stake(amount1)

      await stRSRVotes.connect(addr1).delegate(addr2.address)
      // Mine block
      await advanceTime(1)
      expect(await stRSRVotes.delegates(addr1.address)).to.equal(addr2.address)

      // Advance block
      await advanceBlocks(1)

      // Check current votes
      let user1votes = await stRSRVotes.getVotes(addr1.address)
      console.log("user1 votes", user1votes)

      // Advance time
      await advanceTime(1)

      // User 2 delegates to address 0
      const Delegation = [
        { name: 'delegatee', type: 'address' },
        { name: 'nonce', type: 'uint256' },
        { name: 'expiry', type: 'uint256' },
      ]

      const nonce = await stRSRVotes.nonces(addr1.address)
      const expiry = MAX_UINT256
      const chainId = await getChainId(hre)
      const name = await [http://stRSRVotes.name](https://t.co/yo16sCmMr1)()
      const verifyingContract = stRSRVotes.address

      // Get data
      const buildData = {
        types: { Delegation },
        domain: { name, version: VERSION, chainId, verifyingContract },
        message: {
          delegatee: addr1.address,
          nonce,
          expiry,
        },
      }

      // Get data
      const sig = await ZERO_ADDRESS._signTypedData(buildData.domain, buildData.types, buildData.message)
      const { v, r, s } = ethers.utils.splitSignature(sig)

      // Change delegate for addr1 using signature
      await rsr.connect(addr2).approve(stRSRVotes.address, amount1)
      await stRSRVotes.connect(addr2).delegateBySig(ZERO_ADDRESS, nonce, expiry, v, r, s)

      let user2votes = await stRSRVotes.getVotes(addr2.address)
      console.log("user2 votes", user2votes)

    })
thereksfour commented 2 months ago

See here: https://github.com/code-423n4/2024-07-reserve-findings/issues/93#issuecomment-2314711014 And your POC doesn't seem to work, it reverts at ZERO_ADDRESS._signTypedData.