Taker can provide several instances of the cheapest ERC-1155 item in a multi item bundle

Lines of code Vulnerability details Maker bid for a bundle of ERC-1155 items can be tricked into successful execution by providing several instances of the cheapest item instead of the required bundle. This way a malicious taker can receive full maker's price, providing several instances of the...

Rescue ETH at InfinityExchange.sol is incorrectly implemented

Lines of code Vulnerability details Impact Admins are unable to recover any ETH locked in the contract PoC The function rescueETHInfinityExchange.sol is incorrectly implemented. It is using msg.value instead of the balance of the contract function rescueETHaddress destination external payable...

Wrong gasCost calculation per order

Lines of code Vulnerability details Impact During a match, there's some accounting on how much gas we're spending, so that the executor can be reimbursed. The gas cost is split between multiple orders, computing the difference between the gas at the start and at the end. The gas at the start is...

function removeRouter() in RouterFacet don't check that router has balance and don't transfer it, it just set router owner and recipient to 0x0 which can cause make router balance in danger or unavailable for router owner

Lines of code Vulnerability details Impact There are some security levels for router, like setting owner and recipient and when removeRouter is called this values set to 0x0 and router address become vulnerable. contract should transfer router balance to recipient before removing it. Proof of...

matchOneToManyOrders doesn't consider numItems of the orders

Lines of code Vulnerability details Impact An order can specify a numItems in MakerOrder.constraints0. This number is the min/max number of items the order wants to buy/sell. For example a buy order can provide a list of nfts and say that wants to buy only 3 of them from that list. The function...

Malicious relayer could exploit sponsor vaults

Lines of code Vulnerability details Impact Sponsor vaults drained Proof of Concept reimburseRelayerFees uses SponsorVault funds to repay users the fees they pay to relayers. A malicious relayer could create a large number of transactions with the max reimbursed relay fee specified in SponsorVault...

Issues beyond expected behavior.

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. function unstakeuint256 amount external override nonReentrant whenNotPaused requireamount != 0, 'stake amount cant be 0'; uint256 noVesting = userstakedAmountsmsg.senderDuration.NONE.amount; uint256...

Admin can set arbitrarily high exchange fees

Lines of code Vulnerability details Impact Admins can attack users by frontrunning them Poc In the function setProtocolFeeInfinityExchange.sol function setProtocolFeeuint16 protocolFeeBps external onlyOwner PROTOCOLFEEBPS = protocolFeeBps;@audit medium emit NewProtocolFeeprotocolFeeBps; Admins ca...

Centralization Risk with onlyOwner modifier

Lines of code Vulnerability details Impact During the code review, It has been observed the all currency tokens can be withdraw by owner without timelock. The currency token should not be withdrawn by owner. This poses centralization risk. Proof of Concept 1. Navigate to the following contract...

User may lose funds if msg.value > totalPrice

Lines of code InfinityExchange.solL300-L328 InfinityExchange.solL336-L364 Vulnerability details Impact When using InfinityExchange.sol:takeMultipleOneOrders or InfinityExchange.sol:takeOrders, if currency == address0 and a user has a msg.value totalPrice the overspent ETH is left in the contract...

Extra msg.value in takeOrders

Lines of code Vulnerability details Impact In takeOrders, msg.sender can pay the items in ether or other native coins. // check to ensure that for ETH orders, enough ETH is sent // for non ETH orders, IERC20 safeTransferFrom will throw error if insufficient amount is sent if isMakerSeller &&...

ETH rescue does not work

Lines of code Vulnerability details Impact Both contracts InfinityExchange and InfinityStaker have a function rescueETH to allow an admin to rescue any ETH accidentally sent to the contracts. However, this ETH rescue functionality does not work. The code expects ETH to be sent to this function an...

Protocol can steal WETH founds

Lines of code Vulnerability details Impact The protocol can steal WETH founds with the refunds gas cost mechanism in the functions matchOneToOneOrders, matchOneToManyOrders and matchOrders This functions can call only by the MATCHEXECUTOR but we don't know what is this contract/address according...

_transferNFTs doesn't revert for bad tokens

Lines of code Vulnerability details Impact transferNFTs checks if an item is ERC721 or ERC1155 by using IERC165item.collection.supportsInterface.... function transferNFTs address from, address to, OrderTypes.OrderItem calldata item internal if IERC165item.collection.supportsInterface0x80ac58cd...

Bridging could be temporarily disrupted when setConnext is called without deactivating sponsorVault

Lines of code Vulnerability details When a sponsor wants to stop sponsoring bridge fees, it's possible that they call SponsorVault.setConnext to stop allowing Connext to reimburse fees, but forgot to inform the Connext team. This will cause bridging executions to be temporarily disrupted. For fee...

_transferNFTs() functions doesn't reverts if the transfer item doesn't supports interface for both ERC721 and ERC1155

Lines of code Vulnerability details Impact The transferNFTs function use ERC165 to check if the itemnft supports ERC721 interface or ERC1155 interface and execute transfer accordingly. But if it doesn't supports either, it just exits the functionno revert. Proof of Concept in...

Malicious Relayer Can Replay Execute Calldata On Different Chains Causing Double-Spend Issue

Lines of code Vulnerability details Proof-of-Concept This issue is only applicable for fast-transfer. Slow transfer would not have this issue because of the built-in fraud-proof mechanism in Nomad. First, the attacker will attempt to use Connext to send 1000 USDC from Ethereum domain to Optimism...

Wrong implementation of rescueETH, causing the fees in native tokens to be frozen in the contract

Lines of code Vulnerability details /// @dev used for rescuing exchange fees paid to the contract in ETH function rescueETHaddress destination external payable onlyOwner bool sent, = destination.callvalue: msg.value''; requiresent, 'failed'; Recommendation Change to: /// @dev used for rescuing...

LibDiamond.diamondCut() should check diamondStorage().acceptanceTimes[keccak256(abi.encode(_diamondCut))] != 0

Lines of code Vulnerability details Impact Normally, diamondStorage.acceptanceTimeskeccak256abi.encodediamondCut will be set in LibDiamond.proposeDiamondCut. Then in LibDiamond.diamondCut, it checks that diamondStorage.acceptanceTimeskeccak256abi.encodediamondCut function proposeDiamondCut...

_handleExecuteTransaction may not working correctly on fee-on-transfer tokens. Moreover, if it is failed, fund may be locked forever.

Lines of code Vulnerability details Impact handleExecuteTransaction may not working correctly on fee-on-transfer tokens. As duplicated fee is applied to fee on transfer token when executing a arbitrary call message passing request. Moreover, the Executor contract increase allowance on that token...

Rescue ETH at InfinityStaker.sol is incorrectly implemented

Lines of code Vulnerability details Impact Admins are unable to recover any ETH locked in the contract PoC The function rescueETHInfinityStaker.sol is incorrectly implemented. It is using msg.value instead of the balance of the contract /// @dev Admin function to rescue any ETH accidentally sent ...

InfinityExchange owner can steal user's tokens via front-running

Lines of code Vulnerability details Impact Contract InfinityExchange.sol charges protocol fee through PROTOCOLFEEBPS. The issue is that owner of the contract is able to change protocol fee at any time without any restriction which puts him in a very privileged position and allows him to steal...

Multiple items order can be matched to many same items order

Lines of code Vulnerability details Impact Let's consider an example. Alice makes an order for an ERC1155, where she wants to buy 10 items with id=1 and 10 with id=2. This order can be matched using matchOneToManyOrders with two orders that sell both 10 items with id=1. Basically Alice gets 20 id...

Buyer may not agree on gasCost

Lines of code Vulnerability details Impact Gas cost when matching orders is payed by the buyer. Since buyers don't have control on order execution, they may spend more gas than what they are willing to. Examples: periods of high gasPrice, or if NFTs for some reason consume a extra amount of gas...

rescueETH function doesn't works as expected

Lines of code Vulnerability details Impact The rescueETH function is implemented to collect any unexpected ETH transferred to the infinityExchange.sol contract, But this function will not work as expected. The function is supposed to return the eth from the contract to the specified destination...

ETH mistakenly sent over with ERC20 based takeOrders and takeMultipleOneOrders calls will be lost

Lines of code Vulnerability details takeOrders and takeMultipleOneOrders are the main user facing functionality of the protocol. Both require currency to be fixed for the call and can have it either as a ERC20 token or ETH. This way, the probability of a user sending over a ETH with the call whos...

The amount of gas calculation for refunding is incorrect

Lines of code Vulnerability details Impact While matching the orders the gas cost is calculated so that it can be refunded back to the contract. The calculation for this gas is incorrect. The function keeps track of the gasleft at the beginning of the loop and adds additional amount of gas for pr...

Accidental ETH transfers to InfinityStaker cannot be withdrawn

Lines of code Vulnerability details The rescueETH function is meant to allow the contract owner to withdraw ETH accidentally sent to the contract. However, it will forward the exact ETH amount sent by the caller to the given destination address, rather than withdraw ETH from the contract balance:...

Maker buy order with no specified NFT tokenIds may get fulfilled in matchOneToManyOrders without receiving any NFT

Lines of code Vulnerability details The call stack: matchOneToManyOrders - matchOneMakerSellToManyMakerBuys - execMatchOneMakerSellToManyMakerBuys - execMatchOneToManyOrders - transferMultipleNFTs Based on the context, a maker buy order can set OrderItem.tokens as an empty array to indicate that...

Maker order buyer is forced to reimburse the gas cost at any tx.gasprice

Lines of code Vulnerability details uint256 gasCost = startGasPerOrder - gasleft + wethTransferGasUnits tx.gasprice; // if the execution currency is weth, we can send the protocol fee and gas cost in one transfer to save gas // else we need to send the protocol fee separately in the execution...

Router Owner Could Be Rugged By Admin

Lines of code Vulnerability details Proof-of-Concept Assume that Alice's router has large amount of liquidity inside. Assume that the Connext Admin decided to remove a router owned by Alice. The Connext Admin will call the RoutersFacet.removeRouter function, and all information related to Alice's...

Diamond can be updated without proposing the change

Lines of code Vulnerability details Impact The diamond shall be monitored externally to remove the need of trust to developers. If a timelock can be bypassed, it poses a threat as people who weren't trusted can exploit the system. Additionally, the contract can immediately perform any delegatecal...

Timelock able to be bypassed because of wrong check in LibDiamond

Lines of code Vulnerability details Impact In the walkthrough video, it said that the upgrades of Diamond must go through a proposal window with a delay of 7 days. Upgrade should be done by first call proposeDiamondCut and then wait 7 days and call diamondCut. But this timelock can be bypassed...

BridgeFacet's _executePortalTransfer ignores underlying token amount withdrawn from Aave pool

Lines of code Vulnerability details executePortalTransfer can introduce underlying token deficit by accounting for full underlying amount received from Aave unconditionally on what was actually withdrawn from Aave pool. Actual amount withdrawn is returned by IAavePools.aavePool.withdraw, but...

Swaps done internally will be not be possible

Lines of code Vulnerability details Affected functionsthat rely on swapAsset are: swapAsset facilitates two swaps, either using the internal or external pool. But if an internal pool exists, a swap will be unsuccessful because the call to s.swapStoragescanonicalId.swapInternal takes two incorrect...

nonReentrant modifier is not added to all functions that generate state changes, there is a possibility of re-entry.

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. I noticed that the nonReentrant modifier only adds some of the functions that generate state changes to the InfinityExchange.sol contract, which provides the feasibility of reentrancy between multiple...

User can accidentally permanently freeze the staked funds

Lines of code Vulnerability details User facing changeDuration function allows for setting any newDuration of a stake. However, only THREEMONTHS, SIXMONTHS and TWELVEMONTHS durations are visible to the system in all the subsequent logic. If a user accidentally sets any other duration, the...

Precision Loss During Division

Lines of code Vulnerability details Proof-of-Concept Assume that toSwap = 10 DAI and pathLen = 3 in this example. Therefore, the bridge will pull 10 DAI from the RouterFacet contract. However, within the BridgeFacet.handleExecuteLiquidity function, due to precision loss when solidity handles...

Current implementation of arbitrary call execute failure handler may break some use case for example NFT bridge.

Lines of code Vulnerability details Impact Current implementation of arbitrary call execute failure handler may break some use case for example NFT Bridge. In the case of NFT Bridge, NFT may be lost forever. This is likely to be happened in the case of out of gas. Proof of Concept Relayer receive...

Funds(ETH) permanent lock on InfinityStaker.sol

Lines of code Vulnerability details Impact The rescueETH function does not work as expected and if the contract receives ETH, it gets stuck in the contract Proof of Concept If an addresswallet or contract send ETH to the InfinityStaker.sol contract, the owneradmin can't rescue the ETH because the...

The owner can set arbitrarly high fees

Lines of code Vulnerability details Impact The owner of InfinityExchange contract can set arbitrarly big fees, and coul stole funds from seller. Proof of Concept When an order is executed the payment and the transfer from buyer to seller and viceversa are done by transferNFTsAndFees function wich...

getCurrentPrice will be return a higher value than expected due to insufficient check in isTimeValid

Lines of code Vulnerability details The Boolean value will always return true if start price is zero. The condition will be sufficiently fulfilled since a time will be set in the future but no time is set in the past or at current time: sumCurrentPrices uses the returned value of getCurrentPrice...

Incorrect check in diamondCut allows update be executed without proposal

Lines of code Vulnerability details The current implementation of LibDiamond.diamondCut allows any diamondCut update to be executed instantly, defeating the purpose of the 7 day update delay mechanism. The issue is this check in LibDiamond.diamondCut: require...

Malicious Relayers Could Favor Their Routers

Lines of code Vulnerability details Vulnerability Details Assume that a malicious relayer operates a router in Connext providing fast-liquidity service. A malicious relayer could always swap the routers within the execute calldata with the routers owned by malicious relayer, and submit it to the...

NO TIMELOCK ON setProtocolFee() CAN LEAD TO SELLERS LOSING THEIR NFTs

Lines of code Vulnerability details NO TIMELOCK ON setProtocolFee CAN LEAD TO SELLERS LOSING THEIR NFTs In InfinityExchange.sol, there is no timelock on setProtocolFee. This is the fee that is applied in orders, and determines how much the Exchange receives in fee VS how much the seller receives...

in reimburseLiquidityFees() of SponserVault contract swaps tokens without slippage limit so its possible to perform sandwich attack and it create MEV

Lines of code Vulnerability details Impact when code swaps tokens it should specify slippage but in reimburseLiquidityFees code contract calls tokenExchange.swapExactIn without slippage and it's possible to perform sandwich attack and make contract to swap on bad exchange rates and there is MEV...

Wrong implementation of withdrawAdminFees() can cause the adminFees to be charged multiple times and therefore cause users' fund loss

Lines of code Vulnerability details function withdrawAdminFeesSwap storage self, address to internal IERC20 memory pooledTokens = self.pooledTokens; for uint256 i = 0; i pooledTokens.length; i++ IERC20 token = pooledTokensi; uint256 balance = self.adminFeesi; if balance != 0 token.safeTransferto,...

Tokens with decimals larger than 18 are not supported

Lines of code Vulnerability details For tokens with decimals larger than 18, many functions across the codebase will revert due to underflow. function getPriceFromDexaddress tokenAddress public view returns uint256 PriceInfo storage priceInfo = priceRecordstokenAddress; if priceInfo.active uint25...

routerBalances[msg.sender][_local] can be inflated in repayAavePortal due to underflow in unchecked math

Lines of code Vulnerability details function repayAavePortal address local, uint256 backingAmount, uint256 feeAmount, uint256 maxIn, bytes32 transferId external uint256 totalAmount = backingAmount + feeAmount; // in adopted uint256 routerBalance = s.routerBalancesmsg.senderlocal; // in local //...

Upgraded Q -> M from 225 [1655654402923]

Judge has assessed an item in Issue 225 as Medium risk. The relevant finding follows: C4-011 : Centralization Risk On The teamSummon Function - LOW Impact - LOW With the teamSummon function, owner can mint unlimited warriors. This poses a security risk. The max/min limit should be implemented at...