I've competed in this contest between 12/09/22-19/09/22 and achieved second place. PartyDAO is a decentralized auction platform allowing users to crowdfund purchases of expensive NFTs together. Below are my accepted findings.
HIGH: 3
MED: 3
HIGH: Proposer can double spend his votes as many times as he likes, rugging the party
Description
Proposals are created using PartyGovernance's propose(proposal,..) function, and voted on thereafter using accept(proposal_id,…). To make sure users don't vote twice, every proposal has hasVoted mapping to keep note of votes. The number of votes counted when users vote is calculated by getVotingPowerAt(user, proposedTime,…), which calculates the most recent voting power before or at proposedTime. Additionally, users are able to transfer their voting power by transferring their PartyGovernanceNFT. This is not an issue by itself, because if user A votes on proposal P, and then transfers his voting powers to user B and tries to vote on proposal P from user B's context, the number of votes calculated for B's vote is his power at proposal time.
The one critical weakness in this validation system is that during the proposal creation block, the same votes may be re-used by different users. Attacker can propose a proposal, then immediately transfer his PartyGovernanceNFT to another wallet B, which can call accept() and the registered voting power will be equal to attacker's voting power. In fact, there is no limit to the amount of double-spending possible, with each iteration moving to a new address, and re-voting. The root cause is that in order for accept() to work when called from propose() to automatically register the proposer's vote, getVotingPower accepts a vote snapshot <= proposedTime. But since proposedTime = block.timestamp, during this block there is one final opportunity to manipulate the voting snapshots before they are set in stone.
Impact
Votes may be double spent indefinitely, allowing an attacker with a tiny amount of equity to pass a unanimous proposal to pull the NFT out of the party.
Proof of Concept
Below is a POC modifying an existing test, to show that two users with 25% and 50% voting power, can pass a unanimous vote. In practice, the likely exploit will be to contribute from an attacker contract, that has an attack function which will propose() and loop sub-contract creation, transfer of voting NFT to the new contract, and accept() to re-register the vote from the new address.
// Attack that gains unanimous power with only 75% of votes
function testProposalLifecycle_twoVotersUnanimousAttack() external {
(IERC721[] memory preciousTokens, uint256[] memory preciousTokenIds) =
_createPreciousTokens(2);
TestablePartyGovernance gov =
_createGovernance(100e18, preciousTokens, preciousTokenIds);
address undelegatedVoter1 = _randomAddress();
address undelegatedVoter2 = _randomAddress();
// undelegatedVoter1 has 25/100 intrinsic VP (delegated to no one/self)
gov.rawAdjustVotingPower(undelegatedVoter1, 25e18, address(0));
// undelegatedVoter2 has 50/100 intrinsic VP (delegated to no one/self)
gov.rawAdjustVotingPower(undelegatedVoter2, 50e18, address(0));
// Create a one-step proposal.
PartyGovernance.Proposal memory proposal = _createProposal(1);
uint256 proposalId = gov.getNextProposalId();
// Undelegated voter 1 submits proposal.
_expectProposedEvent(proposalId, undelegatedVoter1, proposal);
// Votes are automatically cast by proposer.
_expectProposalAcceptedEvent(proposalId, undelegatedVoter1, 25e18);
// Voter has majority VP so it also passes immediately.
//_expectProposalPassedEvent(proposalId);
vm.prank(undelegatedVoter1);
assertEq(gov.propose(proposal, 0), proposalId);
// Transfer vote1 -> voter2 , 25 votes
gov.transferVotingPower(undelegatedVoter1, undelegatedVoter2, 25e18);
// Undelegated voter 2 votes.
_expectProposalAcceptedEvent(proposalId, undelegatedVoter2, 75e18);
vm.prank(undelegatedVoter2);
gov.accept(proposalId, 0);
// The vote was unanimous so the proposal should be executable as well.
_assertProposalStatusEq(gov, proposalId, PartyGovernance.ProposalStatus.Ready);
}
Recommended Mitigation Steps
Add additional check in accept() function:
if (proposal.proposedTime == block.timestamp) {
require(proposal.votes == 0);This will not allow accept() to be called in the sensitive block except to register the proposer's votes.
HIGH: A majority attack can steal precious NFT from the party by crafting and chaining two proposals
Description
The PartyGovernance system has many defenses in place to protect against a majority holder stealing the NFT. Majority cannot exfiltrate the ETH gained from selling precious NFT via any proposal, and it's impossible to sell NFT for any asset except ETH. If the party were to be compensated via an ERC20 token, majority could pass an ArbitraryCallsProposal to transfer these tokens to an attacker wallet. Unfortunately, FractionalizeProposal is vulnerable to this attack. Attacker/s could pass two proposals and wait for them to be ready for execution. Firstly, a FractionalizeProposal to fractionalize the NFT and mint totalVotingPower amount of ERC20 tokens of the created vault. Secondly, an ArbitraryCallsProposal to transfer the entire ERC20 token supply to an attacker address. At this point, attacker can call vault.redeem() to burn the outstanding token supply and receive the NFT back.
Impact
A 51% majority could steal the precious NFT from the party and leave it empty.
Proof of Concept
The only non-trivial component of this attack is that the created vault, whose tokens we wish to transfer out, has an undetermined address until VAULT_FACTORY.mint() is called, which creates it. The opcode which creates the vault contract is CREATE, which calculates the address with keccak256(VAULT_FACTORY, nonce). Nonce will keep changing while new, unrelated NFTs are fractionalized. The attack needs to prepare both FractionalizedProposal and ArbitraryCallsProposal ahead of time, so that they could be chained immediately, meaning there would be no time for other members to call distribute() on the party, which would store the fractionalized tokens safely in the distributor. In order to solve this chicken and the egg problem, we will use a technique taken from traditional low-level exploitation called heap feng shui.
Firstly, calculate off-chain, the rate new NFTs are fractionalized, and multiple by a safety factor (like 1.2X), and multiply again by the proposal execution delay. This number, added to the current VAULT_FACTORY nonce, will be our target_nonce. Calculate
target_vault = keccak256(VAULT_FACTORY, target_nonce),
before_target_vault = keccak256(VAULT_FACTORY, target_nonce-1)Firstly, we will create a contract that has an attack function that:
Loop while before_target_vault != created_vault: • Mint new dummy attacker_NFT • created_vault = VAULT_FACTORY.mint(attacker_NFT…)
Call execute() on the FractionalizedProposal // We will feed the execute() parameters to the contract in a separate contract setter. Note that this is guaranteed to create target_vault on the correct address.
Call execute() on the ArbitraryCallsProposal
Then, we propose the two proposals:
Propose a FractionalizedProposal, with any list price and the precious NFT as parameter
Propose an ArbitraryCallsProposal, with target = target_vault, data = transfer(ATTACKER, totalVotingPower)
Then, we set the execute() parameters passed in step 2 and 3 of the attack contract using the proposalID allocated for them.
Then, we wait for execution delay to finish.
Finally, run the attack() function prepared earlier. This will increment the VAULT_FACTORY nonce until it is the one we count on during the ArbitraryCallsProposal. Pass enough gas to be able to burn enough nonces.
At this point, attacker has all the vault tokens, so he may call vault.redeem() and receive the precious NFT.
Recommended Mitigation Steps
Enforce a minimum cooldown between proposals. This will mitigate additional weaknesses of the proposal structure. Here, this will give users the opportunity to call distribute() to put the vault tokens safe in distributor.
A specific fix here would be to call distribute() at the end of FractionalizeProposal so that there is no window to steal the funds.
HIGH: A majority attack can easily bypass Zora auction stage in OpenseaProposal and steal the NFT from the party
Description
The PartyGovernance system has many defenses in place to protect against a majority holder stealing the NFT. One of the main protections is that before listing the NFT on Opensea for a proposal-supplied price, it must first try to be auctioned off on Zora. To move from Zora stage to Opensea stage, _settleZoraAuction() is called when executing ListedOnZora step in ListOnOpenseaProposal.sol. If the function returns false, the next step is executed which lists the item on Opensea. It is assumed that if majority attack proposal reaches this stage, it can steal the NFT for free, because it can list the item for negligible price and immediately purchase it from a contract that executes the Opensea proposal.
Indeed, attacker can always make settleZoraAuction() return false. Looking at the code:
try ZORA.endAuction(auctionId) {
// Check whether auction cancelled due to a failed transfer during
// settlement by seeing if we now possess the NFT.
if (token.safeOwnerOf(tokenId) == address(this)) {
emit ZoraAuctionFailed(auctionId);
return false;
}
} catch (bytes memory errData) {As the comment already hints, an auction can be cancelled if the NFT transfer to the bidder fails. This is the relevant AuctionHouse code (endAuction):
{
// transfer the token to the winner and pay out the participants below
try IERC721(auctions[auctionId].tokenContract).safeTransferFrom(address(this), auctions[auctionId].bidder, auctions[auctionId].tokenId) {} catch {
_handleOutgoingBid(auctions[auctionId].bidder, auctions[auctionId].amount, auctions[auctionId].auctionCurrency);
_cancelAuction(auctionId);
return;
}As most NFTs inherit from OpenZeppelin's ERC721.sol code, safeTransferFrom will run:
function _safeTransfer(
address from,
address to,
uint256 tokenId,
bytes memory data
) internal virtual {
_transfer(from, to, tokenId);
require(*_checkOnERC721Received*(from, to, tokenId, data), "ERC721: transfer to non ERC721Receiver implementer");
}So, an attacker can bid a very high amount on the NFT to ensure it is the winning bid. When AuctionHouse tries to send the NFT to attacker, the safeTransferFrom will fail because attack will not implement an ERC721Receiver. This will force the AuctionHouse to return the bid amount to the bidder and cancel the auction. Importantly, it will lead to a graceful return from endAuction(), which will make settleZoraAuction() return false and progress to the OpenSea stage.
Impact
A majority attack can easily bypass Zora auction stage and steal the NFT from the party.
Proof of Concept
1. Pass a ListOnOpenseaProposal with a tiny list price and execute it
2. Create an attacker contract which bids on the NFT an overpriced amount, but does not implement ERC721Receiver. Call its bid() function
3. Wait for the auction to end ( timeout after the bid() call)
4. Create a contract with a function which calls execute() on the proposal and immediately buys the item on Seaport. Call the attack function.
Recommended Mitigation Steps
_settleZoraAuction is called from both ListOnZoraProposal and ListOnOpenseaProposal. If the auction was canceled due to a failed transfer, as described in the comment, we would like to handle it differently for each proposal type. For ListOnZoraProposal, it should indeed return false, in order to finish executing the proposal and not to hang the engine. For ListOnOpenseaProposal, the desired behavior is to revert in the case of a failed transfer. This is because the next stage is risky and defense against the mentioned attack is required. Therefore, pass a revertOnFail flag to _settleZoraAuction, which will be used like so:
// Check whether auction cancelled due to a failed transfer during
// settlement by seeing if we now possess the NFT.
if (token.safeOwnerOf(tokenId) == address(this)) {
if (revertOnFail) {
revert("Zora auction failed because of transfer to bidder")
}
emit ZoraAuctionFailed(auctionId);
return false;
}MED: Attacker can list an NFT they own and inflate to zero all users' contributions, keeping the NFT and all the money
Description
Crowdfunds split the voting power and distribution of profits rights according to the percentage used to purchase the NFT. When an attacker lists his own NFT for sale and contributes to it, any sum he contributes will return to him once the sell is executed. This behavior is fine so long as the sell price is fair, as other contributors will receive a fair portion of voting power and equity. Therefore, when a maximum price is defined, it is not considered a vulnerability that an attacker can contribute ```maximumPrice - totalContributions``` and take a potentially large stake of the Crowdfund, as user's have contributed knowing the potential maximum price.
However, when maximum price is zero, which is allowed in BuyCrowdfund and CollectionBuyCrowdfund, the lister of the NFT can always steal the entirety of the fund and keep the NFT. Attacker can send a massive contribution, buy the NFT and pass a unanimous proposal to approve the NFT to his wallet. Attacker can use a flash loan to finance the initial contribution, which is easily paid back from the NFT lister wallet.
It is important to note that there is no way for the system to know if the Crowdfund creator and the NFT owner are the same entity, and therefore it is critical for the platform to defend users against this scenario.
Impact
Any crowdfund with zero maximumPrice configured is subject to NFT lister rug pull and complete takeover of the contribution funds.
Proof of Concept
1. Suppose totalContributions=X. Attacker will prepare flashloan callback which does:
contribute() with the borrowed funds
call BuyCrowdfund's buy() with a contract that will purchase the NFT using the large sum
call NFT lister contract's sendProfit() function which will send attacker the NFT sell amount
pay back the flash loan
2. Attacker will call flashloan(10,000X) to buy his own NFT and take unanimous holding of created party
3. Attacker will create ArbitraryCallsProposal with a single call to token.approve(ATTACKER, token_id). It is immediately ready for execution because attacker has > 99.99% of votes. Precious approve() is allowed in unanimous proposals.
4. Attacker calls NFT's transferFrom() to take back control of the NFT, leaving with the Crowdfund's contribution and the listed NFT.
Recommended Mitigation Steps
Disable the option to have unlimited maximumPrice for BuyCrowdfund and CollectionBuyCrowdfund contracts. AuctionCrowdfund is already safe by not allowing a zero maximumBid.
MED: Attacker can force AuctionCrowdfunds to bid their entire contribution up to maxBid
Description
AuctionCrowdfund's bid() allows any user to compete on an auction on the party's behalf. The code in bid() forbids placing a bid if party is already winning the auction:
if (market.getCurrentHighestBidder(auctionId_) == address(this)) {
revert AlreadyHighestBidderError();
}However, it does not account for attackers placing bids from their own wallet, and then immediately overbidding them using the party's funds. This can be used in two ways:
Attacker which lists an NFT, can force the party to spend all its funds up to maxBid on the auction, even if the party could have purchased the NFT for much less.
Attackers can grief random auctions, making them pay the absolute maximum for the item. Attackers can use this to drive the prices of NFT items up, profiting from this using secondary markets.
Impact
Parties can be stopped from buying items at a good value without any risk to the attacker.
Proof of Concept
1. Attacker places an NFT for sale, valued at X
2. Attacker creates an AuctionCrowdfund, with maxBid = Y such that Y = 2X
3. Current bid for the NFT is X - AUCTION_STEP
3. Users contribute to the fund, which now has 1.5X
4. Users call bid() to bid X for the NFT
5. Attacker bids for the item externally for 1.5X - AUCTION_STEP
6. Attacker calls bid() to bid 1.5X for the NFT
7. Attacker sells his NFT for 1.5X although no one apart from the party was interested in buying it above price X
Recommended Mitigation Steps
Introduce a new option variable to AuctionCrowdfunds called speedBump. Inside the bid() function, calculate seconds since last bid, multiplied by the price change factor. This product must be smaller than the chosen speedBump. Using this scheme, the protocol would have resistance to sudden bid spikes. Optionally, allow a majority funder to override the speed bump.
MED: Early contributor can always become majority of crowdfund leading to rugging risks.
Description
Voting power is distributed to crowdfund contributors according to the amount contributed divided by NFT purchase price. Attacker can call the buy() function of BuyCrowdfund / CollectionBuyCrowdfund, and use only the first X amount of contribution from the crowdfund, such that attacker's contribution > X/2. He will pass his contract to the buy call, which will receive X and will need to add some additional funds, to purchase the NFT. If the purchase is successful, attacker will have majority rule in the created party. If the party does not do anything malicious, this is a losing move for attacker, because the funds they added on top of X to compensate for the NFT price will eventually be split between group members. However, with majority rule there are various different exploitation vectors attacker may use to steal the NFT from the party ( detailed in separate reports). Because it is accepted that single actor majority is dangerous, but without additional vulnerabilities attacker cannot take ownership of the party's assets, I classify this as a medium. The point is that users were not aware they could become minority under this attack flow.
Impact
Early contributor can always become majority of crowdfund leading to rugging risks.
Proof of Concept
Victim A opens BuyCrowdfund and deposits 20 ETH
Attacker deposits 30 ETH
Victim B deposits 50 ETH
Suppose NFT costs 100 ETH
Attacker will call buy(), requesting 59ETH buy price. His contract will add 41 additional ETH and buy the NFT.
Voting power distributed will be: 20 / 59 for Victim A, 30 / 59 for Attacker, 9 / 59 for Victim B. Attacker has majority.
User can use some majority attack to take control of the NFT, netting 100 (NFT value) - 41 (external contribution) - 30 (own contribution) = 29 ETH
Recommended Mitigation Steps
Add a Crowdfund property called minimumPrice, which will be visible to all. Buy() function should not accept NFT price < minimumPrice. Users now have assurances that are not susceptible to majority rule if they deposited enough ETH below the minimumPrice.