User funds(ETHs) sent along with bulkExecute tx may be stolen by a reentry attack

Lines of code Vulnerability details Impact The funds ETH that users sent along with the bulkExecute may be stolen. Proof of Concept When a buyer send a bulkExecute tx with msg.value 0 with order of buying token with eth, the sent ETH may be stolen if the tx contains a malicious selling order whic...

Tx should revert when the call in _returnDust failed

Lines of code Vulnerability details Impact User may lose remaining ETH of the transaction Proof of Concept As the returned status of the call in returnDust is ignored, the tx will success even if the call failed, which means the caller does not get his ETH back. Tools Used n/a Recommended...

_returnDust may sweep more eth than it is entitled to

Lines of code Vulnerability details Impact returnDust instead of returning the rightful remainingETH amount, it returns selfbalance. If eth gets into the contract somehow, it can be swept by anyone, who passes in some eth through bulkExecute, even with empty executions parameter. Proof of Concept...

Upgraded Q -> M from #194 [1668467876517]

Judge has assessed an item in Issue 194 as M risk. The relevant finding follows: L-04 Should be a upper-bound on Flash-loan fee There are 1 instances of this issue: File : = Recommended Mitigation There should be a upper bound of Flash-loan fee define in contract so that owner can't able to set f...

Upgraded Q -> M from #323 [1668467355303]

Judge has assessed an item in Issue 323 as M risk. The relevant finding follows: L00: beforeTokenTransfer function called with wrong params in LBToken Line 237 seems to be a copy pasta mistake from line 209 in LBToken.sol. On line 237 when burning tokens, to should be zero, and amount of from's...

Upgraded Q -> M from #70 [1668468349340]

Judge has assessed an item in Issue 70 as M risk. The relevant finding follows: Flash loan fee can be set to 100% Contract: Issue: In setFlashLoanFee function, If Admin has set flashloan fee to 100% then user taking X amount as flashloan has to pay a fee equal to X which does not make sense...

[H-01] owner not set in Pool.sol

Lines of code Vulnerability details The pool.sol contract here is an UUPSUpgradeable contract. But there is no initialize function where Ownableinit is called , due to which owner is 0x0. It would be impossible to call authorizeUpgrade or change ownership of the contract. POC Adding the following...

Upgraded Q -> M from #234 [1668465995897]

Judge has assessed an item in Issue 234 as M risk. The relevant finding follows: 1.LBToken: In the burn function, beforeTokenTransfer uses incorrect from and to. Burning tokens should be transferred to 0 address. Proof of Concept Recommended Mitigation Steps beforeTokenTransfer account, address0,...

Cross-functional re-entrancy resulting in stealing any additional/extra ether sent by the execute() 's caller

Lines of code Vulnerability details Impact The contract Exchange.sol has execute function which can be called by anyone to execute a single buy and sell order. The function calls execute then returnDust. The latter sends the unrequired ether back to the caller. However, a malicious actor could...

deposit and withdraw emit the same event than mint and burn in ERC20 and can affect to Dapps

Lines of code Vulnerability details Impact Deposit and Withdraw emit the same event than burn and mint than ERC20 so it's not possible to distinguish in a DAPP than deposit and withdraw are not mint or burn and could affect to the DAPP logics. For example, cointracking.info could understand in a...

Upgraded Q -> M from #493 [1668465137655]

Judge has assessed an item in Issue 493 as M risk. The relevant finding follows: 108 --- The text was updated successfully, but these errors were encountered: All reactions...

Pool designed to be upgradeable but does not set owner, making it unupgradeable

Lines of code Vulnerability details Description The docs state: "The pool allows user to predeposit ETH so that it can be used when a seller takes their bid. It uses an ERC1967 proxy pattern and only the exchange contract is permitted to make transfers." Pool is designed as an ERC1967 upgradeable...

Broken Upgradable Logic in Pool.sol

Lines of code Vulnerability details Impact The Pool smart contract allows a user to predeposit ETH so that it can be used when a seller takes their bid. It uses an ERC1967 proxy pattern and only the exchange contract is permitted to make transfers. The smart contract inherits the...

User's assets can be drained without payment due to invalid signature check

Lines of code Vulnerability details Impact Exchange::execute uses validateSignatures function to verify if an order is signed by both parties. However, this function does not verify the signature when order.trader == msg.sender. Hence, malicious actor can prepare a bundle of all seller's Orders...

Theft of ETH that was not used for the successful execution of orders

Lines of code Vulnerability details Description There are execute and bulkExecute functions in Exchange smart contract. There is the refund of any ETH that was unused for example that was left due to the unsuccessful order execution at the end of its execution flow: returnDust; returnDust functio...

Exchange refund operation will return all ETH stored in the contract instead of the remaining amount from the exchange operation

Lines of code Vulnerability details Exchange refund operation will return all ETH stored in the contract instead of the remaining amount from the exchange operation The function that refunds remaining ETH in the Exchange contract will send back all the balance present in the contract instead of...

_returnDust doesn't check the return value after call

Lines of code Vulnerability details Impact returnDust doesn't check that the call has been sucesfully. For example, if the buyer is a contract could fail during the reception but due to returnDust doesn't check the return value to revert the transaction in case of failure the tokens could be...

Reentrancy attack can be used to externally call the _execute function in the Exchange contract

Lines of code Vulnerability details Reentrancy attack can be used to externally call the execute function in the Exchange contract The execute function present in the Exchange contract is intended to be called only internally using delegatecall by the execute and bulkExecute functions. This is...

Reentrancy attack on fee transferring

Lines of code Vulnerability details Vulnerability details Description There is execute function in the Exchange smart contract. The function matches two orders, ensuring the validity of the match, transfers the order fees, etc. When transferring fees, the contract just makes a call to the...

Upgraded Q -> M from #449 [1668465467675]

Judge has assessed an item in Issue 449 as M risk. The relevant finding follows: FlashLoanFee should be bounded | Low | 1 --- The text was updated successfully, but these errors were encountered: All reactions...

#bulkExecute() can reenter to steal funds

Lines of code Vulnerability details Impact ExchangebulkExecute can reenter, and internal execution of delegatecall is allowed to fail, not revert, malicious users can reenter to steal funds Proof of Concept Assumptions: There is a malicious user alice,with NFT for sale, paid through eth If user b...

Upgraded Q -> M from #334 [1668467418003]

Judge has assessed an item in Issue 334 as M risk. The relevant finding follows: 2. Rug vectors by the owner A malicious owner can call setLBPairImplementation, setFeeRecipient, setFlashLoanFee , setFeesParameters and forceDecay to advantage himself at expenses of the users...

Upgraded Q -> M from #276 [1668468011155]

Judge has assessed an item in Issue 276 as M risk. The relevant finding follows: Misaligned parameters when calling beforeTokenTransfer in LBToken:burn LBToken.sol L237 address0 and account should be switched on calling beforeTokenTransfer in LBToken:burn. There's no side effects currently becaus...

Upgraded Q -> M from #474 [1668467584757]

Judge has assessed an item in Issue 474 as M risk. The relevant finding follows: L-01 Flashloan fee is not validated flashLoanFee is determined at LBFactory's constructor as; constructoraddress feeRecipient, uint256 flashLoanFee setFeeRecipientfeeRecipient; flashLoanFee = flashLoanFee; emit...

Contract LooksRareAggregator does not verify the proxy has code when delegatecall() is called on the proxy

Lines of code Vulnerability details Impact Delegatecall will return ‘True’ for the status value if it is called on an address that is not a contract and so has no code. This can cause bugs if code expects delegatecall functions to return False when they have to perform special logic. If, for some...

Address(0) owner is dangerous

Lines of code Vulnerability details Impact If the current owner confirms the renouncement, the new owner will have address zero. In this case no new owner can be assigned and the functions with onlyOwner modifier will be un-callable forever. Proof of Concept This mechanism is dangerous, because i...

Missing check of return value of ETH transfer

Lines of code Vulnerability details Impact When users uses ETH to batch buy NFTs, if the transaction fails, the aggregator contract will call the function returnETHIfAny to return the ETH to the user. This function transfers all the balance in the contract to the recipient through call, but there...

It is possible for a user's ETH to be trapped inside the contracts.

Lines of code Vulnerability details Impact It is possible that a user's ETH will be trapped inside the contracts. returnETHIfAnyoriginator; But in the implementation of the returnETHIfAny function, it will just silently return even the returning ETH transaction fails. As a result, even if the use...

Native funds on the aggregator contract balance is a free grab

Lines of code Vulnerability details Native funds on the aggregator contract balance is a free grabLooksRareAggregator's execute returns the native balance of the contract to the caller even when nothing was provided with the call. This happens when LooksRareAggregator's execute is called directly...

Initialization function can be front-run

Lines of code Vulnerability details Detailed description of the impact of this finding: Exchange.sol has initialization function that can be front-run, allowing an attacker to incorrectly initialize the contract. Due to the use of the delegatecall proxy pattern, Exchange.sol cannot be initialized...

Seller can steal ETH which is for other orders of the bulk

Lines of code Vulnerability details Impact The 'bulkExecute' and 'returnDust' functions are susceptible to reentrancy attack. Seller can exploit it to steal ETH which is for other orders of the bulk. Proof of Concept Key steps for successful attack 1 set fee rate to 100% 2 reentrancy call...

_returnETHIfAny() can silently fail, leading to an originator not getting the expected refund

Lines of code Vulnerability details The execute function refunds the unused ETH back to the originator through returnETHIfAny. This internal function uses a low-level call to transfer the ETH. The issue is that the return value of the call is not checked. As per the Solidity documentation...

Pulling user's ERC20 tokens to Contract LooksRareAggregator requires approvals from the user

Lines of code Vulnerability details Impact Contract ERC20EnabledLooksRareAggregator pulls user's ERC20 tokens to Contract LooksRareAggregator without pre-approvals from the user. This will prohibit the token transfer unless somewhere else the approvals have been done e.g. before calling the...

OwnableTwoStep delay not set

Lines of code Vulnerability details Impact Contract TokenRescuer inherits the contract OwnableTwoSteps but does not set any delay. There should be a constructor in TokenRescuer responsible for setting the delay as described in the comments of the OwnableTwoSteps contract. Without any delay,...

ETH could be locked in LooksRareAggregator

Lines of code Vulnerability details Impact ETH is returned to originator at the end of function LooksRareAggregatorexecute.... However, this logic does not check for returned value from originator in case originator is a contract and reverts when receiving ether. The result is that the transactio...

Left ERC20/ETH can be withdrawn by anyone

Lines of code Vulnerability details Impact Any remaining balance can be used by anyone. This can impact on users who transfers directly to the protocol by mistake. Proof of Concept If any user by mistake transfers ERC20/ETH directly not through the ERC20EnabledLooksRareAggregator or execute...

Anyone can get access to ERC20 tokens in LooksRareAggregator contract

Lines of code Vulnerability details Impact The TokenRescuer contract helps to rescue eth and erc20 tokens and can only be called by the owner. However, any trapped eth or erc20 tokens can actually be taken out by anyone. Essentially the function and access control of the TokenReceiver contract ca...

Public to all funds escape

Lines of code Vulnerability details Description The LooksRareAggregator smart contract implements a bunch of functions to escape funds by the contract owner see rescueETH, rescueERC20, rescueERC721, and rescueERC1155. In this way, any funds that were accidentally sent to the contract or were lock...

Transfer error can fail unnoticed

Lines of code Vulnerability details Impact Quoting Solidity docs: The low-level functions call, delegatecall and staticcall return true as their first return value if the account called is non-existent, as part of the design of the EVM. Account existence must be checked prior to calling if needed...

Contract LooksRareAggregator Ownership renouncement may leave a deficient smart contract in operation and there is no way to fix it

Lines of code Vulnerability details Impact LooksRareAggregator can renounce ownership and the system can still operate with already set parameters. However, many important functions cannot be called any more in this situation such as setFee, addFunction rescureERC721, rescueETH, etc. This means...

ETH amount that is trapped in LooksRareAggregator contract can be withdrawn by user who is not LooksRareAggregator's owner

Lines of code Vulnerability details Impact When ETH amount is trapped in the LooksRareAggregator contract, such as when someone accidentally sends some ETH to it, the owner of the LooksRareAggregator contract has the privilege to call the rescueETH function to transfer such amount to a proper...

Unsafe ERC20 operations due to lack of contract length check

Lines of code Vulnerability details Impact Functions executeERC20DirectTransfer and executeERC20TransferFrom replicates solmate libraries methods. The problem with this is that these functions does not check existence of code at the token address. If executeERC20DirectTransferand...

internalCall modifier can be circumvented with execute / bulkExecute

Lines of code Vulnerability details Impact The internalCall modifier ensures that public functions can only be called from another public function and not directly. It is implemented like that: modifier internalCall requireisInternal, "This function should not be called directly"; ; The modifier...

User can drain all ether from LooksRareAggregator contract

Lines of code Vulnerability details Impact Anyone could drain all ether from this contract. Proof of Concept function execute TokenTransfer calldata tokenTransfers, TradeData calldata tradeData, address originator, address recipient, bool isAtomic external payable nonReentrant if recipient ==...

Vampire attack on the LooksRareAggregator

Lines of code Vulnerability details Description The LooksRareAggregator project is almost stateless and most of its code is open. Someone may fork it and make a lower fee for the users or even add other incentives. Unlike Uniswap, the aggregator doesn't have a network effect, so it is economicall...

Seaport proxy is missing ERC20 approval, so it can't buy any order that is listed using an ERC20

Lines of code Vulnerability details Impact Seaport proxy is missing ERC20 approval, so it can't buy any order that is listed using an ERC20. Proof of Concept function execute BasicOrder calldata orders, bytes calldata ordersExtraData, bytes calldata extraData, address recipient, bool isAtomic,...

Pool funds can be used by anyone due to lack of allowance in the custom implementation of transferFrom

Lines of code Vulnerability details Impact Users' Pool assets can be used by anyone to execute a transaction, essentially stealing from them. Proof of Concept Pool contract uses a custom implementation of transferFrom which allows transfer of Pool assets from any address to any address as long as...

Without strictly verifying the attribution of balance and the size of the balance when refunding, hackers may use the attack to steal all ERC20 tokens！

Lines of code Vulnerability details Impact When using any ERC20 token to purchase NFT, after the purchase is successful, the 108th line of code in the contract LooksRareAggregator determines whether there are any remaining unused ERC20 tokens. If there is any remaining, it will be returned to the...

Tokens and ETH can be sweeped from LooksRareAggregator

Lines of code Vulnerability details Impact Tokens and ETH can be sweeped from LooksRareAggregator Proof of Concept A malicious user may transfer out any tokens or ETH that are owned by LooksRareAggregator. To do so, a such user has to call execute - and make the trade. After executing of order...

call opcode's return value not checked.

Lines of code Vulnerability details Impact The call opcode's return value not checked, which could leads to the originator lose funds. Proof of Concept The caller of LooksRareAggregator.sol::execute could be a contract who may not implement the fallback or receive function, when a call to it with...