MultiRewardEscrow.claimRewards() can break for rebasing tokens

Lines of code Vulnerability details Rebasing tokens make balanceOf modifications arbitrarily e.g: Aave share tokens. If such token is used in an escrow, the balance could become insufficient at the time of claiming rewards, making it impossible to claim rewards for that escrow. Impact Medium Proo...

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

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

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

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

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

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

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

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

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

AddressRegistry can associate same CID to different addresses at the same time

Lines of code Vulnerability details The AddressRegistry contract can associate a CID NFT to an account address. As stated in the contest, the CID NFT can be transferred out of the account that registered it. However, once transferred it can be registered again while keeping the previous...

Upgraded Q -> 2 from #508 [1675443058277]

Judge has assessed an item in Issue 508 as 2 risk. The relevant finding follows: New address and existing address inputs can be the same in upgradeExistingContract --- The text was updated successfully, but these errors were encountered: All reactions...

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

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

Upgraded Q -> 2 from #800 [1675429611496]

Judge has assessed an item in Issue 800 as 2 risk. The relevant finding follows: 1- recordStakingError function doesn't decrease the minipool avaxLiquidStakerAmt value : When the function recordStakingError is called by the multisig it decreases both the total AVAX staking amount and the AVAX...

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

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

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

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

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

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

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

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

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

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

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

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