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...

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...

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...

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...

# 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...

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...

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...

[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...

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...

A majority attack can steal precious NFT from the party by crafting and chaining two proposals

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

timewindow can be changed unexpectedly that blocks users from calling deposit function

Lines of code Vulnerability details Impact As shown by the following epochHasNotStarted modifier, which is used by the deposit function below, users can only deposit when block.timestamp modifier epochHa...

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...

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...

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...

Early contributor can always become majority of crowdfund leading to rugging risks.

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

Early contributor can always become majority of crowdfund leading to rugging risks.

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

Eth can be stucked in contract when NFT was gifted to party (AuctionCrowdfund.sol)

Lines of code Vulnerability details Impact We can call function finalize, which will claim the NFT if we won. After claiming the NFT, function will check if contract is the owner of NFT or not. if lastBid == 0 // The NFT was gifted to us. Everyone who contributed wins. lastBid = totalContribution...

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:...

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...

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 through a large “donation”. In the SemiFungibleVault.sol file, the allocation of...

If noone deposit in one Vault, assets deposited to the other Vault will be locked.

Lines of code Vulnerability details Impact In each Vault epoch, idFinalTVL and idClaimTVL values is only set when someone call triggerDepeg or triggerEndEpoch from Controller. In case of depeg, we will swap assets of hedge and risk Vault. But if one Vault has noone deposited, idFinalTVL will be...

splitRecipient resulting voting power tend to be overstated

Lines of code Vulnerability details When burn computes the contributor's share via getFinalContribution and contributor = splitRecipient it transfers the whole share to the splitRecipient in advance, even when not all the voting power were allocated. I.e. some contributors might not burn and the...

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...

[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,...

Possible reentrancy attack on deposit function

Lines of code Vulnerability details Impact In deposit function the shares was calculated before the asset.safeTransferFrom. One possible scenario is when the supply it's 0totalSupplyid == 0, the retrun of convertToShares it's assets, if the asset is an ERC777 a contract sender could call again 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...

havent specify the solidity compiler version, 8.15 and older version may lead to a mis encoded struct parameter

Lines of code Vulnerability details Impact the structure: struct ExecuteProposalParams uint256 proposalId; bytes proposalData; bytes progressData; bytes extraData; uint256 flags; IERC721 preciousTokens; uint256 preciousTokenIds; there are some unknown length elementbytes, in 8.15 and older...

[NAZ-M2] withdraw() Allows Anyone Withdraw Someone Else's Shares

Lines of code Vulnerability details Impact withdraw is meant for the owner or approved receiver to withdraw the owner's deposited assets and burn the owner's shares. It checks if msg.sender == owner OR if isApprovedForAllowner, receiver. If the owner has approved for all a receiver and someone...

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...

Using transferfrom on ERC721 tokens

Lines of code Vulnerability details Impact In the function createParty of contract Crowdfund.sol, when transferring the acquired NFTs to the new party, the transferFrom function is called instead of safeTransferFrom. If the addressparty is a contract address that doesn't support ERC721 tokens , t...

Arbitrary contract call allows attacker to steal from user's wallet

Lines of code Vulnerability details Impact In function prepareGate , in the following line : bool s, bytes memory r = addressgateKeeper.callcreateGateCallData; A call to an arbitrary contractgateKeeper with custom calldata createGateCallData is made in prepareGate, which means the contract...

[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...

Unsafe ERC721 operations

Lines of code Vulnerability details Impact It is recommended to use safeTransfer and safeTransferFrom when transferring ERC721 and ERC20 token Tools Used Manual audit Recommended Mitigation Steps Use the safeTransfer and safeTransferFrom functions to transfer ERC721 and ERC20 tokens --- The text...

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...

Lack of check if token is a contract

Lines of code Vulnerability details Impact solmate won't check if the token is a contract or not. It's possible for an attacker to the create a vault for a non existing token with a deterministic address. The problem occurs if the token gets deployed later, and another user tries to use this toke...

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...

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...

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. ...

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...

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...

Ddos in Governor.sol

Lines of code Vulnerability details Impact An attacker can cancel proposals. Proof of Concept The proposal ID depends on 4 variables:targets, values, calldatas, and descriptionHash. The Problem is that once a proposal is proposed and the proposal ID is stored, no matter if the proposal is execute...

NFT flashloan against governance voting

Lines of code Vulnerability details Impact Specifically, to support a proposal, the voting power is counted at the time of the proposal creation time. However, there are multiple services that support NFT flashloan, e.g., NFTuloan . Since the voting power is counted as the proposal creation time,...

It is possible to add more than 15 properties

Lines of code Vulnerability details The total number of properties is now limited to be 15 or less with hard code on the storage structures level. In the same time it is possible to add unlimited number of properties with MetadataRenderer's addProperties. If this happens, with a malicious intent ...

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...

The contract can transfer Ownership to the zero address.

Lines of code Vulnerability details Impact When we use transferOwnership or safeTransferOwnership to transfer contract ownership, there is the possibility of transferring ownership to the zero address. This will cause the owner to lose its ownership permanently. Proof of Concept In Ownable.sol...

Calling Governor.execute function without sending any ETH can withdraw general ETH funds from the Treasury contract

Lines of code Vulnerability details Impact When calling the following Governor.execute function, the Treasury.execute function is executed. Because Governor.execute is payable, ETH can be sent to the Treasury contract. function execute address calldata targets, uint256 calldata values, bytes...

Bids can be created while paused

Lines of code Vulnerability details createBid allows for bid creation while the Auction is paused. As the latter happens on a system error mint failure, this can allow an attacker to interacts with the malfunctioning system. This at least can lead to misallocation of user's funds, i.e. freezing t...

The users could duplicate their voting weight

Lines of code Vulnerability details Impact Any user who has an NFT could duplicate their voting weight at any time Proof of Concept 1- Alice has one NFT 2- He invokes delegate and passes to to his address 3- on delegate both of from == to are Alice addresses 4- on moveDelegateVotes from is the...