AdminProxy should do some extra security checks

Lines of code Vulnerability details Impact AdminProxy is the hot spot for all low-level calls, therefore it should do some extra security checks that are currently not in place. By design a Solidity low level call to a zero address or an EOA non contract address will return success true. The only...

RewardTokens can be locked in MultiRewardStaking contract when the rewardsEndTimestamp of the rewardsTokens are different.

Lines of code Vulnerability details Impact To claim reward tokens from the MultiRewardStaking contract deployed, a user must call claimRewardsaddress user, IERC20 memory rewardsTokens. The rewardsTokens array is populated with getAllRewardsTokens which returns all the reward Tokens the...

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

Wrong first parameter for _calcRewardsEnd when changing reward speed

Lines of code Vulnerability details Impact The function calcRewardsEnd is called with the previousEndTime as first parameter in MultiRewardStaking.changeRewardSpeed, which leads to wrong calculation of the new rewardsEndTimestamp, causing it to be later than it should be. This will lead to more...

ERC4626 vault shares can be maliciously inflated

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. The price of ERC4626 vault shares can be maliciously inflated during the first deposit, leading to the loss of assets for next depositors Proof of Concept Provide direct links to all referenced code in...

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

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

Inflation attacks with virtual shares and assets

Lines of code Vulnerability details Impact When the BaseAdapter is empty. Someone can frontrun a user to steal his funds by an inflation attack. Senario Lets say Alice wants to deposit 1 token with decimal 18, so 1e18 units to the vault calling deposit. This is how the attack would unfold. The...

Missed owner accrual in MultiRewardStaking _withdraw() leads to reward loss

Lines of code Vulnerability details Impact Function withdraw can be called from an approved caller to withdraw owner funds. The function accrues rewards for caller and receiver but misses the accrual for owner. If, for example, the owner didn't accrue any reward from the beginning of time and all...

Out of gas for view function

Lines of code Vulnerability details Impact View function return's array of IERC20. There is no limit for this array Proof of Concept In one time owner of contract can add too much token's and it would take more and more gas to return this transation. Allocation in memory is not so cheap. After 20...

BeefyAdapter._protocolWithdraw() can revert for some boosters

Lines of code Vulnerability details When withdrawing from an adapter, the function does an internal call to protocolWithdraw 210: function withdraw 211: address caller, 212: address receiver, 213: address owner, 214: uint256 assets, 215: uint256 shares 216: internal virtual override 217: if calle...

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

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

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

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

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

Signature replay attacks possible if deployed on multiple chains

Lines of code Vulnerability details Caller.callSigned operates using an EIP-712 signature which verifies the signed data to be used in a call on behalf of the signer. The problem with this method lies in the fact that it doesn't specify the chain ID, and thus if the contract is ever deployed to...

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

Anyone can call the DripsHub.split function to frontrun the DripsHub.setSplits function call of the trusted user who owns the splittable funds

Lines of code Vulnerability details Impact As mentioned by the following DripsHub.split function's comment, the user, who owns the splittable funds, needs to be trusted by the receivers for such splittable funds. Yet, when this trusted user wants to call the DripsHub.setSplits function below to...

Lack of nonReentrant modifier when using _safeMint function

Lines of code Vulnerability details Impact The safeMint function implementation can allow a contract to reenter the the calling contract. Proof of Concept the safeMint function can be reentered through the safeMint function. function safeMintaddress to, UserMetadata calldata userMetadata public...

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

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

AddressRegistry might have non-actual record

Lines of code Vulnerability details Impact AddressRegistry might has non-actual record, which leads to inconsistent AddressRegistry state, and might affect possible consumers. Proof of Concept To register favorite NFT user calls register function from AddressRegistry. Then he sold this NFT to...

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

Admin role lockout possible

Lines of code Vulnerability details Impact Admin of contracts is controlled by Managed contract, and implements a transfer of admin privilege in a single step. A malicious admin or an error in the new address when calling changeAdmin can prevent all admin activities on all the contracts forever...

Caller.callBatched doesn't enforce msg.value is equal to sum of call values

Lines of code Vulnerability details For each call in callBatched, we pass a value to be sent along with the call: for uint256 i = 0; i calls.length; i++ Call memory call = callsi; returnDatai = callsender, call.to, call.data, call.value; The sum of the values of the calls should be equal to...

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

Multiple addresses can point to the same cidNFTID

Lines of code Vulnerability details Impact During AddressRegistry.register there is no check to ensure that a cidNFTID has been registered to an address. As CID NFT is transferrable, this means that the same CID NFT can be registered by multiple addresses. This could cause troubles for Dapps that...

[M-04] Balance manipulation when contract is paused

Lines of code Vulnerability details Impact State-changing methods missing the whenNotPaused modifier, is a security hole. Even when contract is paused increaseTotalBalance and decreaseTotalBalance methods can be called internally. Therefore, medium severity matches. Proof of Concept function...

Unsafe casting of an int128 to uint128 can cause wrong accounting in _receiveDripsResult

Lines of code Vulnerability details Impact Unsafe casting of int128 to uint128 is dangerous and makes user receive more assets than they should. Proof of Concept state.amtDeltascycle can be a negative value. The delta value at each point is simply the relative change from the previous cycle. If...

SubprotocolRegistry accepts empty string as protocol name

Lines of code Vulnerability details Impact The input sanitization statements in SubprotocolRegistry.sol's register function are: function register bool ordered, bool primary, bool active, address nftAddress, string calldata name, uint96 fee external // ... if !ordered || primary || active revert...

Reentrancy in callBatched

Lines of code Vulnerability details Proof of Concept The Caller contract implements callBatched function in order to execute a batch of calls within one call. The function has payable declaration to be able to send ETH inside the call. The NATSPEC is also provided in parallel; /// @notice Execute...

User can do registration of one CID NFT many times

Lines of code Vulnerability details Impact User can do registration one CID NFT repeatedly, and event CIDNFTAdded will be emited again and again. No checks, that user already registerd. When willl be some logic on frontend, which use this event, it could break, when get one more events CIDNFTAdde...

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