Minipool whose multisig has been disabled cannot be reassigned a valid one

Lines of code Vulnerability details Impact The likelihood of startRewardsCycle reverting due to division by zero is practically not going to happen. This is because disableAllMultisigs is only reasonably invoked when pauseEverything is called by the defender. At his point, startRewardsCycle will...

Mitigation Confirmed for Mitigation of H-06 Issue mitigated

C4 issue H-06: MinipoolManager: node operator can avoid being slashed Comments In the original implementation, there were a few scenarios where malicious node operators can avoid being slashed. Mitigation PR 41 This PR includes mitigation for various issues H-03, H-06, M-13. Just focusing on the...

The mitigation does not sufficiently address the bug report M-02

Lines of code Vulnerability details Impact M-02: The mitigation does not sufficiently address the bug report M-02 Proof of Concept If we look into the M-02 report code-423n4/2022-12-gogopool-findings742 The report points out two issues: Implication 1 The above function upgradeExistingContract...

Early attacker can DOS rToken issuance

Lines of code Vulnerability details Impact An early attacker can DOS the issue functionality in the RToken contract. No issuances can be made. And the DOS cannot be recovered from. It is permanent. Proof of Concept You can add the following test to the Furnace.test.ts file and execute it with yar...

MiniPool.Count state is not fully cleaned up

Lines of code Vulnerability details Impact MiniCount state is not fully cleaned up Proof of Concept According to the PR that aims to address M-19 code-423n4/2022-12-gogopool-findings235 We removed minipool count entirely, in favor of the new AVAXValidating variable that tracks the amount of AVAX...

The node operators are likely to be slashed in an unfair way

Lines of code Vulnerability details C4 issue H-04: Hijacking of node operators minipool causes loss of staked funds Comments In the original implementation, the protocol had some unnecessary state transitions and it was possible for node operators to interfere the recreation process. The main...

Deficiency of slashed GGP amount should be made up from node operator's AVAX

Lines of code Vulnerability details Impact If staked GGP doesn't cover slash amount, slashing it all will not be fair to the liquid stakers. Slashing is rare, and that the current 14 day validation cycle which is typically 1/26 of the minimum amount of GGP staked is unlikely to bump into this...

Upgraded Q -> 2 from #353 [1676219078358]

Judge has assessed an item in Issue 353 as 2 risk. The relevant finding follows: 06 UPGRADEABLE CONTRACT IS MISSING A GAP50 STORAGE VARIABLE TO ALLOW FOR NEW STORAGE VARIABLES IN LATER VERSIONS --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> 2 from #525 [1676219014177]

Judge has assessed an item in Issue 525 as 2 risk. The relevant finding follows: Upgradeable contract is missing a gap50 storage variable to allow for new storage variables in later versions --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> 2 from #250 [1676238307490]

Judge has assessed an item in Issue 250 as 2 risk. The relevant finding follows: In first place, currentIndex which is of type mappingaddress = uint256 is incremented before using it in line 117. This will cause the implementation to miss the zero index, and start from the second place index 1. T...

Upgraded Q -> 2 from #362 [1676219107265]

Judge has assessed an item in Issue 362 as 2 risk. The relevant finding follows: N‑01 Upgradeable contract is missing a gap50 storage variable to allow for new storage variables in later versions --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> 2 from #59 [1676219064442]

Judge has assessed an item in Issue 59 as 2 risk. The relevant finding follows: L-06 Upgradeable contract is missing a gap50 storage variable to allow for new storage variables in later versions --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> 2 from #157 [1676219053268]

Judge has assessed an item in Issue 157 as 2 risk. The relevant finding follows: L-08 No Storage Gap for BaseSmartAccount and ModuleManager --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> 2 from #533 [1676218902616]

Judge has assessed an item in Issue 533 as 2 risk. The relevant finding follows: Upgradeable contract is missing a gap50 storage variable to allow for new storage variables in later versions --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> 2 from #308 [1676219092947]

Judge has assessed an item in Issue 308 as 2 risk. The relevant finding follows: 03 Upgradeable contract is missing a gap50 storage variable to allow for new storage variables in later versions --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> 2 from #504 [1676216850158]

Judge has assessed an item in Issue 504 as 2 risk. The relevant finding follows: Incorrect signature check in the validatePaymasterUserOp function --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> 2 from #293 [1676241639792]

Judge has assessed an item in Issue 293 as 2 risk. The relevant finding follows: 01 USER CAN POSSIBLY TRANSFER NO token0 OR token1 TO TimeswapV2Option CONTRACT IF CORRESPONDING token0 OR token1 IS A REBASING TOKEN When calling the following TimeswapV2Option.mint function, msg.sender uses the...

Upgraded Q -> 2 from #250 [1676238274782]

Judge has assessed an item in Issue 250 as 2 risk. The relevant finding follows: Then, the logic presumably tries to keep a lookup table between token id - index using the ownedTokensIndex variable which is of type mappinguint256 = uint256 . This is also wrong, since ERC1155 tokens can have...

Upgraded Q -> 2 from #272 [1676238909313]

Judge has assessed an item in Issue 272 as 2 risk. The relevant finding follows: function removeTokenEnumerationaddress from, address to, uint256 id, uint256 amount internal if to == address0 if idTotalSupplyid == 0 && additionalConditionRemoveTokenFromAllTokensEnumerationid...

Adding NFTS with AssociationType ORDERED or PRIMARY may cause overwriting

Risk rating Medium Risk Links to affected code Impact Subprotocol NFTs may be trapped in contract CidNFT forever. Proof of Concept When adding NFT to CidNFT with AssociationType ORDERED or PRIMARY, the cidData is written directly, without checking and handling the case that a previously added nft...

Upgraded Q -> 3 from #510 [1675932827359]

Judge has assessed an item in Issue 510 as 3 risk. The relevant finding follows: In red are the state transitions that can only be performed with special privileges recreateMinipool: The following transitions will be performed Withdrawable-PreLaunch Error-PreLaunch createMinipool: will perform th...

Upgraded Q -> 2 from #510 [1675932817801]

Judge has assessed an item in Issue 510 as 2 risk. The relevant finding follows: If the current state is Withdrawable, you can still call createMinipool This will result in: 1:recreateMinipool can be front-run by executing recordStakingEnd to get back the stake first, and then executing...

Upgraded Q -> 2 from #214 [1675930440482]

Judge has assessed an item in Issue 214 as 2 risk. The relevant finding follows: cenario 2 - Use node of node operator In this scenario the NodeOp registers for a duration longer then 14 days. The hacker will hijack the minipool after 14 days and earn rewards on behalf of the node operators node...

MultiRewardStaking.addRewardToken can eventually break the contract

Lines of code Vulnerability details When adding a reward token, the token address is added to rewardTokens. 263: rewardTokens.pushrewardToken; If rewardTokens is large enough, accrueRewards will revert with an out-of-gas error, as it loops through rewardsToken 373: for uint8 i; i 0...

First depositor can break minting of shares

Lines of code Vulnerability details Vulnerability details The calculation of exchange rate for shares in Popcorn Vault is done by dividing the total supply of shares by the totalAssets of the vault. The first depositor can mint a very small number of shares, then donate to the vault to manipulate...

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

managementFee is unfair and can be used to steal stakers deposits

Lines of code Vulnerability details Description managementFee is a fee that is taken on TVL and calculated per year: File: Vault.sol 429: function accruedManagementFee public view returns uint256 430: uint256 managementFee = fees.management; 431: return 432: managementFee 0 433: ?...

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

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

ABI mismatch when calling initialize function

Lines of code Vulnerability details Impact This is the initialize function of contract Vault. It has 5 parameters. function initialize IERC20 asset, IERC4626 adapter, VaultFees calldata fees, address feeRecipient, address owner external initializer But when calling this function in VaultControlle...

Re-entrancy in MultiRewardStaking.claimRewards

Lines of code Vulnerability details Impact If an ERC-777 token is used as reward token for any Staking contract in the system, that reward token can be completely drained from the Staking contract. Proof of Concept Re-entrancy can be done in the MultiRewardStaking.claimRewards function because of...

Does not support non-18 decimals token

Lines of code Vulnerability details Impact Hardcode 1e18 for calculating fees and rewards, can make the template protocoal non flexiable for the calculating...

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 does not work with fee-on-transfer tokens

Lines of code Vulnerability details The way MultiRewardStaking handles deposits does not work with fee-on-transfer tokens: deposit uses convertToShares to compute the number of shares to mint, which returns assets - ie a 1:1 equivalence between shares and the number of asset tokens. 98: function...

MultiStakingReward.sol assumes all RewardTokens are in 18 decimal places

Lines of code Vulnerability details Impact Calculation of accrued rewards will be affected. Proof of Concept The function accrueRewards is called anytime rewards needs to be accrued. The variable supplyTokens is the total supply of the reward token. deltaIndex is calculated by taking the accrued ...

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

Missing owner check in function addTemplate in DeploymentController

Lines of code Vulnerability details Impact Attacker can add malicous Vaults/Adatpors/Strategies template to TemplateRegistry. Attack can frontrun operator's transaction with the same templateCategory and templateId, but with a malicious Vault/Adatpor/Strategy template. If the operator does not...

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

reentrancy in MultiRewardStaking::claimRewards for tokens with transfer callbacks, like erc777

Lines of code Vulnerability details Impact An attacker can drain all the tokens from MultiRewardStaking Proof of Concept In claimtRewards important state changes are done after interactions with tokens: File: MultiRewardStaking.sol function claimRewardsaddress user, IERC20 memory rewardTokens...

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

First depositor for the Vault can be front-run and have part of their deposit stolen

Lines of code Vulnerability details Description The first deposit with a totalSupply of zero shares will mint shares equal to the deposited amount. File: src/vault/Vault.sol 298: supply == 0 299: ? assets 300: : assets.mulDivsupply, totalAssets, Math.Rounding.Down; Link to Code File:...

MultiRewardStaking._accrueRewards can lead to loss of rewards for lower decimal tokens

Lines of code Vulnerability details Rewards accrual is computed in the following way: 406: deltaIndex = accrued.mulDivuint25610decimals, supplyTokens, Math.Rounding.Down.safeCastTo224; This can lead to truncation for low decimal tokens: Consider an instance with DAI as the asset, with a total of...

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

Changing reward speed calculates wrong rewardsEndTimestamp

Lines of code Vulnerability details Impact In MultiRewardStaking.changeRewardSpeed the new rewardsEndTimetamp is calculated based on the current balance of reward tokens in the contract. However, a fraction of this balance might already be accrued and accounted as reward, but just has not been...

Incorrect computation in MultiRewardStaking changeRewardSpeed() leads to loss of rewards

Lines of code Vulnerability details Impact The changeRewardSpeed function computes rewardsEndTimestamp incorrectly for the case block.timestamp block.timestamp ? prevEndTime : block.timestamp.safeCastTo32, rewardsPerSecond, remainder If the prevEndTime block.timestamp then it can be reduced 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...

Any user can drain the entire reward fund in MultiRewardStaking due to incorrect calculation of supplierDelta

Lines of code Vulnerability details Impact Reward deltaIndex in accrueRewards is multiplied by 10decimals but eventually divided by rewards.ONE which is equal to 10IERC20MetadataaddressrewardToken.decimals in accrueUser. If the number of decimals in MultiRewardEscrow share token differs from the...

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

claimRewards is not re-entrancy safe.

Lines of code Vulnerability details Impact In MultiRewardStaking the function claimRewards doesn’t have nonReentrant which makes it possible to re-enter the function. If one of the reward tokens in ERC-777 token, it is possible to re-enter and claim the reward again and again until the contract i...