Use safeTransferFrom() function instead of transferFrom()

Lines of code Vulnerability details Impact The transferFrom method is used in the contract PartyGovernanceNFT.sol, I however argue that this isn’t recommended because: OpenZeppelin’s documentation discourages the use of transferFrom, use safeTransferFrom whenever possible Given that any NFT can b...

Should call triggerEndEpoch() or triggerDepeg() first before withdrawing in testOwnerAuthorize()

Lines of code Vulnerability details Detail In Tests section, sponsors said they would like to assess if testOwnerAuthorize are failed due to a logic fault in the contracts or in testing. In this test case, it wants to check if users can approve to other address to help them call withdraw on Vault...

[NAZ-H1] First depositor can break minting of shares

Lines of code Vulnerability details Impact The attack vector and impact is the same as TOB-YEARN-003, where users may not receive shares in exchange for their deposits if the total asset amount has been manipulated. Proof of Concept In SemiFungibleVault.solL143, the allocation of shares is...

Non-contributor can force contributors to buy the NFT when the contributors no longer want to

Lines of code Vulnerability details Impact For a hyped NFT that is valuable to some users, these users can become contributors to contribute for this NFT through using the BuyCrowdfund contract. Yet, during the crowdfunding phase, the NFT's value drops sharply in which its price drops to 0. At th...

PegOracle reported fraction price is constructed to favor the depeg

Lines of code Vulnerability details Depeg event is defined as linked asset price being below the strike price in the terms of the underlying asset. However, the PegOracle aimed to report the fraction of the pegged asset to the underlying always reports the number below 1, no matter how prices are...

Users can lose funds because It's possible to call withdraw() in Vault without call to endEpoch() by Controller.triggerEndEpoch(),

Lines of code Vulnerability details Impact users shouldn't be allowed to withdraw their funds before epoch settling down, and code should check that endEpoch has been called before allowing withdraw for that epoch. but right now withdraw only checks that epoch has been ended and this would happen...

Ambiguous situation exists for calling triggerDepeg and triggerEndEpoch functions when block.timestamp is set to epochEnd

Lines of code Vulnerability details Impact As shown by the following isDisaster modifier, which is used by the triggerDepeg function below, and the triggerEndEpoch function below, when block.timestamp is set to epochEnd, both of the triggerDepeg and triggerEndEpoch functions are allowed to be...

Hash result truncation

Lines of code Vulnerability details Description There is function hashFixedGovernanceOpts in Crowdfund contract and function getDistributionHash in TokenDistributor contract. The first one truncates the hash result to 16 bytes, the second to 15 bytes. Impact It is possible to find a collision for...

# Only part of keccak256() is used as hash, making it susceptible to collision attacks

Lines of code Vulnerability details At 2 places in the code only part of the output of keccak256 is used as the hash: At TokenDistributor - DistributionState.distributionHash15 - uses only a 15 bytes as a hash This one is intended to save storage At Crowdfund.governanceOptsHash a 16 bytes is used...

ERC20.sol is susceptible to classic ERC20 approve functionality front-running exploit

Lines of code Vulnerability details Proof of concept The problem is perfectly described here The tldr; is that if you approved a person to spend 100 tokens and then you want to decrease his allowance to 50, if he spends his 100 tokens allowance before you set his allowance to 50 he will be able t...

Owner can use changeTimewindow() in VaultFactory and deny other from depositing into the Vaults (As this parameter used in epoch start Time detection). owner can use this for his/her benefits when he sees there is an good opportunity. changing this parameters should be with time-lock mechanism

Lines of code Vulnerability details Impact Owner can control timewindow of a Vault and epoch and by that he can control deposits of others and owner can deny other from depositing into a Vault by increasing timewindow as if he saw any profit by that. changing this type of parameters should be don...

Improper access control in withdraw at Vault.sol

Lines of code Vulnerability details Impact Anyone can withdraw on behalf of approved user Proof of Concept Function withdraw at Vault.sol has incorrect access control. As the owner is passed as a parameter anyone can call withdraw to a approved receiver. This is the same logic used in...

You can give address(0) _mint(), confused with _burn()

Lines of code Vulnerability details Impact You can give address0 mint, confused with burn Proof of Concept For example, the Initialize of AuctionCrowdfund does not determine whether initialContributor is 0, so the address0 can be Mint tokenid. //contracts/crowdfund/AuctionCrowdfund.sol function...

A majority attack can easily bypass Zora auction stage in OpenseaProposal and steal the NFT from the party.

Lines of code Vulnerability details 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 o...

Attacker can force AuctionCrowdfunds to bid their entire contribution up to maxBid

Lines of code Vulnerability details 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.getCurrentHighestBidderauctionId == addressthis revert...

Rounding errors can lead to wrong voting power

Lines of code Vulnerability details Impact The calculation of voting power in Crowdfund.getFinalContribution can lead to loss of voting power due to rounding errors. Tools Used Manual audit --- The text was updated successfully, but these errors were encountered: All reactions...

withdrawal of staking doesnt check for epochs

Lines of code Vulnerability details Impact when unstaking the code doesnt check wether its the end of the epoch or not Proof of Concept Tools Used none Recommended Mitigation Steps check for end of epoch --- The text was updated successfully, but these errors were encountered: All reactions...

rewards stuck in StakingRewards because of division rounding error in notifyRewardAmount() and rewardPerToken() and updateReward()

Lines of code Vulnerability details Impact In all functions notifyRewardAmount and rewardPerToken and updateReward there is a division which cause division rounding error and make some rewards to stuck in contract and users don't get their rewards and so users would lose funds. Proof of Concept...

Divide before multiply may lead to loss of precision

Lines of code Vulnerability details Impact Solidity integer division might truncate. As a result, performing multiplication before division can sometimes avoid loss of precision. Details This would affect the dynamicQuorumVotes vote logic correct functioning Proof of Concept...

Locked ether

Lines of code Vulnerability details Impact In contract Party.sol there is a receive declared. This means that the contract can accept eth payments. But there is no function defined , to withdraw those sent ether. So if a user accidently sent ether to the contract, the ether would be locked, as...

Front-running of accept call

Lines of code Vulnerability details Description There is accept and veto functions in the PartyGovernance contract. The functions accepts the proposalId accept function also accepts snapIndex, which does not contain any information about the proposal itself. As a result, transactions of users can...

The delegation even if you have no VotingPower

Lines of code Vulnerability details Impact when you buy a new VotingPower. You need to undelegated them before the next proposal Proof of Concept -When Alice sell his NFT by calling transferFrom on PartyGovernanceNFT.sol. it will be set for the delegating to the previous delegator. but he actuall...

Attacker can DOS private party by donating ETH then calling buy

Lines of code Vulnerability details Impact Party is DOS'd and may potentially lose access to NFT Proof of Concept Crowdfund.solL280-L298 party = party = partyFactory .createParty addressthis, Party.PartyOptions name: name, symbol: symbol, governance: PartyGovernance.GovernanceOpts hosts:...

PartyGovernance.disableEmergencyExecute() permanently disables emergencyExecute()

Lines of code Vulnerability details Impact PartyGovernance.disableEmergencyExecute will permanently disable PartyGovernance.emergencyExecute as there is no other means to toggle emergencyExecuteDisabled back to false. This means emergencyExecute can never be called once Proof of Concept Tools Use...

multiply users can contribution with one Token

Lines of code Vulnerability details Impact Users, don't have a Token for contribution but they can bypass this check easy Proof of Concept If the Crowdfund is private by using TokenGateKeeper.sol The contributor can contribution and then send the NFT or ERC20 to another user address. So two users...

Missing Check When Attempting to Check if the List of Precious Tokens are Correct

Lines of code Vulnerability details Impact There exists an issue where a missing check to ensure that the preciousTokens and preciousToksnIds list is the same length on line 1102 of the PartyGovernance.sol contract. This may lead to an inaccurate hash when attempting to run the execute function...

[H1] Improper access control in withdraw at SemifungibleVault.sol

Lines of code Vulnerability details Impact Anyone can withdraw on behalf of approved user Proof of Concept Function withdraw at SemifungibleVault.sol has incorrect access control. As the owner is passed as a parameter anyone can call withdraw to a approved receiver. function withdraw uint256 id,...

Proposer can double spend his votes as many times as he likes, rugging the party

Lines of code Vulnerability details Description Proposals are created using PartyGovernance's proposeproposal,.. function, and voted on thereafter using acceptproposalid,…. To make sure users don't vote twice, every proposal has hasVoted mapping to keep note of votes. The number of votes counted...

User fund lost because they can't withdraw() their funds before epoch startTime and they have to stuck in positions that become unprofitable even when epoch is not started

Lines of code Vulnerability details Impact users deposit their funds in Vault when epoch is not started but as other users deposit funds too or price of pegged token changes users get different risk to reward and they may wants to withdraw their funds before epoch start time to get out of bad...

PegOracle.sol#L59 : Issue with "price1" calculation.

Lines of code Vulnerability details Impact The calculated price1 could be negative or outdated one. This could affects the codes places wherever the latestRoundData is used to determine the price. one of the place is in Controller.solL261 - function getLatestPriceaddress token Proof of Concept...

Fee = 0 in case supply is less then 10

Lines of code Vulnerability details In case supply is less then 10 fee will be always = 0, which is almost impossible with ETH but it's not 100% excludible with an ERC20 token in case it's value is very high and precision is low . A check that supply shall more then 10 shall be set. --- The text...

Vault does not verify that tokens were transferred successfully.

Lines of code Vulnerability details Impact If for some reason the ERC20 transfer is temporarily failing, the user could call deposit for free or if calling withdraw the user would totally lose his allocation and funds. All the state variables would already have been updated at this stage, so he...

Griefing attack on the Vaults is possible, withdrawing the winning side stakes

Lines of code Vulnerability details Anyone can withdraw to receiver once the receiver is isApprovedForAllowner, receiver. The funds will be sent to receiver, but it will happen whenever an arbitrary msg.sender wants. The only precondition is the presence of any approvals. This can be easily used ...

[NAZ-M3] Use safeTransferFrom() instead of transferFrom() for ERC721 transfers

Lines of code Vulnerability details Impact The transferFrom method is used instead of safeTransferFrom, presumably to save gas. I however argue that this isn’t recommended because: OpenZeppelin’s documentation discourages the use of transferFrom, use safeTransferFrom whenever possible. Given that...

Invalid Use of BODMAS in TokenDistributor.sol and PartyGovernanceNFT.sol

Lines of code Vulnerability details Invalid Use of BODMAS in TokenDistributor.sol and PartyGovernanceNFT.sol In contract TokenDistributor.sol we initializing the value of uint128 fee using a mathematical way but in a wrong manner their brackets are missing which will clear out the instance of usi...

rewarding is not started after StakingRewards contract deployment and there is no specific time set for contract start time of rewarding, users stacked their funds has no way knowing when is rewarding is going to be started

Lines of code Vulnerability details Impact after StakingRewards the constructor logic set value for rewardRate and rewardsDuration but rewarding is not started for stackers and users has no way to find out when is rewarding going to be started. only reward Distributer can start rewarding process...

BuyCrowdfund.buy lacks access control

Lines of code Vulnerability details Impact Anyone can call BuyCrowdfund.buy and BuyCrowdfund.buy will call any function at any address and can send eth up to maximumPriceWhen maximumPrice is 0, all ETH in the contract can be sent. Consider maximumPrice = 70 ETH, the current seller's listing price...

ArbitraryCallsProposal.sol and ListOnOpenseaProposal.sol safeguards can be bypassed by cancelling in-progress proposal allowing the majority to steal NFT

Lines of code Vulnerability details Note: PartyDAO acknowledges that "canceling an InProgress proposal mid-step can leave the governance party in a vulnerable or undesirable state because there is no cleanup logic run during a cancel" in the "Known Issues / Topics" section of the contest readme. ...

Governance NFT Voting power can be distributed unfairly after the party purchase the NFT because voting power distribution depends on the contribution NFT burning order

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. Voting power can be distributed unfairly after the party purchase the NFT because voting power distribution depends on the contribution NFT burning order. For example, the NFT is purchased at price 1.5...

Why emergencyExecute() is required?

Lines of code Vulnerability details Impact Now PartyDAO multisig can steal all funds from all PartyGovernance instances If malicious user will get control over PartyDAO multisig, he will steal of funds from all projects Tools Used vs code Recommended Mitigation Steps Remove this funtion --- The...

Malicious party active member can approve malicious contract to spend and steal party ERC1155 nft and ERC20 tokens via arbitrary proposal execution

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. Let's look into the implementation in ArbitraryCallsProposal.sol // Check that the call is not prohibited. if !isCallAllowedcall, isUnanimous, preciousTokens, preciousTokenIds revert...

TokenDistributor: ERC777 tokensToSend hook can be exploited to drain contract

Lines of code Vulnerability details Impact TokenDistributor.createERC20Distribution can be used to create token distributions for ERC777 tokens which are backwards-compatible with ERC20. However, this introduces a reentrancy vulnerability which allows a party to get the tokens of another party. T...

The result of getPastVotes can be different time after time

Lines of code Vulnerability details Impact In moveDelegateVotes of ERC721Votes.sol, we write checkpoint every time, so there might be several checkpoints in the same timestamp. In that case, when we get past votes in getPastVotes, we use binary search by timestamp. But there might be several...

Malicious pausing the contract

Lines of code Vulnerability details Vulnerability details Description There is a function createAuction in Auction contract. It consist the following logic: /// @dev Creates an auction for the next token function createAuction private // Get the next token available for bidding try token.mint...

founderPct variable cast

Lines of code Vulnerability details Description There is a function addFounders in Token contract. It accepts array of FounderParams as an input. For each of founders it uses founderPct as an variable to store percent ownership for such founder. It is unsafe to cast it to from uint256 to uint8 in...

# _safeMint() should be used rather than _mint() wherever possible

Lines of code Vulnerability details safeMint should be used rather than mint wherever possible Impact In Token.sol.sol, eventually it is called ERC721 mint. Calling mint this way does not ensure that the receiver of the NFT is able to accept them, making possible to lose them. safeMint should be...

The voting power cannot be returned deterministically, if there are multiple checkpoints sharing the same timestamp

Lines of code Vulnerability details Impact Specifically, when writing a checkpoint, the code does not check whether the latest checkpoint is also in the current block. Consider a user makes multiple times of token transfer, the ERC721Votes contract will record multiple checkpoints with the same...

baseTokenId variable calculation

Lines of code Vulnerability details Description There is addFounders function in the Token contract. There is the following loop: // Used to store the base token id the founder will recieve uint256 baseTokenId; // For each token to vest: for uint256 j; j founderPct; ++j // Get the available token...

Potential of underflow on _moveDelegateVotes()

Lines of code Vulnerability details Impact Attacker could get typeuint192.max of voting weight So he can create a proposal to withdraw an amount from the Treasury.sol and he can pass the proposalThreshold with no need to anyone Proof of Concept 1- Attacker buy one NFT and transfer it to addr1 2-...

Use safeTransfer to send ERC721 tokens

Lines of code Vulnerability details Impact Winner of the auction can lose his NFT Proof of Concept When you settle and auction you transfer the NFT using transferFrom token.transferFromaddressthis, auction.highestBidder, auction.tokenId; Maybe this is just an intended behaviour and is the...