First deposit can break share calculation

Lines of code Vulnerability details Impact Vault share price can be maliciously inflated on the initial deposit, leading to the next depositor losing assets due to precision issues. Proof of Concept 147: shares = convertToSharesassets - feeShares; If feeShares = 0 the first depositor of Vault can...

Vault creator can prevent users from claiming staking rewards

Lines of code Vulnerability details Impact Vault creator can prevent users from claiming rewards from the staking contract. This can boost his liquidity and lure depositors to stake vault tokens. He can present a high APY and low fee percentage which will incentivize stakers When the staking...

MultiRewardStaking claimRewards() reentrancy for ERC-777 reward tokens

Lines of code Vulnerability details Impact A hacker can drain an ERC-777 reward token funds via reentrancy. This is because in the claimRewards function, the transfer of the reward token which triggers the hacker's ERC-777 hook takes place before setting accruedRewardsuserrewardTokensi to zero...

Unsafe token transfer in MultiRewardStaking and VaultController contracts

Lines of code Vulnerability details Impact The vulnerability in the MultiRewardStaking and VaultController contracts lies in the usage of the transfer and transferFrom functions, which does not provide the safety checks for the transfer of tokens, especially since the reward token can have...

Overflow tokens

Lines of code Vulnerability details Impact If admin will add more than 255 tokens in rewardToken, there would'nt any way to withdraw tokens or claim rewards Proof of Concept for example: all users deposit their money.After some time admin's add more tokensa, length of array with Token grow's to...

Unchecked revert causes to

Lines of code Vulnerability details Impact In AdapterBase.Sol when harvesting and exchanging all tokens using Pool2SingleAssetCompounder.SolL44 harvest can harvest all tokens but not swap them for underlying currency. In contract Pool2SingleAssetCompounder...

inital share manipulation attack possible in Vault

Lines of code Vulnerability details Description This is the classic share inflation attack described here: The popcorn Vault is an abstraction on top of other vaults which acts like adapters to wrap other yield bearing protocols. Hence the asset in Vault are the shares in this adapter. An early...

Use safeTransferFrom and safeApprove foe the reward tokens in Vault Controller instead of transferFrom and approve functions

Lines of code Vulnerability details Impact In 3 functions return values of ERC20 contracts either approve or transferFrom are not checked : fundStakingRewards ; addStakingRewardsTokens and handleInitialDeposit. This is especially dangerous in the addStakingRewardsTokens function, because in this...

AdapterBase.accruedPerformanceFee does not work with tokens with low decimals

Lines of code Vulnerability details The performance fees are only accrued when shareValue highWaterMark. 529: function accruedPerformanceFee public view returns uint256 530: uint256 highWaterMark = highWaterMark; 531: uint256 shareValue = convertToAssets1e18; //@audit M: this does not work for...

FIRST ERC4626 DEPOSIT CAN BE EXPLOITED ON SHARE CALCULATION

Lines of code Vulnerability details Impact As also encountered by Uniswap V2 and other protocols, the first depositor of an ERC4626 vault can maliciously manipulate the share price by depositing as low as 1 wei of liquidity prior to deliberately inflating ERC4626.totalAssets to as high as 1:1e18...

Core functionality is not working due to revert in _verifyCreatorOrOwner()

Lines of code Vulnerability details Impact It is not possible to pause/unpause vaults and adaptors nor add staking reward tokens since the verifyCreatorOrOwner function reverts due to a logical error. Proof of Concept The following logic is used to determine if msg.sender is a creator or owner of...

Upgraded Q -> 3 from #648 [1675725284542]

Judge has assessed an item in Issue 648 as 3 risk. The relevant finding follows: 1. Incorrect Minter Address Validation in Mint Function Link : Summary: The mint function in the RabbitHoleReceipt contract does not correctly check the msg.sender address for minter permissions. The onlyMinter...

Upgraded Q -> 2 from #619 [1675724616184]

Judge has assessed an item in Issue 619 as 2 risk. The relevant finding follows: L-06 In contract Quest the function claim shouldn't only set the receipt as claimed, but to burn it as well. As this problem brings the risk, where users can sell already claimed receipts to other people The function...

Upgraded Q -> 3 from #599 [1675723538994]

Judge has assessed an item in Issue 599 as 3 risk. The relevant finding follows: L-01 Erc1155Quest's tokens can be withdrawn before every reward has been claimed Impact The owner can withdraw all the remaining tokens after the Quest endTime. Thus, users who have not claimed their reward at the en...

Upgraded Q -> 3 from #619 [1675724566035]

Judge has assessed an item in Issue 619 as 3 risk. The relevant finding follows: The function withdrawRemainingTokens can be changed in a safer way to handle the withdraw from the owner and the protocol fee as well. This prevent risks allocated with the protocol fees. By the docs this function is...

Upgraded Q -> 2 from #670 [1675726426987]

Judge has assessed an item in Issue 670 as 2 risk. The relevant finding follows: L-04 Changing rabbitholeReceiptContract in QuestFactory will break currently running quests rabbitHoleReceiptContract must be the same in QuestFactory and Quest contracts for quests to function correctly. If there is...

Upgraded Q -> 3 from #664 [1675726078144]

Judge has assessed an item in Issue 664 as 3 risk. The relevant finding follows: L-1 ERC20 Quest: withdrawFee function should only be able to be called once instead of multiple times Issue: The withdrawFee function can be called multiple times by admin after a quest ends, resulting in more than t...

Upgraded Q -> 2 from #648 [1675725337760]

Judge has assessed an item in Issue 648 as 2 risk. The relevant finding follows: 2. Unbounded Array Vulnerability in Claim Function Link : Summary: The claim function in the Quest contract has an unbounded array vulnerability that could lead to an Out-of-Gas OOG error and make the contract...

Upgraded Q -> 3 from #664 [1675726122175]

Judge has assessed an item in Issue 664 as 3 risk. The relevant finding follows: L-2 ERC1155 Quest: withdrawRemainingTokens should factor in total number of receipts minted before withdrawal Issue: There may be users with unredeemed receipts who will not be able to claim if all the remaining toke...

Upgraded Q -> 3 from #621 [1675724753994]

Judge has assessed an item in Issue 621 as 3 risk. The relevant finding follows: L1 - Owner could withdraw all unclaimed tokens while some still should be claimable withdrawRemainingTokens function in the Erc1155Quest contract allows the owner to withdraw all remaining tokens, including unclaimed...

Upgraded Q -> 2 from #619 [1675724510983]

Judge has assessed an item in Issue 619 as 2 risk. The relevant finding follows: L-02 The function mintReceipt should check if the quest has expired on-chain as well The main function mintReceipt responsible for minting receipts lacks an important check to ensure the quest end time hasn't finishe...

Upgraded Q -> 2 from #621 [1675724705438]

Judge has assessed an item in Issue 621 as 2 risk. The relevant finding follows: L2 - mintReceipt function lacks a check to verify if the quest has already ended mintReceipt function missing check for ended quest. This could result in a scenario where a receipt is minted after the quest has ended...

Upgraded Q -> 2 from #670 [1675726386915]

Judge has assessed an item in Issue 670 as 2 risk. The relevant finding follows: L-03 DoS if address owns too many receipts With time it is viable for users to acquire thousands and tens of thousands of receipts. This may happen as a result of buying receipts for example, which was highlighted as...

Upgraded Q -> 2 from #615 [1675724239375]

Judge has assessed an item in Issue 615 as 2 risk. The relevant finding follows: Unbounded gas usage in claim function of Quest contract The claim function has an unbounded gas usage that traverses different arrays many times. The call to RabbitHoleReceipt.getOwnedTokenIdsOfQuest iterates all...

Upgraded Q -> 3 from #154 [1675567996775]

Judge has assessed an item in Issue 154 as 3 risk. The relevant finding follows: Erc20Quest.withdrawFee can be called against a quest more than once function withdrawFee public onlyAdminWithdrawAfterEnd IERC20rewardToken.safeTransferprotocolFeeRecipient, protocolFee; The withdrawFee function does...

Upgraded Q -> 2 from #117 [1675572860639]

Judge has assessed an item in Issue 117 as 2 risk. The relevant finding follows: Description If a single address has certain amount of RabbitHoleReceipt tokens receipts - according to tests 1050, when he tries to call claim function from Quest.sol it will always revert with 'Transaction ran out o...

Upgraded Q -> 2 from #251 [1675573596034]

Judge has assessed an item in Issue 251 as 2 risk. The relevant finding follows: L-03 The claim function might use an amount of gas greater than the block gas limit. Description: The claim function at the Quest.sol contract can consume an amount of gas greater than the block gas limit if the user...

Upgraded Q -> 2 from #329 [1675575934658]

Judge has assessed an item in Issue 329 as 2 risk. The relevant finding follows: Reentrancy issue on claim for Erc1155Quest There is a reentrancy issue when claiming ERC1155 tokens, that will you reenter before redeemedTokens is updated. Here is the callback Quest.solL114 Stick to the check effec...

Upgraded Q -> 2 from #365 [1675443623313]

Judge has assessed an item in Issue 365 as 2 risk. The relevant finding follows: L-4 Misleading comments - Multisig are still managing pool --- The text was updated successfully, but these errors were encountered: All reactions...

_updateReceiverStates CAN WORK INCORRECTLY WHEN CURRENT RECEIVER AND NEW RECEIVER MATCHES WITH UPDATED DRIP TIMES

Lines of code Vulnerability details Impact States can be incorrectly updated. Proof of Concept In updateReceiverStates method of Drips.sol, I noticed a different pattern of Using addDeltaRange when current receiver and new receiver matches with updated drip times. To Remove an existing drip, the...

nftAddress can be a malicious NFT

Lines of code Vulnerability details Impact In the subprotocolregistry contract, users are allowed to register any subprotocols. As long as the supportsinterface condition is met. The validity of the nftaddress address is not checked. function register bool ordered, bool primary, bool active,...

Multiple people can register the same cidNFTID

Lines of code Vulnerability details Impact Multiple people can register the same cidNFT in a way that the same "canonical on-chain identity" can be shared accross multiple real-life identities. Proof of Concept cidNFTs can be transfered as any ERC721 token. After each transfer the new owner can...

_squeezeDrips() passed the amount argument in place of amtPerSec for the _addDeltaRange, causing either underflow or the sender losing lots of fund!

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. squeezeDrips passes the amount argument in place of amtPerSec for its callee addDeltaRange in the following line addDeltaRangestate, cycleStart, cycleStart + 1, -int256amt AMTPERSECMULTIPLIER; The last...

[M-01] emitUserMetadata function may fail due to exceed gas limit

Lines of code Vulnerability details Impact The function emitUserMetadata in DripsHub may fail due to unbounded loop over userMetadata can be very large due to the user input. However, function could be called only from drivers, it's still public and large array could be passed. And the loop in...

callSigned() can be front-runned

Lines of code Vulnerability details Impact Attacker able to bypass check present in callSigned by passing a proper valid sender address as parameter to callSigned function Proof of Concept For signature check following function checks requiresigner == sender, "Invalid signature"; Where sender...

Squeezing drips from a sender can be front-run and prevented by the sender

Lines of code Vulnerability details Squeezing drips from a sender requires providing the sequence of drips configurations see NatSpec description in L337-L338: /// It can start at an arbitrary past configuration, but must describe all the configurations /// which have been used since then includi...

UNEXPECTED DATA LOSS IS POSSIBLE DUE TO UNSAFE DOWN CASTING OF UINT40 TO UINT32

Lines of code Vulnerability details Impact In the dripsRange function of the Drips.sol contract variable end is declared as of type uint40. After the arithmetic operations and conditional checks have been performed, the end variable is returned as uint32end thus down casting it unsafely. Proof of...

Solmate safeTransfer and safeTansferFrom does not check the code size of the token address

Lines of code Vulnerability details Impact The safeTransfer and safeTransferFrom don't check the existence of code at the token address. This is a known issue while using solmate's libraries. Hence this may lead to miscalculation of funds and may lead to loss of funds, because if safeTransfer and...

## Mathematical Modelling Cause a Significant Roundoff Error

Lines of code Vulnerability details Mathematical Modelling Cause a Significant Roundoff Error Function drippedAmt calculates the ended cycles. when calculating ended cycles it takes two divisions which cause round off error. Let's consider this scenario. Deposited token - WBTC amtPerSec = 1000000...

CidNFT#add will break if _type is not passed in correctly

Lines of code Vulnerability details Impact CIDNFT holders that wants to add subprotocols to their NFT will not be able to anymore because the subprotocol NFT will be stuck in the CidNFT.sol contract. Proof of Concept When a CIDNFT holder wants to add subprotocols to their NFT, he will call...

Griefing risk in mint

Lines of code Vulnerability details Impact CidNFT.mint has an optional parameter addList that enables users to register subprotocol NFTs to the CID NFT right after the mint. However, there is no guarantee that the cidNFTID encoded in addList is the same ID as the newly minted NFT. If there is a...

Upgraded Q -> 2 from #836 [1675451857205]

Judge has assessed an item in Issue 836 as 2 risk. The relevant finding follows: Underflow error when redeeming to 0 after minting some rewards --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> 2 from #615 [1675442567749]

Judge has assessed an item in Issue 615 as 2 risk. The relevant finding follows: 3:upgradeExistingContract need unregisterContract first and then registerContract. Avoid newAddr==existingAddr. unregisterContract remove newAddr function upgradeExistingContract address newAddr, string memory newNam...

Upgraded Q -> 2 from #846 [1675451731129]

Judge has assessed an item in Issue 846 as 2 risk. The relevant finding follows: L-2 no way to remove compromised/broken multisigs without upgrading the contract --- The text was updated successfully, but these errors were encountered: All reactions...

Split vulnerable to preimage attack

Lines of code Vulnerability details Impact A motivated attacker could invest the resources to craft a malicious SplitsReceiver to steal all of a users' pending funds. Proof of Concept This is a non-practical implementation of the attack, but shows by extending the SplitsReceiver array by any numb...

The users can't add traits for their CidNFT's

Lines of code Vulnerability details Proof of Concept The CidNFT contract implements the add function for the users to add traits/subprotocol Id's to their CidNFT's. During calling add function, it validates whether the user is the owner of the provided CiDNFT and the user is approved by the owner...

Upgraded Q -> 2 from #338 [1675444008015]

Judge has assessed an item in Issue 338 as 2 risk. The relevant finding follows: L-04 Staking.restakeGGP function should have whenNotPaused modifier Staking.sol 1 --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> 3 from #867 [1675460716325]

Judge has assessed an item in Issue 867 as 3 risk. The relevant finding follows: L-02, MinipoolManager, lines 670 - 684: The slash function slashes a node operator for the amount of whole duration. Since the cycles are in 14 days and the slashing is checked in the recordStakingEnd, if an operator...

Incorrect shift in assembly

Lines of code Vulnerability details Impact Shift operators shlx, y, shrx, y, sarx, y in Solidity assembly apply the shift operation of x bits on y and not the other way around, which may be confusing. Check if the values in a shift operation are reversed. Proof of Concept File: Drips.sol 823: val...

Upgraded Q -> 2 from #748 [1675439377792]

Judge has assessed an item in Issue 748 as 2 risk. The relevant finding follows: L MultisigManager doesn't have unregister function MultisigManager.sol contains a hard limit of MULTISIGLIMIT to 10. Meanwhile in the contract, there are functions like registerMultisig, enableMultisig, and...