[NAZ-M2] First ERC4626Upgradeable Deposit Exploit Can Break Share Calculation

Lines of code Vulnerability details Impact ERC4626Upgradeable is an upgradeable version of Solmate's ERC4626 Token. Solmate's convertToShares function follow the formula: assetDepositAmount totalShareSupply / assetBalanceBeforeDeposit. The share price always return 1:1 with asset token. If...

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

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

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

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

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

MinipoolManager.sol : createMinipool does not check the owner of node when overwriting

Lines of code Vulnerability details Impact owner will be deprived from picking the node id which they already used. when natspec says // If nodeID exists, only allow overwriting if node is finished or canceled The pool that are marked as Withdrawable and Error are also used to overwrite the node ...

EXTERNAL CALL TO UNTRUSTED PARTIES

Lines of code Vulnerability details Impact Functions in this contract or inherited functions can be reentered. Proof of Concept In Vault.sol the withdrawAVAX and redeemAVAX functions make an external call to the msg.sender by way of safeTransferETH. This allows the caller to reenter this and othe...

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

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

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

PROTOCOL MIGHT NOT BE ABLE TO OPERATE DUE TO LACK OF FUNDS

Lines of code Vulnerability details Impact Redeem/withdraw functionality will fail under certain conditions and users who want to redeem/withdraw their AVAX will not be able to. Proof of Concept Users stake their AVAX and in return get ggAVAX. The AVAX provided by the users is then staked by the...

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

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

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

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

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

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

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

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

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

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

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

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

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

SOLMATE SAFETRANSFER AND SAFETRANSFERFROM DOES NOT CHECK THE CODESIZE OF THE TOKEN ADDRESS, WHICH MAY LEAD TO FUND LOSS

Lines of code Vulnerability details Impact Reference from a previous Contrest : In uniswapV3SwapCallback and sendPaprFromAuctionFees the safetransfer and safetransferfrom doesn’t check the existence of code at the token address. This is a known issue while using solmate’s libraries. Hence this ma...

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

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

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

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

Extensive permissions for owner

Lines of code Vulnerability details Impact & Proof Of Concepts / Implications Some privileged functions are often unavoidable in smart contracts. However, in these contracts, the privileges are unnecessarily very extensive and without checks on the smart contract side: 1. He can use ownerMint or...

A BETTER APPROACH TO REVERTING CODE LINES ON _ADDCOLLATERALTOVAULT() AND _REMOVECOLLATERAL()

Lines of code Vulnerability details Impact In PaperControl.sol, callers would have to deal with function pre-maturely reverting upon encountering an element in collateralArr failing to pass the if statement in the function logic of addCollateralToVault and removeCollateral. The amount of gas wast...

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

incorrect message signer check in ecrecover() and no validation for oracleSigner which would cause all the unsigned messagess to be considered asvalid ones

Lines of code Vulnerability details Impact address oracleSigner has been used to verify signed messages by comparing the return value of ecrecover by oracleSigner. but the return value of ecrecover would be 0x0 and code doesn't check that return value is not 0x0. if owner set oracleSigner as 0x0 ...

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

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

code doesn't check that the Uniswap pool is not exist and if attacker creates the uniswap pool before hand with calculatable papr address and underlying address and exact fee amount, then the deployment transaction would fail always

Lines of code Vulnerability details Impact attacker can perform DOS and prevent contract from getting deployed. the address of papr can be calculate by attacker before its deployment and the address of the underlyingUSDC and the fee is known, attacker can create uniswap pool before-hand and cause...

Contract owner can drain all NFT collateral

Lines of code Vulnerability details Impact To add NFTs as collateral to the protocol and it to create debt, the owner has to approve each NFT contract address first. As anyone can create their own papr vaults with arbitrary allowed NFTs by design, this opens the possibility for malicious actors t...

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

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

PaprController should not pay the swap fee in buyAndReduceDebt

Lines of code Vulnerability details Impact The best case scenario is that the paprController doesn't have any underlying tokens, in which case, buyAndReduceDebt won't work when there is a swapFee. Otherwise, paprController ends up paying for the swapFee. Even if there isn't a swap fee, the user c...

There is no way to extract fees when someones wants to reduce a debt by paying with underlying tokens

Lines of code Vulnerability details Impact When a user is buying the debt of an account or its own debt, it either have the choice to use the reduceDebt function in order to pay with PAPR tokens, or by paying in underlying tokens can be USDC, WETH, .... A protocol that would choose to...

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

setPlotsAvailablePerSize does not work correctly

Lines of code Vulnerability details Impact The function setPlotsAvailablePerSize can be used for two things: 1. Decreasing the number of plots that is available for a certain size 2. Increase the number of plots that is available for a certain size However, in both cases it can introduce errors...

Initial value of auction starter discount is set wrong

Lines of code Vulnerability details Impact The protocol offers a discount for auction starters when purchasing the liquidated NFT as an incentive for users to start an auction. I believe the intended discount should be 10%, but is currently implemented at 90%. 90% is too massive a discount for th...

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

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

Faulty fee handling in buyAndReduceDebt

Lines of code Vulnerability details Impact When passing fee params to buyAndReduceDebt with swapFeeTo and swapFeeBips, the PaprController will try to send the underlying token on the following line: underlying.transferparams.swapFeeTo, amountIn params.swapFeeBips / BIPSONE; But in a normal state,...