The quorumVotes can be bypassed

mediumCode4rena

Lines of code

https://github.com/code-423n4/2023-12-revolutionprotocol/blob/08ff070da420e95d7c7ddf9d068cbf54433101c4/packages/revolution/src/CultureIndex.sol#L209-L234

Vulnerability details

Impact

This vulnerability allows for the minting and auctioning of an art piece that has not met the required quorum. It enables malicious voters to influence outcomes with fewer votes than what is stipulated by the protocol. This undermines a key invariant of the protocol:

    An art piece that has not met quorum cannot be dropped.

https://github.com/code-423n4/2023-12-revolutionprotocol/blob/08ff070da420e95d7c7ddf9d068cbf54433101c4/README.md?plain=1#L291

Proof of Concept

The quorumVotes for an art piece are calculated at its creation as a fraction of the totalVotesSupply, which depends on the total supply of erc20VotingToken and erc721VotingToken:

    (quorumVotesBPS * newPiece.totalVotesSupply) / 10_000.

javascript📋
File: src/CultureIndex.sol
209:     function createPiece(
210:         ArtPieceMetadata calldata metadata,
211:         CreatorBps[] calldata creatorArray
212:     ) public returns (uint256) {
213:         uint256 creatorArrayLength = validateCreatorsArray(creatorArray);
214: 
215:         // Validate the media type and associated data
216:         validateMediaType(metadata);
217: 
218:         uint256 pieceId = _currentPieceId++;
219: 
220:         /// @dev Insert the new piece into the max heap
221:         maxHeap.insert(pieceId, 0);
222: 
223:         ArtPiece storage newPiece = pieces[pieceId];
224: 
225:         newPiece.pieceId = pieceId;
226:         newPiece.totalVotesSupply = _calculateVoteWeight(
227:             erc20VotingToken.totalSupply(),
228:             erc721VotingToken.totalSupply()
229:         );
230:         newPiece.totalERC20Supply = erc20VotingToken.totalSupply();
231:         newPiece.metadata = metadata;
232:         newPiece.sponsor = msg.sender;
233:         newPiece.creationBlock = block.number;
234:         newPiece.quorumVotes = (quorumVotesBPS * newPiece.totalVotesSupply) / 10_000;

https://github.com/code-423n4/2023-12-revolutionprotocol/blob/08ff070da420e95d7c7ddf9d068cbf54433101c4/packages/revolution/src/CultureIndex.sol#L209-L234

The totalVotesSupply is calculated using total supply of erc20VotingToken and erc721VotingToken at the time the piece is created. It intends to calculate all the voting power that can vote for this art piece.

javascript📋
File: src/CultureIndex.sol
284:     function _calculateVoteWeight(uint256 erc20Balance, uint256 erc721Balance) internal view returns (uint256) {
285:         return erc20Balance + (erc721Balance * erc721VotingTokenWeight * 1e18);
286:     }

https://github.com/code-423n4/2023-12-revolutionprotocol/blob/08ff070da420e95d7c7ddf9d068cbf54433101c4/packages/revolution/src/CultureIndex.sol#L284-L286

The vulnerability arises because the totalVotesSupply is computed based on the token supplies at the time of art piece creation. However, due to the block-based clock mode in the vote checkpoint, the total supplies of erc20VotingToken and erc721VotingToken can increase within the same block, resulting in an underestimation of totalVotesSupply and consequently, quorumVotes.

javascript📋
File: src/base/VotesUpgradeable.sol
85:     /**
86:      * @dev Clock used for flagging checkpoints. Can be overridden to implement timestamp based
87:      * checkpoints (and voting), in which case {CLOCK_MODE} should be overridden as well to match.
88:      */
89:     function clock() public view virtual returns (uint48) {
90:         return Time.blockNumber();
91:     }

https://github.com/code-423n4/2023-12-revolutionprotocol/blob/08ff070da420e95d7c7ddf9d068cbf54433101c4/packages/revolution/src/base/VotesUpgradeable.sol#L85-L91

Possible attack scenarios include:

• A voter back-running the createPiece transaction and purchasing governance tokens in the same block, thereby artificially lowering the quorumVotes.
• A creator front-running a significant token purchase or auction settlement, leading to a similar underestimation of quorumVotes.

POC: This POC demonstrates the first case when a voter back-run the createPiece transaction to understate the totalVotesSupply and quorumVotes.

• Navigate to : cd packages/revolution
• Create a test file test/BypassQuorum.t.sol
• Execute forge test -vvvvv --match-path test/BypassQuorum.t.sol --match-test testBypassquorumVotes

javascript📋
// SPDX-License-Identifier: MIT
pragma solidity 0.8.22;

import { Test } from "forge-std/Test.sol";
import {console} from "forge-std/console.sol";
import { AuctionHouseTest } from "./auction/AuctionHouse.t.sol";
import { IERC20TokenEmitter } from "../../src/interfaces/IERC20TokenEmitter.sol";

contract POCTest is AuctionHouseTest {

    function testBypassquorumVotes() public {

        uint256 verbId0 = createDefaultArtPiece();

        (,,,,uint256 creationBlock ,uint256 quorumVotes,,uint256 totalVotesSupply)= cultureIndex.pieces(0);
        console.log("creationBlock: ", creationBlock); // creationBlock:  1
        console.log("quorumVotes: ", quorumVotes); // quorumVotes:  0
        console.log("totalVotesSupply: ", totalVotesSupply); // totalVotesSupply:  0

        // Voter back-run the createPiece transaction and buy vote tokens in the same block, the supply is not reflected in the piece info and the quorum is understated at 0
        uint256 buyAmount = 100 ether;
        vm.deal(address(21), buyAmount);

        address[] memory recipients = new address[](1);
        recipients[0] = address(1);

        uint256[] memory bps = new uint256[](1);
        bps[0] = 10_000;
        vm.stopPrank();

        vm.prank(address(21));
        erc20TokenEmitter.buyToken{ value: buyAmount }(
            recipients,
            bps,
            IERC20TokenEmitter.ProtocolRewardAddresses({
                builder: address(0),
                purchaseReferral: address(0),
                deployer: address(0)
            })
        );

        console.log("Should be quorum: ", cultureIndex.quorumVotes()); // Should be quorum:  1940052234587701020

    }
}

Tools Used

Manual

Recommended Mitigation Steps

When the piece is created, only store the creationBlock. The quorum should not be stored. It should be calculated directly using VotesUpgradeable.getPastTotalSupply, this will return the value at the end of the corresponding block.

It ensures that quorumVotes accurately reflects the voting power at the end of the block in which the art piece was created, thereby mitigating the risk of quorum bypass through token supply manipulation within the same block.

diff📋
    function dropTopVotedPiece() public nonReentrant returns (ArtPiece memory) {
        require(msg.sender == dropperAdmin, "Only dropper can drop pieces");

        ICultureIndex.ArtPiece memory piece = getTopVotedPiece();
-       require(totalVoteWeights[piece.pieceId] >= piece.quorumVotes, "Does not meet quorum votes to be dropped.");
+       uint256 totalVotesSupply = _calculateVoteWeight(
+               erc20VotingToken.getPastTotalSupply(piece.creationBlock),
+               erc721VotingToken.getPastTotalSupply(piece.creationBlock)
+            );
+       uint256 quorumVotes = (quorumVotesBPS * totalVotesSupply) / 10_000;
+       require(totalVoteWeights[piece.pieceId] >= quorumVotes, "Does not meet quorum votes to be dropped.");

Assessed type

Governance