[WP-H3] S2S Transfer from the origin schain to another schain with automatic deploy disabled can cause funds to be frozen

Lines of code Vulnerability details When moving tokens that are native on the origin schain, to another schain, TokenManagerERC20.soltransferToSchainERC20 will be called, which calls exit - receiveERC20: if isMainChainToken data = receiveERC20 chainHash, addresscontractOnSchain, msg.sender, amoun...

DepositBoxERC20 does not support fee-on-transfer token

Lines of code Vulnerability details Impact The transfered amount is saved without checking the actual amount of token received after the transfer. Proof of Concept saveTransferredAmountschainHash, erc20OnMainnet, amount; require ERC20Upgradeableerc20OnMainnet.transferFrom msg.sender, addressthis,...

areOperatorsImported has incorrect logic

This issue has been created to upgrade a QA report submission to a medium severity finding. From 0xliumin: areOperatorsImported has incorrect logic Right now, this function returns false if the implementation AND the selector don't match. It's possible to provide a destination with either a...

BURNER_ROLE can burn any amount of EthErc20 from an arbitrary address

Lines of code Vulnerability details Proof of Concept Using the forceBurn function of EthErc20, an address with BURNERROLE can burn an arbitrary amount of tokens from any address. We believe this is unnecessary and poses a serious centralization risk. A malicious or compromised BURNERROLE address...

function rebuildCache() in MixinOperatorResolver does not delete removed operators from operatorCache

This issue has been created to upgrade a QA report submission to a medium severity finding. From pauliax: function rebuildCache in MixinOperatorResolver does not delete removed operators from operatorCache. resolverOperatorsRequired return current active operators, so it will not contain removed...

Check if a value is in an array before a push

This issue has been created to upgrade a QA report submission to a medium severity finding. From Dravee: Check if a value is in an array before a push In NestedRecords.sol's store function, it's possible to push an existing address token several times in the same array File: NestedRecords.sol 130...

Schain owners can rug pull users' funds

Lines of code Vulnerability details Impact Once a chain has been killed the chain owner is able to call getFunds on each of the deposit boxes and transfer funds/tokens wherever he/she wishes Even if the owner is benevolent the fact that there is a rug vector available may negatively impact the...

Private sale replay attack

Lines of code Vulnerability details Impact The private sale signature used in buyFromPrivateSaleFor does not prevent replay attacks. If the NFT ends up in the original seller's wallet again within the deadline it can be purchased from them again. POC S sells to B using a private sale. S creates a...

Escrowed NFT can be stolen by anyone if no active buyPrice or auction exists for it

Lines of code Vulnerability details Impact If a NFT happens to be in escrow with neither buyPrice, nor auction being initialised for it, there is a way to obtain it for free by any actor via makeOffer, acceptOffer combination. I.e. a malicious user can track the FNDNFTMarket contract and obtain a...

NFT Creators Can Avoid Paying The Primary Foundation Fee

Lines of code Vulnerability details Impact The primary foundation fee is charged on the initial sale of newly minted NFTs. However, the isCreator && !nftContractToTokenIdToFirstSaleCompletednftContracttokenId check can easily be bypassed to avoid paying the primary foundation fee by selling the...

[WP-M5] Royalties can be distribution unfairly among creatorRecipients for NFT contracts with non-standard getRoyalties() returns

Lines of code Vulnerability details Based on our research, getRoyalties is not a standardized API for NFT contracts to indicate how the royalties should be distributed among the recipients. However, in the current implementation, it always assumes that getRoyalties return in terms of BPS. if...

[WP-M6] Inappropriate support of EIP-2981

Lines of code Vulnerability details if nftContract.supportsERC165InterfacetypeIRoyaltyInfo.interfaceId try IRoyaltyInfonftContract.royaltyInfo gas: READONLYGASLIMIT tokenId, BASISPOINTS returns address receiver, uint256 / royaltyAmount / if receiver != address0 recipients = new address payable;...

Users Can Lock Funds by Backing Out of an Auction

Lines of code Vulnerability details Impact The createReserveAuction function allows users to create duplicate auctions with the same NFT but different auctionIds. As a result, a user could back out of an active auction by creating and then cancelling a duplicate auction. This leads to locked user...

NFT Sellers Can Steal Funds if an Offer is Accepted on an Unfinalized Auction

Lines of code Vulnerability details Impact NFTs can be sold to users in a variety of ways. One main method is for a seller to auction their NFT by creating a reserve auction. Once the auction is active, it is guaranteed that the sale will go to the highest bidder. Once an auction has ended, anyon...

LockedBalance library should drop parameters to 96/32 bits

Lines of code Vulnerability details Impact The LockedBalance contract takes 256-bit amount values but performs bit math on them as if they were 96 bit values. Bits could spill over to a different locked balance in the else part lockedBalance stores two 128-bit locked balances in one 256-bit stora...

Missing receiver validation in withdrawFrom

Lines of code Vulnerability details Impact The FETH.withdrawFrom function does not validate its to parameter. Funds can be lost if to is the zero address. Similar issues have been judged as medium recently, see Sandclock M-15 / Github issue Recommended Mitigation Steps Check that to != 0. --- The...

adminAccountMigration() Does Not Update buyPrice.seller

Lines of code Vulnerability details Impact The adminAccountMigration function is called by the operator role to update all sellers' auctions. The auction.seller account is updated to the new address, however, the protocol fails to update buyPrice.seller. As a result, the protocol is put in a...

Missing receiver validation in withdrawFunds

Lines of code Vulnerability details Impact The CollateralManagement.withdrawFunds function does not validate its to parameter. Funds can be lost if to is the zero address. Similar issues have been judged as medium recently, see Sandclock M-15 / Github issue Recommended Mitigation Steps Check that...

MAX_ROYALTY_RECIPIENTS_INDEX set too low

Lines of code Vulnerability details Impact The creator payouts are capped at MAXROYALTYRECIPIENTSINDEX. It's currently set to 4 and only 5 creators are paid out. Other creators are ignored. Recommended Mitigation Steps I don't think cases with more than 5 creators / royalty receivers are unlikely...

Rare scenario where seller can steal NFT after an auction by creating an offer for it through reentrancy

Lines of code Vulnerability details Impact It's a very specific scenario. The seller auctions off an NFT. If the buyer relists it directly while keeping the token in the escrow, the seller has the opportunity to steal it. They do it by creating an offer for it and accepting it. Proof of Concept...

There is no Support For The Trading of Cryptopunks

Lines of code Vulnerability details Impact Cryptopunks are at the core of the NFT ecosystem. As one of the first NFTs, it embodies the culture of NFT marketplaces. By not supporting the trading of cryptopunks, Foundation is at a severe disadvantage when compared to other marketplaces. Cryptopunks...

An offer made after auction end can be stolen by an auction winner

Lines of code Vulnerability details Impact An Offer which is made for an NFT when auction has ended, but its winner hasn't received the NFT yet, can be stolen by this winner as transferFromEscrow being called by acceptOffer will transfer the NFT to the winner, finalising the auction, while no...

buyFromPrivateSaleFor() Will Fail if The Buyer Has Insufficient Balance Due to an Open Offer on The Same NFT

Lines of code Vulnerability details Impact The buyFromPrivateSaleFor function allows sellers to make private sales to users. If insufficient ETH is provided to the function call, the protocol will attempt to withdraw the amount difference from the user's unlocked balance. However, if the same use...

[WP-M3] PRIMARY_FOUNDATION_FEE can be bypassed

Lines of code Vulnerability details The protocol is charging a different fee rate when the NFT is sold by the creator for the first time on the platform, the fee rate is currently set to a constant value of 15%. For other sales, the fee rate is only 5%. if isCreator &&...

Lack of exception handling

Lines of code Vulnerability details Impact After looking through the following functions: NFTMarketOffer.sol::264 - IERC721nftContract.transferFrommsg.sender, offer.buyer, tokenId NFTMarketPrivateSale.sol::177 - nftContract.transferFromseller, msg.sender, tokenId; We were able to reach the...

Primary seller can avoid paying the primary fee

Lines of code Vulnerability details Impact A primary seller can circumvent the 15% fee and pay 5% as a secondary seller. Context The foundation protocol charges a 15% fee if the sale is a primary sale and 5% if it's a secondary sale. There are 2 conditions that must be met for a sale to be...

Upgradable escrow contract

Lines of code Vulnerability details Impact Upgradable escrow contract pose great risk to user who approved their NFT to the contract. Most popular token / NFT exchange do not require user approve their asset to admin upgradable contract. This also increase user gas usage because they would have t...

Fees Are Incorrectly Charged on Unfinalized NFT Sales

Lines of code Vulnerability details Impact Once an auction has ended, the highest bidder now has sole rights to the underlying NFT. By finalizing the auction, fees are charged on the sale and the NFT is transferred to auction.bidder. However, if auction.bidder accepts an offer before finalization...

transferredAmount on mainnet can be drained if a malicious account can mint more tokens on Schain

Lines of code Vulnerability details Impact Anyone on Schain that is able to mint more tokens, other than the mint action from postMessage in tokenManagerERC20 by bridging tokens over, can potentially drain the locked tokens in transferredAmount in depositBoxERC20 on mainnet by calling exit with t...

dangerous receive function

Lines of code Vulnerability details the receive function doesn't check the msg.sender is supposed to pay, risking someone to accidentally sending ether and losing it. --- The text was updated successfully, but these errors were encountered: All reactions...

_getCreatorPaymentInfo() is Not Equipped to Handle Reverts on an Unbounded _recipients Array

Lines of code Vulnerability details Impact The getCreatorPaymentInfo function is utilised by distributeFunds whenever an NFT sale is made. The function uses try and catch statements to handle bad API endpoints. As such, a revert in this function would lead to NFTs that are locked in the contract...

[WP-H7] In some cases, when acceptOffer(), the buyer's ETH payment will be sent to the seller but the NFT will not be transferred to the buyer (paid and get nothing in return)

Lines of code Vulnerability details PoC Case A: Given: A ReserveAuction is waiting to be finalized: the creator createReserveAuction the bidder bid with 1 ETH wait until auction.endTime When: buyer makeOffer with: 1 ETH bidder acceptOffer Expected Results: 1. creator to receive bidder's payment o...

EIP-712 signatures can be re-used in private sales

Lines of code Vulnerability details Impact Within a NFTMarketPrivateSale contract, buyers are allowed to purchase a seller's NFT. This is done through a seller providing a buyer a EIP-712 signature. The buyer can then call buyFromPrivateSaleFor providing the v, r, and s values of the signature as...

Centralisation Risk: Admin Role of TokenManagerEth can Rug Pull All Eth from the Bridge

Lines of code Vulnerability details Impact There is a Centralisation risk of the bridge where the DEFAULTADMINROLE of TokenManagerEth.sol is able to modify the ERC20 token on the SChain to any arbitrary address. This would allow the admin role to change the address to one where they have infinite...

Private sale spoofing

Lines of code Vulnerability details Impact Similar to spoofing in finance, users can create private sales with correct signatures but then frontrun the buy with a transfer to a different wallet they control. No funds are lost as this the NFT FETH exchange is atomic but it can be bad if third...

NFTMarketReserveAuction: After calling adminAccountMigration to update auction.seller, other users cannot buy the NFT even before bidding starts.

Lines of code Vulnerability details Impact The adminAccountMigration function will update auction.seller to newAddress. Consider the following situation: 1. User A calls the setBuyPrice function and the createReserveAuction function to set the buy price and start the auction for his NFT. 2. The...

Exchange does not split royalty revenue correctly

Lines of code Vulnerability details According to the README.md If royalty information was not defined when the NFT was originally deployed, it may be added using the Royalty Registry which will be respected by our market contract. The actual exchange code only respects the Royalty Registry or oth...

Incorrect implementation when accepting offer from unsettled auctions

Lines of code Vulnerability details Impact FNDNFTMarket allows market offer to be made on auctioned NFTs that have not been finalised by the auction winner. Auction winner can call acceptOffer to accept the offer and finalise the auction in a single call. However, the current implementation has a...

Gas Pricing Can Be Used To Extort Funds From Users of SChain Owner

Lines of code Vulnerability details Impact The function refundGasByUser can be exploited by the message sender to drain nodes and SChain owners of their balances when processing incoming messages. When a node collates a set of exits from an SChain to Ethereum, they are submitted on-chain via...

Creators can steal sale revenue from owners' sales

Lines of code Vulnerability details According to the README.md All sales in the Foundation market will pay the creator 10% royalties on secondary sales. This is not specific to NFTs minted on Foundation, it should work for any NFT. If royalty information was not defined when the NFT was originall...

_autoAcceptOffer doesn't check if caller of setBuyPrice owns the NFT

Lines of code Vulnerability details Impact An attacker can create an offer then auto accept that offer for an NFT they don't own. This can happen for any NFT that the contract is approved for, or any NFT left in escrow with no active limitations Proof of concept Alice has an NFT that they either...

NFT tranfered to market directly can be took by anyone

Lines of code Vulnerability details Proof of Concept 1. Offer can be made on any NFT if: 1. Not in active auction 2. No existing offer 3. The new offer amount is greater than existing offer 1. No access control on acceptOffer , anyone can send tx and reach this line which lead to transfer from...

NFT owner can create multiple auctions

Lines of code Vulnerability details Impact NFT owner can permanently lock funds of bidders. Proof of concept Alice the attacker calls createReserveAuction, and creates one like normal. let this be auction id 1. Alice calls createReserveAuction again, before any user has placed a bid this is easy ...

Auction DOS On The Bidding

Lines of code Vulnerability details Impact During the code review, It has been observed that If the bidder is contract, the bidder can make auction infunctional. Proof of Concept 1. Navigate to the following contract. 2. Auction has public function named as placeBid. 3. Example Poc steps can be...

Mapping nftContractToTokenIdToBuyPrice is not deleted correctly

Lines of code Vulnerability details Impact Users that have bought an NFT previously through the NFTMarketBuyPrice functionality will not be able to make use of this functionality again as the nftContractToTokenIdToBuyPrice mapping is not correctly deleted anywhere in the NFTMarketBuyPrice contrac...

Issues with removing a connected chain

Lines of code Vulnerability details Impact A connected chain can be removed which will make all pending messages fail. If the chain is reinitialized again at some point, its incomingMessageCounter will start at zero and allow replay attacks of all previous messages. function...

No ERC20 safe* versions called on mainnet deposit

Lines of code Vulnerability details Impact Some mainnet tokens like USDT don't correctly implement the EIP20 standard and their transfer/transferFrom function return void instead of a success boolean. Calling these functions with the correct EIP20 function signatures will always revert. Non-safe...

NFT owner can change tokenURI

Lines of code Vulnerability details Impact In the ERC721OnChain implementation the token owner can set the token's URI using setTokenURI. Usually, this is token URI points to data defining the NFT attributes, images, etc.. It's usually set by the contract owner. A user that owns an NFT can just...

Fee-on-transfer/rebalancing tokens are not supported

Lines of code Vulnerability details Impact Some ERC20 tokens make modifications to their ERC20's transfer or balanceOf functions. One type of these tokens is deflationary tokens that charge a certain fee for every transfer or transferFrom. Others are rebasing tokens that increase in value over ti...

Can drain ETHERBASE

Lines of code Vulnerability details Impact The postIncomingMessages function tops up the contract with the gas token in topUpBalance. Note that the amount of gas tokens to be topped up from the ETHERBASE contract depends on msg.sender.balance, the caller's gas token balance. Attackers with a zero...