AdapterBase FEE_RECIPIENT is not implemented or updateable

Lines of code Vulnerability details Impact The FEERECIPIENT state variable in the AdapterBase.sol has an example address, TODO comment, and there is no way to update the value after deployment. This address is used to mint fees from the adapter when Adapter.harvest is called via the takeFees...

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

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

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

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

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

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

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

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

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

Locked Ether

Lines of code Vulnerability details Impact Contract with a payable function, but without a withdrawal capacity. Every Ether sent to Caller will be lost. Proof of Concept File: Caller.sol function callAsaddress sender, address to, bytes memory data public payable returns bytes memory returnData...

MinipoolManager: node operator can avoid being slashed

Lines of code Vulnerability details Impact When staking is done, a Rialto multisig calls MinipoolManager.recordStakingEnd . If the avaxTotalRewardAmt has the value zero, the MinipoolManager will slash the node operator's GGP. The issue is that the amount to slash can be greater than the GGP balan...

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

Upgraded Q -> 2 from #769 [1675429128999]

Judge has assessed an item in Issue 769 as 2 risk. The relevant finding follows: L-1 requireNextActiveMultisig always returns the 1st enabled Multisig Relevant code: As the name suggested, MultisigManager.requireNextActiveMultisig should return the next enabled Multisig. However, it actually alwa...

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

Lack of double step transfer in admin modification in a upgradeable contract is dangerous

Lines of code Vulnerability details Lack of double step transfer in admin modification in a upgradeable contract is dangerous Summary Double step transfer of admin / ownership should be a must in upgradeable contracts Vulnerability Detail Admin is changed with changeAdmin that calls changeAdmin,...

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

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

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

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

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

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

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

Support for fee-on-transfer tokens

Lines of code Vulnerability details Impact The AddressDriver.sol do not support fee-on-transfer tokens. If the asset is a fee-on-transfer token, tokens received from users could be less then the amount specified in the transfer. The protocol could suffer a loss of funds. Proof of Concept function...

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

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

Potential DOS in Contract Inheriting UUPSUpgradeable.sol

Lines of code Vulnerability details Impact There is a contract which inherit UUPSUpgradeable.sol, namely; Managed.sol . The contract is deployed using a proxy pattern whereby the implementation contract is used by the proxy contract for all its logic. The proxy contract will make delegate calls t...

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 #783 [1675429188655]

Judge has assessed an item in Issue 783 as 2 risk. The relevant finding follows: restakeGGP doesn't have whenNotPaused modifier At line 319 stakeGGP has whenNotPaused modifier, while restakeGGP has not. Suggest to add the same modifier to restakeGGP. --- The text was updated successfully, but the...

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

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