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's _returnDust() does not validate return value of call() to send excess ETH to sender.

Lines of code Vulnerability details Impact When a caller to Exchange's execute or bulkExecute includes more ETH than is required to complete the transactions, returnDust is intended to return this excess back to the caller. However, returnDust does not validate that the call it performs to return...

Upgraded Q -> M from #451 [1668467945427]

Judge has assessed an item in Issue 451 as M risk. The relevant finding follows: THRESHOLD LIMIT CHECKS Certain parameters of the contracts can be configured to invalid values, causing a variety of issues and breaking expected interactions between contracts. setFlashLoanFee allows the owner of...

Exchange.sol has payable functions but no way of withdrawing

Lines of code Vulnerability details Exchange.sol has payable functions but no way of withdrawing Impact Functions are payable: IExchange.execute Exchange.execute Exchange.bulkExecute Exchange.execute UUPSUpgradeable.upgradeToAndCall But there is no way of withdrawing, so funds can be lost PoC But...

Admin can drain user funds from the Pool or buy assets for free

Lines of code Vulnerability details Impact We assume that the admin is honest, however there is still possibility of exploiting asset policy contract to and set price to 0 in oder to buy an asset for free - or even worse - drain user funds by setting the price really high in...

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

Counterparty can cancel subsequent orders within ETH or token transfer callback.

Lines of code Vulnerability details Impact The scenario is when a caller uses bulkExecute to request a sequence of executions on the Exchange. The caller will generally expect that all orders valid prior to calling bulkExecute will be valid during the execution of bulkExecute. However, a...

Upgraded Q -> M from #407 [1668467735071]

Judge has assessed an item in Issue 407 as M risk. The relevant finding follows: Incorrect parameters for beforeTokenTransfer hook In LBToken.sol, the beforeTokenTransfer hook has the following parameters: 317: /// @param from The address of the owner of the token 318: /// @param to The address o...

Upgraded Q -> M from #336 [1668467652322]

Judge has assessed an item in Issue 336 as M risk. The relevant finding follows: L-1: Volatility accumulator can be be prevented from decaying by way of dust transactions There is no required minimum swap amount for updating the volatility accumulated. The fp.time is always updated during a swap...

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

Upgraded Q -> M from #446 [1668468223347]

Judge has assessed an item in Issue 446 as M risk. The relevant finding follows: L-01 There should be an upper limit for LBFactory.flashLoanFee If the admin sets the flashLoanFee too high, the flash loan functionality might be useless as users won't use it. --- The text was updated successfully,...

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

EVERY TIME ONCE _execute FUNCTION COMPLETED NEED TO SET isOpen TO 0. OTHERWISE WE CAN CALL EXECUTE FUNCTIONS MULTIMPLE TIMES.

Lines of code Vulnerability details Impact IN THIS WAY WE CAN CALL AND EXECUTE FUNCTIONS MULTIPLE TIMES . EVERY TIME BEFORE CALL EXECUTE NEED TO CALL INITIALIZE FUNCTIONS. Proof of Concept function executeInput calldata sell, Input calldata buy public payable reentrancyGuard internalCall...

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

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

Signature Malleability in case of using EVM built-in ecrecover()

Lines of code Vulnerability details Impact The ecrecover function which is used verify a signature. The built-in EVM precompile ecrecover is susceptible to signature malleability because of non-unique s and v values which could lead to replay attacks. Proof of Concept Tools Used Manual Analysis...

unconventional reentrant structure can result in reentrance into _returnDust

Lines of code Vulnerability details Impact unconventional nonreentrant code structure allows for reentrance from returnDust Proof of Concept Once execute finishes execution, the reentrancy guard is reset to be not in effect, and the flow goes into returnDust. Now caller's receive function can cal...

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

Tokens transfers do not verify that the tokens were successfully transferred

Lines of code Vulnerability details Impact Some tokens like zrx do not revert the transaction when the transfer/transferfrom fails and return false instead, which requires us to check the return value after calling the transfer/transferfrom function. Even though, the exchange seems to only suppor...

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

Unintended code path execution by modifying user controlled input

Lines of code Vulnerability details Impact Unintended code path execution by modifying user controlled input. The deal maker gets to pick if buyer or seller matching policy are executed. The actual impact of this vulnerability would depend on the matching policy. Any where from wasted gas to loss...

On _returnDust, 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 Exchange.sol::execute or Exchange.sol::bulkExecute could be a contract who may not implement the fallback or receive function, when...

Upgraded Q -> M from #215 [1668468272191]

Judge has assessed an item in Issue 215 as M risk. The relevant finding follows: 7. No max fee is set on setFlashLoanFee function There is no max fee set. This allows a malicious owner to set a very high amount of flashloan fee , which allows the malicious admin to grief other users. He can also...

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

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

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

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

LooksRareProxy is missing the fee mechanism, protocol may loss the revenue from fee

Lines of code Vulnerability details Impact LooksRareProxy is missing the fee mechanism, the protocol may lose revenue from the fee. Even if you intend to have a free fee, the fee mechanism should be implemented in case you need to enforce the fee in the future. Proof of Concept function execute...

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

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

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

The attacker can steal the ETH of users that wanted to buy an NFT by sandwiching the victim transaction.

Lines of code Vulnerability details Impact The attacker can steal the ETH of users that wanted to buy an NFT by sandwiching the victim transaction. Proof of Concept In non-atomic some of the orders could fail and funds should return and the end of execute function - . But the returnETHIfAny...

Any user can collect tokens trapped in the aggregator

Lines of code Vulnerability details Impact Any user can execute a trade on the aggregator to collect trapped tokens. Which should be an action only allowed by the owner. The issue is in how returnERC20TokensIfAny gets the amount to send back by checking the balances of the contract. Proof of...

No Revert on Failure, an order can be execute successfully by disguising them as NFTs.

Lines of code Vulnerability details Impact ERC20 tokens don't throw a error when failed in transfer. A malicious user can pretend to be sending an ERC721 token while it is something else. Orders are getting executed inside LooksRareProxy.sol, an attacker as a maker can make an BasicOrder that has...

The owner of the contract can broke the storage of the LooksRareAggregator contract

Lines of code Vulnerability details Impact The owner of the contract can broke the storage of the LooksRareAggregator contract Proof of Concept The addFunction function - can be called by the owner to to set proxy address and function selector for the function to be called inside this proxy. This...

Theft of ETH that was not used for successful execution of orders in non-atomic execution

Lines of code Vulnerability details Description There is an execute function in LooksRareAggregator contract. It refunds any ETH that was unused for example that left due to the unsuccessful execution of an order at the end of its execution flow: returnETHIfAnyoriginator; returnETHIfAny function ...

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

calling execute() may lead to stealing funds if some ERC20 is stuck on the contract

Lines of code Vulnerability details Impact if some tokens is sent erroneously or not to the contract, anyone that calls correctly LooksRareAggregator.execute will be able to steal those coins. to execute the function using ethers as payment, these conditions must be true: there is at least a trad...

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

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

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

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

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

Buyers unused ETH funds can be stolen (Direct theft of funds)

Lines of code Vulnerability details Impact The protocol has recognized the need to track buyers ETH in order to refund unused ETH by implementing the returnDust function and setupExecution modifier. The implementation creates an attack vector that allows the seller to steal the unused ETH...

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

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

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

LowLevelETH: _returnETHIfAny; _returnETHIfAny; _returnETHIfAnyWithOneWeiLeft do not check if call was successful

Lines of code Vulnerability details Impact This means that the caller won't receive ETH but the transaction will continue, this could specially affects to function LookRareAggregator.execute, which use returnETHIfAnyaddress function, leading to originator lose of funds or mess up its state...