Platform: Code4rena
Start Date: 13/12/2023
Pot Size: $36,500 USDC
Total HM: 18
Participants: 110
Period: 8 days
Judge: 0xTheC0der
Id: 311
League: ETH
Rank: 53/110
Findings: 2
Award: $68.11
🌟 Selected for report: 0
🚀 Solo Findings: 0
42.484 USDC - $42.48
When _settleAuction
in AuctionHouse is called, it sends the rewards to all of the creators of the art piece in the same transaction. However, malicious creators can make a smart contract that drains all of the gas, making the transaction extremely expensive.
Let's take a look at the part of the code in _settleAuction
that sends all of the creator rewards:
//Transfer creator's share to the creator, for each creator, and build arrays for erc20TokenEmitter.buyToken if (creatorsShare > 0 && entropyRateBps > 0) { for (uint256 i = 0; i < numCreators; i++) { ICultureIndex.CreatorBps memory creator = verbs.getArtPieceById(_auction.verbId).creators[i]; vrgdaReceivers[i] = creator.creator; vrgdaSplits[i] = creator.bps; //Calculate paymentAmount for specific creator based on BPS splits - same as multiplying by creatorDirectPayment uint256 paymentAmount = (creatorsShare * entropyRateBps * creator.bps) / (10_000 * 10_000); ethPaidToCreators += paymentAmount; //Transfer creator's share to the creator _safeTransferETHWithFallback(creator.creator, paymentAmount); } }
So if there are a lot of creator addresses that are made specifically to drain gas, the transaction will become very expensive. The current maximum number of creators per art piece is 100, defined in CultureIndex:
// Constant for max number of creators uint256 public constant MAX_NUM_CREATORS = 100;
and the current maximum gas spent per call to send ETH is 50,000:
function _safeTransferETHWithFallback(address _to, uint256 _amount) private { // Ensure the contract has enough ETH to transfer if (address(this).balance < _amount) revert("Insufficient balance"); // Used to store if the transfer succeeded bool success; assembly { // Transfer ETH to the recipient // Limit the call to 50,000 gas success := call(50000, _to, _amount, 0, 0, 0, 0) } // If the transfer failed: if (!success) { // Wrap as WETH IWETH(WETH).deposit{ value: _amount }(); // Transfer WETH instead bool wethSuccess = IWETH(WETH).transfer(_to, _amount); // Ensure successful transfer if (!wethSuccess) revert("WETH transfer failed"); } }
With the current restrictions, malicious creators can drain at least 5,000,000 gas, without accounting for the gas that'll be spent for the other operations involved in settling and creating a new auction.
With the current network usage, this would equal 5M gas * 40 gwei = 0.2 ETH = 450$ at current prices.
Manual review
You could make the maximum number of creators a small number, for example, 5. Even if the creators are more than 5 they can submit a payment splitter smart contract as a creator and split the payment themselves.
Or you could keep the creator rewards as a mapping, and implement a function claimCreatorRewards
that lets creators claim their creators' rewards. That way the gas cost will be paid by them and the attack vector will be mitigated
ETH-Transfer
#0 - c4-pre-sort
2023-12-22T19:03:34Z
raymondfam marked the issue as sufficient quality report
#1 - c4-pre-sort
2023-12-22T19:03:43Z
raymondfam marked the issue as duplicate of #93
#2 - c4-pre-sort
2023-12-24T14:36:00Z
raymondfam marked the issue as duplicate of #195
#3 - MarioPoneder
2024-01-06T13:24:13Z
See comment on primary issue: https://github.com/code-423n4/2023-12-revolutionprotocol-findings/issues/195#issuecomment-1879684718
#4 - c4-judge
2024-01-06T13:24:17Z
MarioPoneder marked the issue as partial-25
#5 - c4-judge
2024-01-11T18:25:51Z
MarioPoneder marked the issue as not a duplicate
#6 - c4-judge
2024-01-11T18:41:43Z
MarioPoneder marked the issue as duplicate of #93
#7 - c4-judge
2024-01-11T18:41:51Z
MarioPoneder marked the issue as satisfactory
🌟 Selected for report: pavankv
Also found by: 0xAsen, ABAIKUNANBAEV, Raihan, Sathish9098, ZanyBonzy, albahaca, hunter_w3b, ihtishamsudo, kaveyjoe, peanuts, unique, wahedtalash77
25.6332 USDC - $25.63
Revolution is a protocol that improves on Nouns DAO - a generative avatar collective that auctions off one ERC721, every day, forever. 100% of the funds from each auction go into a shared DAO treasury, and owning an NFT gives you 1 vote over the treasury.
The Revolution twist is that, instead of auctioning off a generative NFT like nouns, anyone can upload their art pieces, and the community votes on their favorites.
The auction proceeds are split between the art creator and the owner of the auction contract:
1. AuctionHouse.sol:
Purpose: AuctionHouse.sol facilitates the sale of ERC721 tokens, specifically VerbsTokens. Forked from NounsAuctionHouse, it ensures fair compensation for creators by splitting auction proceeds.
Key functionality:
_createAuction
: Initiates the creation of an auction by attempting to mint a VerbsToken. It checks for sufficient gas before executing the minting process, sets up the auction parameters such as start and end times, and emits an AuctionCreated
event if successful. If the minting operation fails, the function pauses the contract.settleAuction
: Finalizes an auction, paying out to the owner, creator(s), and the winning bidder based on predefined rates. If there are no bids and the reserve price isn't met, the Verb is burned. In case of a successful auction, the winning bidder receives the Verb, the owner gets a portion of the bid, and the creators are rewarded with both ether and ERC20 governance tokens according to the defined rates.createBid
: Enables users to place ETH bids on specific Verb auctions, validating bid conditions, including the bidder and auction status. It updates auction details, extends if needed, and refunds the previous bidder2. ERC20TokenEmmiter.sol: Purpose: the contract is a linear VRGDA that mints an ERC20 token when the payable buyToken function is called, and enables anyone to purchase the ERC20 governance token at any time. A portion of value spent on buying the ERC20 tokens is paid to creators and to a protocol rewards contract.
Key functionality:
buyToken
: The buyToken
function facilitates the purchase of tokens using ETH. It validates the sender and payment amount, distributing the ETH between treasury and creators based on predefined rates. It calculates tokens for both buyers and creators, transferring these tokens accordingly. Ensures the correctness of the basis point splits and returns the total tokens allocated to buyers.3. VerbsToken.sol:
Purpose: fork of the NounsToken contract. VerbsToken owns the CultureIndex. When calling mint() on the VerbsToken, the contract calls dropTopVotedPiece on CultureIndex, and creates an ERC721 with metadata based on the dropped art piece data from the CultureIndex.
Key functionality:
mint
/ burn
- minting and burning of Verbs tokens4. CultureIndex.sol:
Purpose: CultureIndex.sol serves as a repository for uploaded art pieces, allowing anyone to contribute media. Users who own ERC721 or ERC20 tokens can vote on art pieces, with their voting power determined by their token balances. The art piece voting data is managed using MaxHeap.sol, a heap data structure that facilitates efficient retrieval of the highest-voted art piece. The contract includes a dropTopVotedPiece function, which removes the top-voted item from the MaxHeap and returns it.
Key functionality:
createPiece
: generates a new art piece with specified metadata and a list of contributing creators, each assigned a basis point percentage. The function validates the creator array, ensuring it does not contain zero addresses and that the sum of basis points is exactly 10,000. The created art piece is assigned a unique ID, added to a max heap for efficient voting, and relevant events are emitted, including details about the piece, its sponsors, and the contributing creators.vote
: allows a voter to cast a vote for a specific ArtPiece. It checks for the validity of the piece ID, ensures the voter's address is valid, and verifies that the voter has not already voted for the given piece. The function checks that the piece has not been dropped, and the voter's weight exceeds the minimum vote weight. Once validated, the vote is recorded, the total vote weight for the piece is updated, and relevant events, including details about the vote and total weight, are emitted.voteforManyWithSig
: allows a voter to execute multiple votes using a signature. It first verifies the validity of the signature and upon successful verification, the function proceeds to cast votes for the specified list of pieceIds
on behalf of the specified voter.dropTopVotedPiece
: allows the designated dropper admin to pull and drop the top-voted piece from the culture index. It verifies that the caller is the designated admin and checks if the top-voted piece meets the required quorum votes to be dropped. If the conditions are met, it marks the piece as dropped, removes it from the heap, and emits a PieceDropped
event, returning the details of the dropped piece.5. MaxHeap.sol:
Purpose: Implements a MaxHeap data structure. More about the structure can be read here: https://www.geeksforgeeks.org/introduction-to-max-heap-data-structure/
6. NontransferableERC20Votes.sol:
Purpose: Simple nontransferable ERC20 token that represents votes in the system
7. VRGDAC.sol:
Purpose: Implementation of a VRGDA(Variable Rate Gradual Dutch Auction) used by the ERC20TokenEmitter
to emit ERC20 tokens at a predictable rate. The VRGDA contract dynamically adjusts the price of a token to adhere to a specific issuance schedule. If the emission is ahead of schedule, the price increases exponentially. If it is behind schedule, the price of each token decreases by some constant decay rate.
8. TokenEmitterRewards.sol:
Purpose: abstract contract that extends the RewardSplits
contract. It's designed to compute and handle rewards and deposits for the TokenEmitter
9. RewardSplits.sol:
Purpose: The contract is responsible for calculating and distributing rewards based on different parameters like builder referral, purchase referral, deployer rewards, and revolution rewards. These are represented as basis points (BPS) and are used to calculate the respective shares of the total reward.
Here is a basic diagram of how the core contracts of the system interact with each other:
Link if the image doesn't work: https://img001.prntscr.com/file/img001/9ksx3pWLQVSFa2siX7a9yQ.png
Here is a list of the governor roles and what rights they have for each contract.
In AuctionHouse:
Owner - can pause/unpause, setMinCreatorRateBps
, setEntropyRateBps
, setTimeBuffer
, setReservePrice
, and authorize upgrades via _authorizeUpgrade
In ERC20TokenEmmiter:
Owner - can pause/unpause, setEntropyRateBps
, setCreatorRateBps
, and setCreatorsAddress
In VerbsToken:
Owner - can setContractURIHash
, setMinter
, lockMinter
, setDescriptor
, lockDescriptor
, setCultureIndex
, lockCultureIndex
, and _authorizeUpgrade
In CultureIndex:
Owner - can _setQuorumVotesBPS
, and _authorizeUpgrade
In NontransferableERC20Votes:
Owner - can mint
an unlimited amount of tokens
In MaxHeap:
Owner - can _authorizeUpgrade
Centralization risks:
Token Concentration: If a small group of participants holds a significant amount of tokens, they could have a disproportionate influence on decision-making and governance.
The owner can mint an unlimited amount of NontransferableERC20Votes tokens
The owner can change the CreatorRateBps
, MinCreatorRateBps
, and CreatorsAddress
which means that in case the owner gets compromised or is malicious, he can direct a big % of the auction proceeds to addresses of his own.
Overall the codebase is relatively simple and of good quality, the functions have implemented proper access control, and are interacting well with each other. A 100% test coverage should be achieved.
Day 1-3:
Day 3-6:
Day 6-8:
In conclusion, the Revolution protocol represents a significant evolution in the realm of decentralized autonomous organizations (DAOs), building upon the foundational concepts established by Nouns DAO. By auctioning community-voted art pieces and directly compensating creators, Revolution introduces a more inclusive and dynamic approach to governance and token distribution.
Key features of the protocol include:
The project's aim to balance cultural contributions with capital influence and to make governance token ownership more accessible aligns with a growing trend in the blockchain community towards more equitable and participatory models.
Overall, the Revolution protocol embodies a progressive step in the evolution of DAOs, focusing on fair ownership distribution and community empowerment.
20 hours
#0 - c4-pre-sort
2023-12-24T00:46:37Z
raymondfam marked the issue as sufficient quality report
#1 - c4-judge
2024-01-07T14:26:17Z
MarioPoneder marked the issue as grade-b