If SChain is Removed Before kill() and getFunds() all Tokens are Locked in the Bridge

Lines of code Vulnerability details Impact If the SChain is removed before all the funds are withdrawn from the bridge they will be permanently locked in the bridge. When a SChain is removed in by the SKALE protocol via the function SchainsInternal.removeSchain the data including the owner will b...

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

Forcing ERC20Upgradeable when calling transfer() reverts when used with some ERC20 tokens

Lines of code Vulnerability details Some tokens do not implement the ERC20 standard properly but are still accepted by most code that accepts ERC20 tokens. For example Tether USDT's transfer and transferFrom functions do not return booleans as the specification requires, and instead have no retur...

[WP-H1] Transactions can be replayed when a connectedChain is removed and then reconnected

Lines of code Vulnerability details function removeConnectedChainstring memory schainName public virtual override onlyChainConnector bytes32 schainHash = keccak256abi.encodePackedschainName; requireconnectedChainsschainHash.inited, "Chain is not initialized"; delete connectedChainsschainHash;...

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

Anyone can set gas price

Lines of code Vulnerability details Impact The setGasPrice function does not have any access restrictions. Anyone can call it and set the gas price to typeuint256.max, leading to DoS in any computation that uses it. It does currently not seem to be used. Recommended Mitigation Steps Add proper...

Reentrancy in MessageProxyForSchain leads to replay attacks

Lines of code Vulnerability details Impact The postIncomingMessages function calls callReceiverContractfromChainHash, messagesi, startingCounter + 1 which gives control to a contract that is potentially attacker controlled before updating the incomingMessageCounter. for uint256 i = 0; i...

Missing access restriction on addConnectedChain

Lines of code Vulnerability details Impact The addConnectedChain function is supposed to be called only by the CHAINCONNECTORROLE. / Requirements: - Function caller has to be granted with CHAINCONNECTORROLE. - Target chain must be different from the current. / However, there is no such restrictio...

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