Wrong accounting logic when syncRewards() is called within beforeWithdraw makes withdrawals impossible

Lines of code Vulnerability details Impact sfrxETH.beforeWithdraw first calls the beforeWithdraw of xERC4626, which decrements storedTotalAssets by the given amount. If the timestamp is greater than the rewardsCycleEnd, syncRewards is called. However, the problem is that the assets have not been...

Revoking a claim blocks a recipient from withdrawing any unclaimed but due amount.

Lines of code Vulnerability details Impact Whenever an admin revokes a claim and the recipient has any unclaimed but vested balance , the unclaimed part is also revoked. Take for instance a total amount of 365 tokens vested over 1 year with a release interval of 1 day. If the recipient at day 30...

It's possible to send tokens to an unwanted wallet

Lines of code Vulnerability details Impact recoverEther and recoverERC20 could send tokens to a lost wallet. Proof of Concept In the contract frxETHMinter, the methods recoverEther and recoverERC20 doesn't receive the destination address, it sends the ether to the owner, but it is possible that t...

Upgraded Q -> M from 496 [1663882181323]

Judge has assessed an item in Issue 496 as Medium risk. The relevant finding follows: --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> M from 143 [1663859521800]

Judge has assessed an item in Issue 143 as Medium risk. The relevant finding follows: 2.ETHRegistrarController.register can pass any "resolver" and "data" parameters , then ETHRegistrarController do "functionCall " It is possible to pass malicious parameters example : resolver =...

Upgraded Q -> M from 238 [1663858577471]

Judge has assessed an item in Issue 238 as Medium risk. The relevant finding follows: 2. call should be used instead of transfer on an address payable Sometimes this kind of issue is considered as Medium risk. The use of the deprecated transfer function for an address will inevitably make the...

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

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

Missing Transfer Verification

Lines of code Vulnerability details Impact The ERC20 standard token implementation functions return the transaction status as a boolean. It is a good practice to check for the return status of the function call to ensure that the transaction was executed successfully. It is the developer's...

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

Some of user funds stuck in contract because of division rounding error in beforeWithdraw()

Lines of code Vulnerability details Impact Function beforeWithdraw has been used in withdraw of Vault contract to calculates withdraw amount of users but because of division rounding error in this funds some user's funds would stuck in contract. Proof of Concept This is beforeWithdraw code:...

Unbounded proposal calls array length could cause DoS

Lines of code Vulnerability details Impact It's possible to render the governance and execution contract inoperable for a period of time and effectively kill some proposals due to execution failure, which will break the governance routine and function. This could be a mistake or on purpose. Proof...

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

A malicious user could stuck crowdfund ETH

Lines of code Vulnerability details Impact A user could freeze funds accumulated in a crowdfund and execute a "fake sale" that would be the approval of the token. Proof of Concept A group of users create a crowdfund, they raise money to buy the most good looking Bored Ape. An attacker would buy i...

Unchecked Transfer

Lines of code Vulnerability details High Severity - Unchecked Transfer Line 167 of Vault.sol has an unchecked Transfer. Reference and recommendation from slither. Tools Used: Slither --- The text was updated successfully, but these errors were encountered: All reactions...

[NAZ-M1] No Storage Gap for Upgradeable Contract Might Lead to Storage Slot Collision

Lines of code Vulnerability details Impact For upgradeable contracts, there must be storage gap to "allow developers to freely add new state variables in the future without compromising the storage compatibility with existing deployments" quote OpenZeppelin. Otherwise it may be very difficult to...

Tokens available to the TokenDistributor are not safe

Lines of code Vulnerability details Impact Any funds available to the TokenDistributor contract can be stolen. Via creating new distributions with a Party contract controlled by a hacker. When funds are in the TokenDistributor contract, for instance, transferred via the PartyGovernace.distribute...

Admin can prevent deposits by manipulating timewindow

Lines of code Vulnerability details Impact The protocol is open to an administrator's maliciousness. Proof of Concept As we can see in this line, the contracts in this project has some sanity checks even for admin only calls. However, the administrator is free to change a hedge/risk pair timewind...

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

[NAZ-H2] withdraw() does not update allowances

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. Since it does this for all of the owner's shares, the approved receiver should not be approved for all anymore after burning. Proof...

Problems with PegOracle

Lines of code Vulnerability details Impact There are at least a few problems with the PegOracle. I am grouping them into one submission because some of them are not that significant but the last one I believe deserves a higher severity. 1. Function latestRoundData queries getOracle2Price but...

[H3] Deposit ETH is not working as expected

Lines of code Vulnerability details Impact Deposit ETH deposit to msg.sender but not to receiver as stated in docs Proof of Concept Function depositETH at Vault.sol is not working as described in docs. @param receiver address of the receiver of the shares provided by this function, that represent...

Return value of transferFrom() does not check

Lines of code Vulnerability details Impact Return value of transferFrom inside deposit function does not checked: function deposituint256 id, uint256 assets, address receiver public override marketExistsid epochHasNotStartedid nonReentrant returns uint256 shares // Check for rounding error since ...

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

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

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

Wrong Math for latestRoundData

Lines of code Vulnerability details Impact Mathematical equivalent of price1 and price2 numbers in latestRoundData function is incorrect Proof of Concept The following example returns incorrect results with numbers contract Test function latestRoundDatapublic view returns uint256 uint256 price1 =...

Chainlink's latestRoundData might return stale or incorrect results

Lines of code Vulnerability details Impact On Controller.sol and in PegOracle.sol we are using latestRoundData, but there is no check if the return value indicates stale data. This could lead to stale prices according to the Chainlink documentation: Proof of Concept Tools Used Visual Inspection...

recoverERC20() in StakingRewards shouldn't allow transfer of rewardToken because it belongs to users who stacked their funds and owner shouldn't be able to immediately withdraw those users funds

Lines of code Vulnerability details Impact rewardToken balance in StakingRewards contract is belongs to staking users and owner shouldn't be able to withdraw all balance of rewardToken immediately, because this can cause users to lose their funds if owner perform this action by intention or by...

[NAZ-M3] Use safeTransfer()/safeTransferFrom() instead of transfer()/transferFrom()

Lines of code Vulnerability details Impact It is a good idea to add a require statement that checks the return value of ERC20 token transfers or to use something like OpenZeppelin’s safeTransfer/safeTransferFrom unless one is sure the given token reverts in case of a failure. Failure to do so wil...

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

VaultFactory.sol : changeTreasury, changeTimewindow, changeController - all these function will not update the vault parameters as they are read from memory.

Lines of code Vulnerability details Impact Reading the vault in memory and updating its parameters will not update its values permanently. Proof of Concept Tools Used Manual code review. Recommended Mitigation Steps Read vault in storage instead of memory Update in following lines of codes. --- T...

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

this is a test

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. Proof of Concept Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept. Tools Used Recommended Mitigation Steps --- The...

No control for the future Airdrop

Lines of code Vulnerability details Impact You can't control the airdrop NFT on this contract PartyGovernance.sol. You can only control the preciousListHash Proof of Concept The execute is check that the precious list preciousTokenIds and preciousTokens is valid or not if...

Anyone could steal the funds

Lines of code Vulnerability details Impact Anyone can create a contract. and send all the funds if maximumPrice == 0 or at the list he can get the maximumPrice Proof of Concept Create a contract to send the funds to it Invoke buy on CollectionBuyCrowdfund.sol or BuyCrowdfund And it will transfer...

Anyone can initialize contracts

Lines of code Vulnerability details Impact Party.initialize , CollectionBuyCrowdfund.initialize , BuyCrowdfund.initialize , AuctionCrowdfund.initialize are functions called with the onlyConstructor modifier . This allows these functions to only be called by a constructor function in any contract...

An operator can steal an NFT after its listed

Lines of code Vulnerability details Impact The ArbitraryCallsProposal contract prevents calls to setApprovalForAll on NFTs the Party owns. If an NFT is listed, calls to setApprovalForAll via the ArbitraryCallsProposal contract will succeed because the Party isn’t in possession of the NFT. Anyone...

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

Attackers can mislead users to think they delegated their voting power to them

Lines of code Vulnerability details Impact The delegation of the voting power in the Crowdfund contract can be changed even after the party is created. If a user changes this delegation, it won't change the actual delegation, which can help attackers to mislead users into thinking they delegated...

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

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

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

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

Previously nominated delegate can reset the delegation

Lines of code Vulnerability details burn allows for previously recorded delegate to set himself to be contributor's delegate even if another one was already chosen. This can be quite material as owner choice for the whole voting power is being reset this way to favor the old delegate. Proof of...

mapping deletion will not remove the full content

Lines of code Vulnerability details mapping deletion will not remove the full content Summary A deletion in a structure containing a mapping will not delete the mapping see the Solidity documentation. The remaining data may be used to compromise the contract. Proof of Concept Navigate to the...

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