Frontrunning the winnerClaimNFT is possible

Lines of code Vulnerability details Impact The winner in the NFT raffle may be frontrun if he has listed his NFT for sale on a marketplace, stealing his raffle NFT. Proof of Concept A new VRFNFTRandomDraw Clone contract is deployed using makeNewDraw, with some NFT collection as drawingToken and...

ERC20 can be mistakenly used instead of ERC721

Lines of code Vulnerability details Impact // Attempt to transfer token into this address try IERC721EnumerableUpgradeablesettings.token.transferFrom // @audit could use ERC20 here msg.sender, addressthis, settings.tokenId catch revert TOKENNEEDSTOBEAPPROVEDTOCONTRACT; Both ERC20 and ERC721 has t...

In case the winner is the address(0)

Lines of code Vulnerability details Impact Temporary freezing NFT this can be more than one period Proof of Concept On VRFNFTRandomDraw.fulfillRandomWords 254 request.currentChosenTokenId = 255 randomWords0 % tokenRange + 256 settings.drawingTokenStartId; In case ownerOfrequest.currentChosenToken...

Owner can potentially prevent winner from claiming by starting draw after recover timelock and then calling lastResortTimelockOwnerClaimNFT

Lines of code Vulnerability details Impact The owner can call startDraw at any time, including after the recoverTimelock has expired, which means the owner can call lastResortTimelockOwnerClaimNFT and potentially reclaim the NFT immediately after the draw completes if they do not want the winner ...

Prize token can overlap with drawingToken and prize tokens from other draws.

Lines of code Vulnerability details Impact Creators can create multiple draws with the same prize, but only the first draw to call startDraw will have the prize. This can trick users into entering raffle pools that does not have a prize. Furthermore, the prize token can also be one of the tokens ...

Truncate of values can be avoided

Lines of code Vulnerability details Truncate of values can be avoided Summary Solidity integer division might truncate. As a result, performing multiplication before division can sometimes avoid loss of precision. Details In general, this is a problem due to precision. In this case, it also affec...

Use safeTransferFrom instead of transferFrom for ERC721 transfers

Lines of code Vulnerability details Impact In the contract VRFNFTRandomDraw.sol every transfer of ERC721 are done with the transferFrom instead of the recommended safeTransferFrom. This transferFrom does not check whether the receiver is capable of proper handling of NFTs. Proof of Concept If the...

Might lose manager forever

Lines of code Vulnerability details Impact If project sets new manager with a typo, might lose manager forever Proof of Concept function setManager address manager public onlyOwner manager = manager; https://twitter.com/realgmhacker/status/1603362870699429889?s=20&t=vm4wY1ITefLhCW8BDa0oig...

lastResortTimelockOwnerClaimNFT() can retrieve the token although drawing is in progress

Lines of code Vulnerability details Impact owner can retrieve the token although drawing is in progress Proof of Concept lastResortTimelockOwnerClaimNFT Be used in: " If no users ultimately claim the NFT, the admin specifies a timelock period after which they can retrieve the raffled NFT. " But i...

Choosing a cheap gas lane may result in no winners

Lines of code Vulnerability details Impact The drawer can choose whichever gas lanekeyHash they like. Giving this choice to the drawer may result in no winners if the network is congested and the drawer chooses a cheap gas lane. Recommended Mitigation Steps Check the options of keyHashes that can...

integer overflow or underflow

Lines of code Vulnerability details Impact If an integer overflow or underflow occurs in the contract, it could lead to incorrect calculations and potentially unintended consequences, such as the transfer of incorrect amounts of tokens or the allocation of incorrect amounts of rewards. This could...

Functions of Trading contract can be reentered by Position.sol#mint

Lines of code Vulnerability details Impact Both the contracts of Position and Trading may not work correctly. Proof of Concept The Position.solmint calls safeMint will trigger a checkOnERC721Received callback, which can be used to reenter. Crackers can use this vulnerability to attack the protoco...

Lack of access control

Lines of code Vulnerability details The 'createReferralCode' function in the 'Referrals' contract allows any address to create a referral code. This could potentially lead to spam or misuse of the system. Impact If an attacker is able to create a large number of referral codes, they could...

Pending owner can front-run current owner when current owner wants to cancel the ownership transfer.

Lines of code Vulnerability details Impact OwnableUpgradable contract has been modified to transfer ownership in a two-step ownership transfer way. This introduces an issue of front-run when admin adds a pendingOwner but later on decides to cancel the ownership transfer. Pending owner can become...

owner can withdraw the NFT at any time if they wait with starting the draw until after recoverTimelock

Lines of code Vulnerability details Description When creating a random draw the owner specifices a recoverTimelock which is a last resort option to recover the raffled NFT if the draw fails. There are some validations that this is between a week and a year in the future but there's no guarantee...

Unprotected contract

Lines of code Vulnerability details Impact Anyone can call makeNewDraw and become admin. src/VRFNFTRandomDrawFactory.sol function makeNewDrawIVRFNFTRandomDraw.Settings memory settings external returns address address admin = msg.sender; // Clone the contract address newDrawing =...

Draw admin/owner can rug the winner after recoverTimelock expires.

Lines of code Vulnerability details Impact The admin/owner of VRFNFTRandomDraw can wait for recoverTimelock to expire before making the draw. This way he can use lastResortTimelockOwnerClaimNFT to take back the reward NFT from the contract without any time to allow for the winner to claim. He cou...

Draw can be configured without uncancellable-redraw mechanism

Lines of code Vulnerability details Description RandomDraw initialize contains several checks for the range of drawBufferTime and recoveryTimelock. redraw buffer time can be between 1 hour and 1 month, while recoveryTimelock is between 1 week and 1 year from now. The issue is that the relative...

Wrong address input in BondNFT.createLock results in wrong _owner being saved in memory

Lines of code Vulnerability details Impact Wrong address input in createLock results in function failure of extendLock and claim. Proof of Concept When someone calls lock in Lock.sol, the function BondNFT.createLock is called . bondNFT.createLock asset, amount, period, msg.sender;...

Miners Can Re-Roll the VRF Output to Game the Protocol

Lines of code Vulnerability details Impact Miners are able to rewrite a chain's history if they dislike the VRF output used by the protocol. Consider the following example: A miner or well-funded user is participating in the PoolTogether protocol. A VRF request is made and fulfilled in the same...

safeTransfer should be used instead of transferFrom in winnerClaimNFT

Lines of code Vulnerability details Impact The winner of the raffle may be a smart contract which doesn't handle NFTs, thus leaving the NFT irretrievably lost instead of being awarded to an address which can handle NFTs. Additionally, the NFT should be in the VRFNFTRandomDraw contract at the time...

The recoverTimelock does not sufficiently protect against an admin withdrawing the NFT before a user is able to claim it

Lines of code Vulnerability details Impact When a VRFNFTRandomDraw contract is initialized, the recoverTimelock variable is set. The variable should be used to prevent the admin from calling the lastResortTimelockOwnerClaimNFT function before a certain amount of time has passed to ensure that the...

Potential race condition when claiming prize

Lines of code Vulnerability details Impact To determine whether a caller has won the raffle, the hasUserWon function of the VRFNFTRandomDraw contract queries the ownerOf function of the drawingToken contract, passing the currentChosenTokenId as argument. In essence, the function is only checking...

A compromised owner of VRFNFTRandomDraw can claim the NFT to another accomplice addresss

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. A compromised owner of VRFNFTRandomDraw can claim the NFT to another accomplice addresss Proof of Concept Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other...

LOWER BOUNDARY OF DRAWING TOKEN RANGE IS TOO LOW

Lines of code Vulnerability details Impact The current logic in VRFNFTRandomDraw.sol could lead to undesirable edge cases due to allowing the lower limit of the drawing token range to be as low as 2. It could lead to a long drag before the raffle could end or cancel if one of the drawing tokens i...

Wrong value of MONTH_IN_SECONDS could make it impossible to recover NFT in 7 years

Lines of code Vulnerability details Impact Constant MONTHINSECONDS has incorrect value. Instead of 1 month, it has the value of 7 months. // @dev about 30 days in a month uint256 immutable MONTHINSECONDS = 3600 24 7 30; // @audit wrong value, could allow bufferTime and recoverTimelock become too...

Attacker can disable contract functionality

Lines of code Vulnerability details Impact Current setup of the protocol is vulnerable to a DoS attack. This can be achieved by anyone calling initialize on the implementation VRFNFTRandomDraw contract. With the implementation contract initialized the created clones cannot be re-initialized and...

Malicious ChainLink's VRF manager can decide to not whitelist VRFNFTRandomDraw or brick ongoing raffles

Lines of code Vulnerability details Impact ChainLinks VRF manager has priviledged position, as all VRFNFTRandomDraw instances share the same VRFCoordinatorV2 address, and have to be whitelisted in order to be able to send requestRandomWords function. There is centralization risk in this case, tha...

LP Rewards can be increased infinitely by a malicious liquidity provider

Lines of code Vulnerability details Impact LP Rewards can be increased infinitely by a malicious liquidity provider Proof of Concept A Bond NFT holder can claim pending rewards from a bond using the function Lock.claim. function claim uint256 id public returns address claimGovFees; uint amount,...

Bad actor can burn VRFCoordinatorV2 LINK's balance

Lines of code Vulnerability details Impact All VRFNFTRandomDraw instances share the same VRFCoordinatorV2 address, and LINK fees are deducted from this smart contract. There are a couple of issues that may be exploited by malicious actor to overinflate calls to VRF and in lead to griefing, e.g. B...

Error in trade accumulated interest calculation

Lines of code Vulnerability details Impact The trade's accumulated interest may result smaller than expected when long open interest is zero. Proof of Concept function tradesuint id public view returns Trade memory ... int256 pendingFunding; if trade.direction && longOi...... 0 pendingFunding =...

Draw organizer can time draws so that user's have the illusion of fair random, but draw can be cancelled.

Lines of code Vulnerability details Description In RandomDraw, host can call startDraw or redraw to request a Chainlink random number, which will be used to select the winning user. They may then collect the prize NFT using winnerClaimNFT. The issue is that in the two draw functions, it is never...

Use of resignOwnership can lead to stuck NFT in contract

Lines of code Vulnerability details Impact Contract OwnableUpgradeable has a resignOwnership function that, if called, can potentially cause the loss of the NFT after a draw has been started. Proof of Concept These are the steps/conditions that make this issue happen: When the owner calls...

The way the winner claiming NFT is vulnerable to flashloan attack.

Lines of code Vulnerability details Impact The way the winner claiming NFT is vulnerable to flashloan attack. Proof of Concept This kind of attack is similar to ApeCoin Airdrop attack. To determine who is winng the current drawing, there is a hashUserWon function: function hasUserWonaddress user...

Extending period in Lock contract will block user funds

Lines of code Vulnerability details Impact Extending lock will block users' funds. Contract: Lock.sol Proof of Concept Locking tokens with function lock... will update mapping mapping totalLocked for given asset with value of amount, here: totalLockedasset += amount. However, when extending lock...

DoS after creating 100 raffles under one subscriptionID

Lines of code Vulnerability details Impact If a user adds new consumer, function VRFCoordinatorV2::addConsumer is called: function addConsumeruint64 subId, address consumer external override onlySubOwnersubId nonReentrant // Already maxed, cannot add any more consumers. if...

The owner can swap the proxy implementation with a malicious one

Lines of code Vulnerability details The owner of VRFNFTRandomDrawFactory.sol could swap the current implementation with a malicious one at any moment, without a waiting period. Impact The worse case scenario is one in which the private key of the contract owner gets stolen. In this case the owner...

Winning NFT owner/admin can rug pull attack or DoS attack on winner by removing the winning NFT.

Lines of code Vulnerability details Impact Given the current logic, it is possible to call the redraw method even after recoverTimelock has passed. If the owner does so, the contract will select a new winner for the winning NFT. But it will be up to the owner to give as much time to the winner to...

The NFT can be transferred to the owner immediately after startDraw()

Lines of code Vulnerability details lastResortTimelockOwnerClaimNFT as the name says is used in case the winning user doesn't retrieve the won NFT token and in such case the owner can rescue the NFT from the contract. The mentioned function can be only called after a certain period is passed: if...

Contract Owner Possesses Too Many Privileges

Lines of code Vulnerability details Impact The owner has many privileges in the contract: setBlockDelay, setAllowedVault, setMaxWinPorcent, setLimitOrdenPriceRange, setFees, setTradingExtension Proof of Concept function setBlockDelay uint blockDelay external onlyOwner blockDelay = blockDelay; /...

[NAZ-H1] Reentrancy From Use of _safeMint()

Lines of code Vulnerability details Impact The mint function is used to mint a new position NFT and is only callable by minter. This function uses safeMint to mint these position NFTs which has been known in the past to have Reentrancy issues. Proof of Concept The dangers of surprising code by...

Governance NFT holder, whose NFT was minted before Trading._handleOpenFees function is called, can lose deserved rewards after Trading._handleOpenFees function is called

Lines of code Vulnerability details Impact Calling the following Trading.handleOpenFees function does not approve the GovNFT contract for spending any of the Trading contract's tigAsset balance, which is unlike calling the Trading.handleCloseFees function below that executes...

Distribute is open to rewards manipulation

Lines of code Vulnerability details Impact The distribute function is prone to manipulation by the first depositor if the totalShares is low, since the result of transferFrom in distribute is not checked. This can happen if a malicious user calls createLock whereby shares = 1 then calls distribut...

Transfering GovNFT does not update mapping userDebt

Lines of code Vulnerability details Impact Because of not updating the mapping userDebt for receiver userDebtfor on the transfer of GovNFT token, the receiver may be harmed, because of not getting collected fees. Proof of Concept Function transfer... is not updated for userDebtfor, here. Tools Us...

Chainlink price feed is not sufficiently validated and can return stale price

Lines of code Vulnerability details Impact As mentioned by , "Prices provided by the oracle network are also compared to Chainlink's public price feeds for additional security. If prices have more than a 2% difference the transaction is reverted." The Chainlink price verification logic in the...

Unable to release funds deposited through Lock-extendLock

Lines of code Vulnerability details Impact Users depositing tig assets to the bondNFT through the Lock contract could find themselves unable to release their funds. The only explanation the user would receive is an "Arithmetic over/underflow" error. This issue greatly deteriorates the user...

Wrong deposit amount in Trading.addToPosition()

Lines of code Vulnerability details Impact Users should deposit a full amount of addMargin but it deducts fee now. As a result, users will pay less funds than they should. Proof of Concept In addToPosition, users deposits addMargin - fee after the fee calculation. File:...

Wrong update of stoploss in TradingExtension._limitClose()

Lines of code Vulnerability details Impact TradingExtension.limitClose returns a wrong stoploss which is favorable for users and it would be a significant loss for the protocol. Proof of Concept TradingExtension.limitClose is used to set takeprofit/stoploss prices for the pending order and execut...

Lack of Input Validation

Lines of code Vulnerability details Impact Without proper input validation, it is possible for attackers to pass malicious input to the contract, potentially causing unintended behavior or even allowing the attacker to exploit the contract. Proof of Concept an attacker could pass a negative value...

The raffle could be slightly unfair as the owner of NFT ID which is closer to drawingTokenStartId could have more chance to win

Lines of code Vulnerability details Impact The raffle could be slightly unfair as the owner of NFT ID which is closer to drawingTokenStartId could have more chance to win. Proof of Concept As written in , "We want to raffle away a single NFT token based off of another NFT collection or drawingTok...