code-423n4 / 2023-12-revolutionprotocol-findings

3 stars 2 forks source link

Lack of Rate Limiting Enables Vote Spamming Denial of Service #119

Closed c4-bot-2 closed 8 months ago

c4-bot-2 commented 8 months ago

Lines of code

https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/revolution/src/CultureIndex.sol#L332-L334 https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/revolution/src/CultureIndex.sol#L332-L334 https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/revolution/src/CultureIndex.sol#L307-L324 https://github.com/code-423n4/2023-12-revolutionprotocol/blob/d42cc62b873a1b2b44f57310f9d4bbfdd875e8d6/packages/revolution/src/CultureIndex.sol#L307-L324

Vulnerability details

Impact

In CultureIndex.sol, we can see: #L332-L334

function vote(uint256 pieceId) public nonReentrant {
    _vote(pieceId, msg.sender);
}

The external vote function is protected by the nonReentrant modifier from OpenZeppelin. This prevents reentrant calls to lock up state.

Additionally, each individual vote call in _vote:

  1. Snapshots past voting power, not current
  2. Checks hasVoted mapping for voter
  3. Updates vote totals and mappings

So every vote call does minimal work.

This leads to a couple natural mitigations against spam:

  1. Attackers can't amplify gas cost through reentrancy
  2. Voting has logarithmic gas cost growth not linear

However, there is no explicit rate limiting logic checking for example msg.sender vote frequency/totals over past blocks.

Proof of Concept

The CultureIndex.sol contract has no limits on the number of votes that can be submitted. This allows an attacker to spam votes and make voting prohibitively expensive.

We can see there is no any explicit rate limiting logic around voting: CultureIndex.sol#L332-L334

function vote(uint256 pieceId) public nonReentrant {
    // No rate limiting check 
    _vote(pieceId, msg.sender);
}

CultureIndex.sol#L307-L324

  function _vote(uint256 pieceId, address voter) internal {
    // Record vote
    // Again no rate limiting check
  }

This remains a vulnerability that could be exploited by a determined attacker to spam votes and block the contract.

The _vote function processes each vote without checking for previous votes by the sender or total votes per block. This allows unlimited votes to be submitted.

An attacker could spam votes from new addresses, costing ~50k gas each. After 100 votes, voting would cost ~5 ETH. After 500 votes, the cost would exceed 50 ETH making voting unusable.

  1. Attacker generates 500 random new addresses
  2. Attacker calls cultureIndex.vote(0) from each address
  3. Total accumulated gas cost makes voting unusable for normal users

The _vote function on lines 307-324 processes votes without limit:

function _vote(uint256 pieceId, address voter) internal {

  // Record every vote   
  votes[pieceId][voter] = Vote({
    weight: weight  
  }); 

  // Update totals
  totalVoteWeights[pieceId] += weight;

}

Tools Used

Vs

Recommended Mitigation Steps

Implement a require check on total votes per block as well as limits based on past activity per address.

Assessed type

Governance

raymondfam commented 8 months ago

All 500 pieces would just be lying on the ground inconsequential to voting.

c4-pre-sort commented 8 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 8 months ago

raymondfam marked the issue as duplicate of #118

c4-judge commented 8 months ago

MarioPoneder marked the issue as unsatisfactory: Insufficient proof