Malicious user can use previously used nodeID to prevent user(s) from withdrawing minipool funds

Lines of code Vulnerability details In createMinipool, an event is emitted with details of a newly created minipool. This includes relevant information that a subsequent user can utilise to create another minipool.The only condition that prevents a minipool from being created again with the same...

Underlying assets stealing in token via share price manipulation

Lines of code Vulnerability details Impact asset can be stolen from depositors in the vault by manipulating the price of a share. Proof of Concept ERC4626 vaults are subject to a share price manipulation attack that allows an attacker to steal underlying tokens from other depositors this is a kno...

NODE OPERATORS CAN WITHDRAW ALL THEIR GGP COLLATERAL BEFORE VALIDATION PERIOD ENDS THEREBY AVOIDING SLASHING.

Lines of code Vulnerability details Impact 1. Node operators can avoid slashing, thus no penalties. 2. Node operators can withdraw their entire GGP collateral before the validation period is over. Proof of Concept The withdrawGGP function in Staking.sol transfers back to node operator excess GGP...

Contract cannot be initialized due to revert

Lines of code Vulnerability details Impact TokenggAVAX.initialize would revert due to the constructor setting initialized to typeuint8.max = 255 thus making initialized not less than 1. This does not pass the require check in initializer modifier, thus resulting to a revert thereby making...

Wrong reward distribution because protocol won't reset avaxAssignedHighWater value for a user if calculateAndDistributeRewards() doesn't get called for that user in that cycle

Lines of code Vulnerability details Impact node operators ggp rewards are distributed by function calculateAndDistributeRewards which is called by Multisig and function calculateAndDistributeRewards can only distribute current cycle rewards. the rewards are calculated based on user's...

TokenggAVAX.sol : First depositor can break minting of shares

Lines of code Vulnerability details Impact A well known attack vector for almost all shares based liquidity pool contracts, where an early user can manipulate the price per share and profit from late users' deposits because of the precision loss caused by the rather large value of price per share...

FIRST DEPOSIT CAN BREAK SHARE CALCULATIONS

Lines of code Vulnerability details Impact Future depositors are forced to pay a huge value of assets to deposit. It is not practically possible for all users. This could directly affect the attrition of users towards this system. Proof of Concept A well-known attack vector for almost all...

The owner minipool count is not decreased in the case of a staking error

Lines of code Vulnerability details Impact When a node operator creates a new pool or the recreateMinipool function is called the minipool count of the owner is increased by 1 and when the staking ends the multisig calls the recordStakingEnd function which will decrease the owner minipool count b...

Funds of Node Operators can be nullified by any attacker

Lines of code Vulnerability details Impact The MinipoolManager.createMinipool function do not validate the caller's address due to which any address can invoke the createMinipool function with any nodeID existing or new as input. For any existing nodeID the function can be invoked as long as the...

Increase in ERC4626 shares due to inflation

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. A bad actor can exploit the Vault by depositing a small amount of asset tokens 1 wei and receiving 1 wei of shares tokens. The attacker can then send a large amount of asset tokens 10000e18 - 1 to infla...

Possible to block withdrawal of staked funds after recordStakingEnd or stakingError

Lines of code Vulnerability details Impact Node operators can lose their staked AVAX after stakingEnd or stakingError. Funds will be locked in the Staking contract, but impossible to withdraw. A bad actor does need to supply 1000 AVAX which he gets back and has not have real incentive to do it, b...

TokenggAVAX share price manipulation

Lines of code Vulnerability details Impact Reporting this issue as medium severity as a leak of value. Solmate's ERC4626 convertToShares calculates shares as assets totalSupply / totalAssets. It is possible to exploit this function by depositing 1 wei of asset in exchange 1 share totalSupply = 1...

getStakers() and getMinipools() could return wrong values (Access Control)

Lines of code Vulnerability details Impact Staking.sol and MinipoolManager.sol contracts use the eternal storage pattern. The contracts are a key-value store that all protocol contracts can write to and read. more info: Functions getStakers.staking and getMinipools.MinipoolManager are implemented...

GGP slashing mechanism is incomplete.

Lines of code Vulnerability details Impact The protocol docs mentions that "If the validator is failing at their duties, their GGP will be slashed and used to compensate the loss to our Liquid Stakers." But the actual implementation of the Staking.slashGGP function is very different from the abov...

Owner may lose funds if Minipool is recreated before funds are withdrawn

Lines of code Vulnerability details The createMinipool function of the MinipoolManager contract can be used to reinitialize an existing minipool and potentially lose user funds. If the given nodeID has an existing minipool index, then the state for the minipool is reset: if minipoolIndex != -1...

SLASH LOGIC INAPPROPRIATELY IMPLEMENTED

Lines of code Vulnerability details Impact The slash logic in the protocol has overlooked the following two issues: slashminipoolIndex is only called when avaxTotalRewardAmt is equal to 0 which forgoes all other low performing instances even if the node operator has only brought in 1 wei of AVAX...

Upgraded Q -> M from #188 [1671981716625]

Judge has assessed an item in Issue 188 as M risk. The relevant finding follows: 188 --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> M from #4 [1671756144822]

Judge has assessed an item in Issue 4 as M risk. The relevant finding follows: GroupBuy: Insertion timestamp ignored The documentation states that "If the users have the same quantity as well, the bid that was placed later will have Raes removed.". However, with the current implementation, this i...

Upgraded Q -> M from #32 [1671721748112]

Judge has assessed an item in Issue 32 as M risk. The relevant finding follows: 2. StableVault deposits are limited to 18 decimals During deposit and withdraw to/from StableVault contract, it mints/burns the same amount of stable tokens with respect to decimals. The current implementation support...

RuniverseLand.sol#mint() can be bricked

Lines of code Vulnerability details Impact RuniverseLand.solmint can be bricked. Proof of Concept The mint function uses numMinted to generate the tokenId: File: RuniverseLand.sol 72: function mintaddress recipient, PlotSize size 73: public 74: override 75: returns uint256 76: 77: uint256 tokenId...

Non-standard ERC20 tokens are locked in the contract

Lines of code Vulnerability details Impact The function forwardERC20s transfers ERC20 tokens out of the contract to the owner. However, it does not properly handle non-standard ERC20 tokens such as USDT which do not return a bool when the transfer is called. The issue is that token is of type...

RuniverseLandMinter._mintTokensUsingTokenId does not verify that the tokenId matches the corresponding plotSize

Lines of code Vulnerability details Impact The first eight digits of the RuniverseLand TokenID indicate the corresponding plotSize of the NFT owner can call RuniverseLandMinter.ownerMintUsingTokenId directly to mint the NFT for a specific TokenID In RuniverseLandMinter.mintTokensUsingTokenId, the...

secondaryMinter may break plotsAvailablePerSize

Lines of code Vulnerability details Impact RuniverseLand allows primaryMinter and secondaryMinter to mint NFT. function mintTokenId address recipient, uint256 tokenId, PlotSize size public override nonReentrant requirenumMinted MAXSUPPLY, "All land has been minted"; require msgSender ==...

Grief on transfers due to vestingStart during vesting

Lines of code Vulnerability details Impact Past similar finding with the same severity: code-423n4/2022-05-runes-findings30 While centralization risk is acknowledged by the team & the C4udit tool: this may lead to loss of functionality grief. Proof of concept There is no requirement for the start...

RuniverseLand mint function does not work

Lines of code Vulnerability details Impact The function mint of RuniverseLand will not work and seemingly unnecessary Proof of Concept RuniverseLand.sol has a public function mint that can be used to mint a new plot. Note that this function uses numMinted as a new token ID while numMinted denotes...

Liquidation logic is incorrect in some conditions

Lines of code Vulnerability details Impact Because purchaseLiquidationAuctionNFT function clears remaining debt of debtor if he has no more collateral, it's possible that when 2 auctions exists in same time, liquidation logic will not work properly and debt will be nullified before last auction i...

totalCollateralValue in maxDebt may not be calculated correctly everytime

Lines of code Vulnerability details Impact totalCollateralValue in maxDebt may not be calculated correctly sometimes which leads to questionable lending procedures. Proof of Concept Bob has 10 BAYC NFTs, one of which is an extremely rare one with a crown and a rainbow body, which costs about 150...

function underwritePriceForCollateral() uses message.timestamp > block.timestamp to validate oracle message timestamp but it can create MEV as miners can control block.timestamp and revert some of the user's transactions

Lines of code Vulnerability details Impact Function underwritePriceForCollateral validates the oracle message which includes the price of the NFT and returns the price of an asset from a signed oracle message. to check the validity of the message's timestamp code checks that if...

NFT owner only is allowed for liquidation, this may not work for all the cases, the debt can be insolvent

Lines of code Vulnerability details Impact When bad debt is not paid or not able to recover the through auction of NFT, then the debt will be insolvent. Proof of Concept Contract allows for liquidation to recover the debt. Also, it has the auctioning mechanism to recover the debt by selling the...

Reentrancy attack allows to get loan for free

Lines of code Vulnerability details Impact Reentrancy attack allows to get loan for free when startLiquidationAuction is called on last collateral token. Proof of Concept When user has a bad debt, then anyone can start auction for his nft. To purchase token, liquidator can call...

Borrowers don’t need to run the risk of being liquidated

Lines of code Vulnerability details Impact Borrowers are able to borrow Papr, swap it for another asset in Uniswap without ever having to pay their debt because they can remove their collateral without paying their debt. Ultimately leading to free assets at the cost of others. Proof of Concept As...

purchaseLiquidationAuctionNFT() may incorrectly returned the Auction funds to the liquidated user

Lines of code Vulnerability details Impact may incorrectly returned the Auction funds to the liquidated user in purchaseLiquidationAuctionNFT, After someone purchases the auction NFT, the amount of the auction received will be distributed. In the existing logic, when the amount of the auction is...

Wrong implemention of ERC721TokenReceiver interface leads to incorrect collateral ownership and NFT loss

Lines of code Vulnerability details Impact To add a collateral, one could send the NFT directly to the contract, onERC721Received is called then to handle adding the collateral to the vault. However, if the user sends the NFT via an operator then the ownership of the collateral will be assigned t...

PaprController.sol: reservoir oracle price equal to 0 allows bypassing NFT liquidations

Lines of code Vulnerability details Impact When there is a reservoir oracle message with the price of the NFT equal to zero, the debt of a vault will be reset in the PaprController.purchaseLiquidationAuctionNFT function when a NFT is bought in an auction. So the borrower can now withdraw all othe...

PaprController.removeCollateral() only takes the price of the first collateral to determine whether the NFTs can be withdrawn

Lines of code Vulnerability details Impact Unintended leniency of protocol will be taken advantage by users. Users can withdraw NFTs even if their debt of a particular NFT is higher than intended. Proof of Concept When a user wants to removeCollateral, he calls removeCollateral which loops each...

code doesn't check that To address is not 0x0 in contract function, if user call contract's function with wrong value he would lose his funds

Lines of code Vulnerability details Impact There is no sanity checks when user wants to get loan or ... for a to address and if the value of to set as 0 user would lose those funds. Proof of Concept none of the functions increaseDebtAndSellmintTo, , increaseDebtmintTo, ,...

mintlistMint: Address that is in both merkle trees not correctly handled

Lines of code Vulnerability details Impact It is possible to set two merkle roots for the mintlist mintlistMerkleRoot1 and mintlistMerkleRoot2 and both trees are used in queries: require MerkleProof.verifymerkleProof, mintlistMerkleRoot1, leaf || MerkleProof.verifymerkleProof, mintlistMerkleRoot2...

Start an auction without an NFT

Lines of code Vulnerability details Impact Requires that the account being liquidated has more than one NFT in PaprController. When an operator calls purchaseLiquidationAuctionNFT the price variable is determined by a call to purchaseNFTAndUpdateVaultIfNeeded which calls purchaseNFT which calls...

Early NFT collections such as CryptoPunks and MoonCats can not be used as a collateral

Lines of code Vulnerability details Impact Early NFT collections such as CryptoPunks, EtherRocks and MoonCats are not compliant with ERC721 standard. therefore, they can not be used as a collateral since transferring will fail. I beleive it would be a huge advantage for the protocol to support su...

Potential DOS in removeCollateral

Lines of code Vulnerability details Impact Function removeCollateral may fail under certain circumstances, potentially causing DOS to user trying to withdraw their collateral asset. This action may be time critical and may cause the user to lose funds due to price change etc. Proof of Concept Cal...

Protocol faces truncation issue in some places due to solidity integer division

Lines of code Vulnerability details Impact UniswapOracleFundingRateController.sol : periodRatio , targetMarkRatio - less multiplier output than the actual value that is possible. EDAPrice.sol: again the final outcome affects the multiplier. Please refer the code link in POC. PaprController.sol:...

Some NFTs could be possibly stuck and can not be withdrawn when removing the collateral

Lines of code Vulnerability details Impact When adding a collateral, the protocol transfers the NFT from the user to the contract by calling ERC721's transferFrom, and when removing the collateral, safeTransferFrom is called to allow for onReceive hook to be triggered as intended by the protocol...

Signature mallebaility in underwritePriceForCollateral

Lines of code Vulnerability details Impact underwritePriceForCollateral has signature malleability as it doesn't check for a returned address of not 0. Value can be set to 0 oracleSigner Proof of Concept function underwritePriceForCollateralERC721 asset, PriceKind priceKind, OracleInfo memory...

When liquidation is not locked, anyone can liquidate another persons' collateral

Lines of code Vulnerability details Impact Petty users can liquidate other people's NFT immediately when the liquidation threshold is reached. Proof of Concept The owner controls the function setLiquidationsLocked and calls the function when a collateral needs to be liquidated function...

function buyAndReduceDebt() spend more underlying token than user specified and also code doesn't check that swapFeeBips is less than BIPS_ONE and user can lose some of his underlying token balance that he gave protocol spending approval

Lines of code Vulnerability details Impact user can specify fee recipient and fee amount to send to that recipient and it is calculated by amount swapFeeBips / BIPSONE but there is no check in the code to make sure swapFeeBips is less than BIPSONE and if user set wrong value by mistake or client...

PaprController is vulnerable to reentrancy attacks

Lines of code Vulnerability details Impact Malicious users can steal PAPR tokens through reentrancy attacks. Proof of Concept Function PaprController.purchaseLiquidationAuctionNFT may trigger a reentrancy becase it calls ERC721.safeTransferFrom in NFTEDA.purchaseNFT:...

PUNK does not conform to ERC721 standard hence safeTransferFrom from solmate does not work

Lines of code Vulnerability details Impact Since protocol's intended NFT target is PUNK currently, the protocol will be hugely affected. Proof of Concept Attempts to transfer the NFT is done in 2 places. 1. removeCollateral 2. purchaseNFT Firstly, this means that once a collateral is added to the...

Users may be liquidated right after taking maximal debt

Lines of code Vulnerability details Impact Since there's no gap between the maximal LTV and the liquidation LTV, user positions may be liquidated as soon as maximal debt is taken, without leaving room for collateral and Papr token prices fluctuations. Users have no chance to add more collateral o...

Operator can buy papr with PaprController as the debtor.

Lines of code Vulnerability details Impact This requires that an NFT is in auction. When an operator calls purchaseLiquidationAuctionNFT the price variable is determined by a call to purchaseNFTAndUpdateVaultIfNeeded which calls purchaseNFT which calls safeTransferFrom on the auctionAssetContract...

NFT operators of OpenZeppelin-based NFTs can issue debt

Lines of code Vulnerability details Description The Papr protocol allows NFT owners to issue debt against NFTs they deposit, denominated in papr. The solmate contracts that are used as imports for the project do not allow an operator on the NFT to participate in debt-creation; the ERC721 contract...