Anyone can use funds in GroupBuy.sol to buy the NFTs for themselves

Lines of code Vulnerability details The GroupBuy contract allows users to pool their funds in order to buy specific NFTs once enough funds have been raised. The purchace function does not do any caller authorization and allows the caller to pass in an arbitrary address for executing the buy. The...

rejectProposal/rejectActive/cancel did not update pendingBalances

Lines of code Vulnerability details Impact In the OptimisticListingSeaport contract, pendingBalances represents the number of Rae tokens sent to the contract by the user when creating the proposal, which can be withdrawn in the withdrawCollateral function. function withdrawCollateraladdress vault...

User will often overpay when adding liquidity

Lines of code Vulnerability details Impact One of the assets either baseTokens or fractionalTokens will likely be overpaid when the user calls Pair.add liquidity. Proof of Concept The Pair.add function takes baseTokenAmount, fractionalTokenAmount and minLpTokenAmount as inputs. The first two...

Users that send funds at a price lower than the current low bid have the funds locked

Lines of code Vulnerability details If a user contributes funds after there is no more supply left, and they don't provide a price higher than the current minimum bid, they will be unable to withdraw their funds while the NFT remains unbought. Impact Ether becomes stuck until and unless the NFT i...

Merkle root of zero bypasses validation

Lines of code Vulnerability details Impact The merkle root validation happens with this require in validateTokenIds. If the merkle root is zero, this check is skipped. Proof of Concept This require validates that the merkle root is valid. This code is never reached if merkleRoot == bytes230 becau...

Surplus token after adding liquidity is not refunded to liquidity providers. LP might suffer front-running attack and lose funds.

Lines of code Vulnerability details Impact Function Pair.add receives base token and fractional token from liquidity providers and mint equivalent amount of LP token for them. The amount of LP token be minted is calculate in function addQuote function addQuoteuint256 baseTokenAmount, uint256...

When user of GroupBuy is a contract, refunds will be permanently frozen.

Lines of code Vulnerability details Description claim function is used in GroupBuy to mint Raes proportional to user's contribution to the purchased NFT. withdrawBalance is used to get back funds which are not part of the contribution. They both contain an unsafe call with ETH. For example:...

GroupBuys that are completely filled still don't raise stated target amount

Lines of code Vulnerability details Description createPool in GroupBuy.sol creates a new contribution pool around an NFT. It specifies a target initialPrice as minimum amount of ETH the NFT will cost, and totalSupply which is the number of Raes to be minted on purchase success. minBidPrices is...

sellQuote might lead to unexpected reverts when returning zero

Lines of code Vulnerability details Impact the implementation of sellQuote follows the logic of getAmountOut from Uniswap V2 Library. However it doesn't add +1 as the original implementation, this could make the function return zero and lead to reverts. Proof of Concept Tools Used Manual review...

Proposal can be infinitely DOSed with no additional conditions

Lines of code Vulnerability details Description OptimisticListingSeaport.sol is easily DOSable due to the following conditions: 1. A new proposal overrides the existing proposal 2. The deposited collateral can be any non-zero amount 3. Only requirement is that pricePerToken is lower than before. ...

Reentrancy in GroupBuy.purchase allows buying NFT twice

Lines of code Vulnerability details Impact In GroupBuy.purchase, poolInfopoolId.success which prevents buying the same NFT again is only set to true after the sale was executed. This can be exploited by reentering in the following line: address vault = IMarketBuyermarket.executevalue:...

OptimisticListingSeaport.propose sets pendingBalances of newly added proposer instead of previous one

Lines of code Vulnerability details Impact In OptimisticListingSeaport.propose, pendingBalances is set to the collateral. The purpose of this is that the proposer of a previous proposal can withdraw his collateral afterwards. However, this is done on the storage variable proposedListing after the...

GroupBuy: Lost ETH when the NFT is bought for less than the minimum reserve price

Lines of code Vulnerability details Impact The purchase function does not require that an NFT is bought for exactly minReservePricespoolId filledQuantitiespoolId, the price is only not allowed to be greater: if price minReservePricespoolId filledQuantitiespoolId revert InvalidPurchase; This makes...

User can provide malicious _market in GroupBuy.purchase to steal funds or NFT

Lines of code Vulnerability details Impact The argument market of GroupBuy.purchase is not validated. The following call is directly performed on it: address vault = IMarketBuyermarket.executevalue: pricepurchaseOrder; Then, it is checked that the returned address owns the NFT: if...

Groupbuy: Construction of merkle tree allows some unintended IDs to be bought

Lines of code Vulnerability details Impact In GroupBuy.purchase, when no proof is provided, it is required that the provided token ID is equal to the stored merkleRoot: if purchaseProof.length == 0 // Hashes tokenId to verify merkle root if proof is empty if bytes32tokenId != merkleRoot revert...

Groupbuy: _verifyUnsuccessfulState and _verifySuccessfulState both can return true when block.timestamp == pool.terminationPeriod

Lines of code Vulnerability details Impact The functions verifyUnsuccessfulState and verifySuccessfulState should always have a differing behavior with regards to reversion, i.e. when one does not revert, the other should revert. In one condition, this is not true. Namely, when we have pool.succe...

GroupBuy does not check return value of call

Lines of code Vulnerability details Impact Both usages of call do not check if the transfer of ETH was succesful: payablemsg.sender.callvalue: contribution""; ... payablemsg.sender.callvalue: balance""; This can become very problematic when the recipient is a smart contract that reverts for...

GroupBuy may purchase NFT not in the allowed list

Lines of code Vulnerability details Impact When purchaseProof.length == 0, GroupBuy.purchase compare the tokenId with the merkleRoot. This allow any tokenId that match the merkleRoot to be purchased, even if they are not included in the allow list during setup. if purchaseProof.length == 0 //...

OptimisticListingSeaport: pendingBalances overwritten instead of increased

Lines of code Vulnerability details Impact The functions list and cash overwrite the current value instead of increasing it: pendingBalancesvaultactiveListing.proposer = activeListing.collateral; pendingBalancesvaultactiveListing.proposer = collateral; This can be very problematic because the val...

Upgraded Q -> M from #315 [1671270502271]

Judge has assessed an item in Issue 315 as M risk. The relevant finding follows: Lines of code Vulnerability details Description Collateral.sol exposes a permissioned withdraw function: function managerWithdrawuint256 amount external override onlyRoleMANAGERWITHDRAWROLE nonReentrant if...

Owners can delay the call to startDraw to reject the draw result

Lines of code Vulnerability details Impact Owners can call lastResortTimelockOwnerClaimNFT to reclaim NFT back if settings.recoverTimelock block.timestamp // Stop the withdraw revert RECOVERYISNOTYETPOSSIBLE; ... If an owner delays the startDraw until settings.recoverTimelock, the owner can call...

winnerClaimNFT() does not check if recipient can receive ERC721

Lines of code Vulnerability details Impact In src\VRFNFTRandomDraw.sol winnerClaimNFT, you should use safeTransferFrom to protect against the case that the recipient cannot receive the ERC721 tokens, and thus will lose the tokens. This is more-so the case when we expect that normal users will be...

The transferFrom() method is used instead of safeTransferFrom(), which I assume is a gas-saving measure. I however argue that this isn’t recommended because:

Lines of code Vulnerability details Vulnerability Detail OpenZeppelin’s documentation discourages the use of transferFrom; use safeTransferFrom whenever possible The recipient could have logic in the startDraw, fwinnerClaimNFT, lastResortTimelockOwnerClaimNFT, function src/VRFNFTRandomDraw.sol-18...

If the subscription does not have enough funds the winner can not be chosen

Lines of code Vulnerability details Impact VRF's subscriptionId can be chosen by the drawer, and it is the drawer's responsibility to fund the subscription. If the drawer, maliciously or not, fails to do so, the draw will result in no winners, and the drawer can re-claim the offered NFT...

IERC20.transfer does not support all ERC20 token

Lines of code Vulnerability details Impact Proof of Concept Token like USDT known for using non-standard ERC20. Missing return boolean on transfer. Contract function handleDeposit will always revert when try to transferfrom this kind of tokens. Tools Used manual review Recommended Mitigation Step...

Error when handling deposit in the addToPosition function

Lines of code Vulnerability details Impact In the function addToPosition from the Trading contract the amount of open fees are handled using the handleOpenFees function but when calling the handleDeposit function the wrong margin is passed, in fact the handleDeposit function gets addMargin - fee...

Users can bypass the maxWinPercent limit using a partially closing

Lines of code Vulnerability details Impact Users can bypass the maxWinPercent limit using a partial closing. As a result, users can receive more funds than their upper limit from the protocol. Proof of Concept As we can see from the documentation, there is limitation of a maximum PnL. Maximum PnL...

Delayed contests might be rugpulled by admin

Lines of code Vulnerability details In order to guarantee the raffle NFT will not be stuck in the contract, there’s a last resort option which allows the admin the reclaim the NFT from the contract. However, the timestamp as from which this action can take place, is calculated based on when the...

recoverTimelock can have a value shorter than the winner's deadline to claim

Lines of code Vulnerability details Impact The value of recoverTimelock is checked to be greater than a week and less than a year, but it should never be allowed to be shorter than block.timestamp + settings.drawBufferTime, which is the time given to the winner to claim the NFT. Otherwise it coul...

After the redrawing the winner can lose his NFT for the Admin

Lines of code Vulnerability details Impact The owner can manipulate the raffle by deprivation of a specific address winner Proof of Concept Please copy the following POC on VRFNFTRandomDraw.t.sol function testadminRedrawingandReclaimnft public address winner = address0x1337; address winner2 =...

Attackers can obtain rewards through the NFT of the flash loan winning ID

Lines of code Vulnerability details Impact The contract judges whether the user has won a prize, but only judges whether the owner of the nft with the specified ID is equal to the user's address user == IERC721EnumerableUpgradeablesettings.drawingToken.ownerOf request.currentChosenTokenId ; But i...

GovNFT contract's owner can stop Governance NFT holders from receiving more rewards from trades' DAO fees, and such reward amounts can remain in Trading contract without belonging to anyone

Lines of code Vulnerability details Impact According to , "Profits from trading fees are paid out to Governance NFT holders in real-time...Rewards are paid out in Tigris stablecoins." However, for some legitimate reasons, such as if the corresponding Tigris stablecoin has a bug, or if the owner o...

Minter privilege escalation vulnerability

Lines of code Vulnerability details Impact The contract owner could potentially abuse their privileges to manipulate the token supply and undermine the integrity of the token economy. Proof of Concept The 'setMinter' function in the contract allows the contract owner to set the minter status for...

Draw organizer can rig the draw to favor certain participants such as their own account.

Lines of code Vulnerability details Description In RandomDraw, the host initiates a draw using startDraw or redraw if the redraw draw expiry has passed. Actual use of Chainlink oracle is done in requestRoll: request.currentChainlinkRequestId = coordinator.requestRandomWords keyHash:...

Token decimals may be above 18 leading to underflow issues

Lines of code Vulnerability details Impact In both StableVault and Trading, there is an assumption that token decimal amounts are =18. However this can break if token decimals are 18, resulting in underflow which can lead to unpredictable behaviors, and extreme amounts during deposits and...

The parameters in the setting of the draw are in consistent

Lines of code Vulnerability details Impact When initializing a draw, parameters in settings can be easily set to inconsistent, which can lead to the winner cannot claim the NFT even before reaching the drawBufferTime – time until a re-drawing can occur if the selected user cannot or does not clai...

Not enough margin pulled or burned from user when adding to a position

Lines of code Vulnerability details Impact When adding to a position, the amount of margin pulled from the user is not as much as it should be, which leaks value from the protocol and lowering the collateralization ratio of tigAsset. Proof of Concept In Trading.addToPosition the handleDeposit...

INITREFS() COULD BE USED TO OVERWRITE ALL REFFERAL DATA

Lines of code Vulnerability details Impact Although denoted as deprecated, initRefs is readily there in Referrals.sol that could be called by the owner whose private keys might have been compromised to reassign all the referral data. This could impact the referrers specifically who might have...

_priceData.price is not verified in _limitClose

Lines of code Vulnerability details Impact In the function limitClose from the TradingExtension contract the priceData.price is not verified with the getVerifiedPrice function instead its value is directly used, and because the the getVerifiedPrice internally calls the function...

Deposits would revert

Lines of code Vulnerability details Impact When the margin asset is USDT, after the first deposit all following ones would revert allowing no more trades. Proof of Concept The handleDeposit function in Trading.sol's Trading contract is calling approve inconditionally at every deposit. The USDT...

Wrong calculation of MONTH_IN_SECONDS

Lines of code Vulnerability details Wrong calculation of MONTHINSECONDS : line 33 on VRFNFTRandomDraw.sol is calculating 30 weeks of 7 days instead of 30 days. 30 weeks of 7 days = 3600 24 7 30. 30 days = 3600 24 30. so and wont work properly --- The text was updated successfully, but these error...

Wrong position size calculation in TradingLibrary.pnl()

Lines of code Vulnerability details Impact Users will pay less closing fees than they should when they have a profitable short position. Also, they will pay more fees when they have a lost short position. Proof of Concept TradingLibrary.pnl calculates the new position size like below. function...

Admin can withdraw the NFT before the winner timelock ends

Lines of code Vulnerability details Impact The admin could set recoverTimelock before drawBufferTime , thus he can withdraw the NFT before the winner Draw buffer time ends. Proof of Concept The drawBufferTime need to be more then an hour and less then a month and the recoverTimelock need to be at...

Function fulfillRandomWords in VRFNFTRandomDraw contract must not revert

Lines of code Vulnerability details The VRFNFTRandomDraw contract implements the Chainlink VFR feature to pull random data to select the raffle winner. As per their security guidelines the implementation of the fulfillRandomWords function must not revert. Impact If the fulfillRandomWords function...

Lack of validation on price feeds

Lines of code Vulnerability details Impact Lack of validation on Chainlink price feeds may result in incorrectly functioning or non-functioning protocol. For example: during high volatility a price feed may be suspended or become stale; on L2 networks the sequencer might be down on L2 networks th...

Unreleased locks cause the reward distribution to be flawed in BondNFT

Lines of code Vulnerability details Impact After a lock has expired, it doesn't get any rewards distributed to it. But, unreleased locks cause other existing bonds to not receive the full amount of tokens either. The issue is that as long as the bond is not released, the totalShares value isn't...

Ignored return value from "IERC20.transferFrom()"

Lines of code Vulnerability details Impact The return value from IERC20.transferFrom in "StableVault" was not checked, a malicious actor could first deposit a number of tokens without actually having it and then subsequently withdraw that amount of tokens from the "StableVault" Proof of Concept...

Raffle is fair only if tokenRange is a power of 2.

Lines of code Vulnerability details Impact Based on the contract logic, the raffle works by sampling a value $ x \in \mathbbZ | 0 \leq x \lt 2^256 $ aka uint256 from a uniform distribution provided by Chainlink VRF, then applying modulo tokenRange, generating a new value $ y \in \mathbbZ | 0 \leq...

[NAZ-M1] fulfillRandomWords() Must Not Revert

Lines of code Vulnerability details Impact In the current implementation of fulfillRandomWords can revert which it is not supposed to. Proof of Concept In the Chainlink documentation it states: "If your fulfillRandomWords implementation reverts, the VRF service will not attempt to call it a secon...

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