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

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

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

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

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

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

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

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

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

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

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

Overpayment of ETH is not refunded to the buyer

Lines of code Vulnerability details Impact Overpayment of ETH is not refunded to the buyer Proof of Concept ' function executeFundsTransfer address seller, address buyer, address paymentToken, Fee calldata fees, uint256 price internal if msg.sender == buyer && paymentToken == address0...

The bulkExecute function does not bypass failed executions.

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. According to the documentation, "Add bulk execute function; attempted executions that fail should be bypassed". However, the implementation of bulkExecute does not conform to this spec. Fail executions...

Upgraded Q -> M from #272 [1668215320788]

Judge has assessed an item in Issue 272 as M risk. The relevant finding follows: Inaccurate comment can be misleading. The following retrievePledgeRewards function is for retrieving the non-distributed rewards from an expired pledge, not just a closed pledge. Calling this function for a pledge th...

Loss of user funds

Lines of code Vulnerability details Impact The LooksRareAggregator contract can receive ether but cannot return it unless through the returnERC20TokensIfAny or returnETHIfAny function. This means that there is a scenario where a user can accidentally transfer funds to this contract and those fund...

Upgraded Q -> M from #107 [1668211909800]

Judge has assessed an item in Issue 107 as M risk. The relevant finding follows: L-01 WardenPledge inherits Ownable instead of Owner The contract imports Owner.sol but inherits Ownable - should both import and inherit Owner --- The text was updated successfully, but these errors were encountered:...

Replay attack for authorized orders

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. Nonce is used by sellers and buyers to avoid replay attack for their orders. However, since sellHash and buyHash only contain the order info and the nonce, it is still possible to have a replay attack...

Upgraded Q -> M from #43 [1668210521611]

Judge has assessed an item in Issue 43 as M risk. The relevant finding follows: Q-02 - Huge trust assumption in recoverErc20 Reward tokens are transferred into this contract when a pledge is created. The comment notes " Recovers ERC2O tokens sent by mistake to the contract", but this function can...

Upgraded Q -> M from #20 [1668209885833]

Judge has assessed an item in Issue 20 as M risk. The relevant finding follows: 2. Ownable and Pausable The contract WardenPledge is Ownable and Pausable, so the owner could resign while the contract is paused, causing a Denial of Service. Owner resignation while the contract is paused should be...

Unsafe usage of ERC20 transfer and transferFrom

Lines of code Vulnerability details Impact Some ERC20 tokens functions don’t return a boolean, for example, USDT, BNB, OMG. So the LowLevelERC20Transfer contract simply won’t work with tokens like that as the token. Proof of Concept USDT’s transfer and transferFrom functions don't return a bool,...

Re-Entrancy when invoking ERC20EnabledLooksRareAggregator.execute() function

Lines of code Vulnerability details Impact We didn't check the address of currency . We don't know if the currency is ERC20 or not, and then just invoke currency.call. For these functions, we don't have any Re-Entrancy check. It will definitely lead to an Re-Entrancy issue on the currency side...

The execute() function of SeaportProxy.sol will always fail.

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. While L69 of SeaportProxy.sol is successful in preventing the function being called by a contract other than the LooksRareAggregator, unfortunately, the current implementation will fail the calling from...

Closing a single credit line can be reentered and allows closing all open credit lines

Lines of code Vulnerability details Closing a single credit line can be reentered and allows decrementing count as many times as needed to reach the value 0. As soon as the value is 0, the status of all credit lines will be updated to LineLib.STATUS.REPAID. Reentrancy is possible by the borrower ...

Revenue stream split can be bypassed

Lines of code Vulnerability details The Spigot.claimRevenue function allows anyone to claim revenue tokens from the spigot push and pull payments and escrows them for the owner to withdraw later. The revenue is automatically split between the treasury and escrow according to the settings in...

Chainlink USD price feeds can have more than 8 decimals

Lines of code Vulnerability details Impact Collateral value is overestimated by orders of magnitude if asset that has a chainlink oracle USD price feed with more than 8 decimals is added. A borrower can borrow much more than allowed in an EscrowedLine Proof of Concept In Oracle.sol the USD price...

When borrower repays, it can overflow and make them owe 2^256 tokens to lender.

Lines of code Vulnerability details Description CreditLib's repay function is the actual accounting of repayments in a LineOfCredit: function repay ILineOfCredit.Credit memory credit, bytes32 id, uint256 amount external returns ILineOfCredit.Credit memory unchecked if amount =...

Borrower can craft a borrow that cannot be liquidated, even by arbiter.

Lines of code Vulnerability details Description LineOfCredit manages an array of open credit line identifiers called ids. Many interactions with the Line operate on ids0, which is presumed to be the oldest borrow which has non zero principal. For example, borrowers must first deposit and repay to...

Borrower can mark his line of credit as repaid by calling line.close(fakeId) repeatedly

Lines of code Vulnerability details Description This vulnerability can be summarized in the following 4 observations: 1. There are no checks to prevent calling line.close with id that doesn’t correspond to any open position. 2. The effect of calling line.close with a fake position id is...

Borrower can put their address when submiting addCredit() which can make the lender not be able to withdraw tokens

Lines of code Vulnerability details Impact when calling addCredit by borrower, they can choose to put in their address as the lender instead of the actuall lender. this can bring problems later on when the lender is trying to withdraw funds from the contract. they wont be able to withdraw the fun...

Borrower can manipulate the repayment queue, avoid paying back the initial lender

Lines of code Vulnerability details Impact In contract LineOfCredit, the repayment queue protects lenders and assured that they have to get paid back. Without a queue, a borrower could constantly refinance to lower rates and avoid paying back the initial lender from sponsors. Every time, borrower...

Line of credit status can be set to REPAID even if having credits with debt

Lines of code Vulnerability details A malicious borrower can close non-existing credits to alter the status of the credit to LineLib.STATUS.REPAID, even if having open credit with debt. Impact The close function in the LineOfCredit contract can be used to close non-existing credits, which will...

ETH can be accidentally sent with ERC20 in sendOutTokenOrETH function

Lines of code Vulnerability details Impact ETH can be accidentally sent with ERC20 in sendOutTokenOrETH function User who wants to send ERC20 due to payable function can write a value in the value field Proof of Concept contracts/utils/LineLib.sol: 33 / 34: function sendOutTokenOrETH 35: address...

SpigotLib._claimRevenue is marked public instead of internal

Lines of code Vulnerability details Impact SpigotLib.claimRevenue is marked public instead of internal. This public function is wrapped in the external claimRevenue function. Attacker can call claimRevenue to claim Revenue Tokens into the Spigot escrow for later withradrawal. Tools Used Manual...

Logic in LineLib does not work correctly with fee-on-transfer tokens

Lines of code Vulnerability details Proof of Concept Some tokens take a transfer fee e.g. STA, PAXG, some do not currently charge a fee but may do so in the future e.g. USDT, USDC. The code currently receives ERC20 tokens but does not account for the potential fees paid in the transfer. If such a...

SpigotedLineLib::trade won’t work with tokens with approval race protection (USDT)

Lines of code Vulnerability details Proof of Concept Some tokens e.g. USDT, KNC do not allow approving an amount M 0 when an existing amount N 0 is already approved. This is to protect from an ERC20 attack vector described here. The problem is the code in trade is the following...

Use of .transfer()

Lines of code Vulnerability details Impact Note: I realize that this was flagged in the C4udit output, though it seems to believe that it's an ERC20 transfer. The use of transfer to send ETH only forwards 2300 gas to the recipient. This is not enough gas to execute a gnosis safe delegatecall. Fun...

Borrower's principal can be enormous due to underflow

Lines of code Vulnerability details Impact The lender and the borrower can repay borrower's debt using unusedTokens in SpigotedLine.useAndRepay function. The repay amount can be as much as unusedTokenscredit.token in SpigotedLine.useAndRepay143. The actual repay is handled in CreditLib.repay, and...

State varaible marketplace has never been initialized in LooksRareProxy.sol

Lines of code Vulnerability details Impact Detailed description of the impact of this finding. State varaible marketplace has never been initialized in LooksRareProxy.sol. Although it has been initialized in the constructor, it won't count since functions such as execute will only be called via...

revenueContract that uses AccessControl instead of Ownable can’t work with SpigotLib's removeSpigot()

Lines of code Vulnerability details Proof of Concept The code in removeSpigot is the following bool success, = revenueContract.call abi.encodeWithSelector self.settingsrevenueContract.transferOwnerFunction, self.operator // assume function only takes one param that is new owner address ; So...

## Potential for loops hitting the block gas limit

Lines of code Vulnerability details Proof of Concept A malicious/compromised LineOfCredit arbiter can add a huge amount of collaterals to the Escrow by calling enableCollateral. This method pushes each collateral token to an array that is then iterated over in getCollateralValue. If the array siz...

Potential DoS when closing a credit nominated in ETH in the LineOfCredit contract

Lines of code Vulnerability details When closing a credit that was issued in ETH, the LineOfCredit contract will send the lender his deposit and any accrued interests using the address.transferamount function, which may fail and revert the whole function, leading to an eventual DoS. Impact The...

Logic in SpigotLib will not work with rebasing tokens

Lines of code Vulnerability details Proof of Concept Some tokens may make arbitrary balance modifications outside of transfers e.g. Ampleforth style rebasing tokens, Compound style airdrops of governance tokens, mintable / burnable tokens. The code in the claimEscrow function uses a previously...

Lender will not receive facilityFee if borrower leaves the borrow open with principal zero.

Lines of code Vulnerability details Description The SecuredLine's liquidate function is supposed to be a defense of last resort, for arbiter to claim collateral from borrower who has not paid: function liquidate uint256 amount, address targetToken external whileBorrowing returnsuint256 ifmsg.send...

LineLib.receiveTokenOrETH() can accept more ETH than it accounts for

Lines of code Vulnerability details Impact LineLib.receiveTokenOrETH can receive ETH and accounts for an amount to receive but it is possible to send more ETH than amount, meaning that any extra ETH sent will be lost to the contract. Proof of Concept LineLib.receiveTokenOrETH is as follows...

Reentrancy in _close() allows single lender to steal all deposits from other lenders

Lines of code Vulnerability details Impact Upon calling close, a lender's credit position is deleted AFTER the transfer out of their deposit. Therefore, an ERC777 will allow the lender to call close again and receive the same amount of funds. The lender will be able to reenter the contract as man...

Closing an unexisting credit can overflow the credit count variable in the LineOfCredit contract

Lines of code Vulnerability details The LineOfCredit contract doesn't verify if a credit exists and is properly initialized when closing it, and will update its state anyway, leading to an inconsistent count. Impact The close function in the LineOfCredit contract doesn't check if the given credit...

Functions addCredit(...) and increaseCredit(...) can lock lender’s ETH forever

Lines of code Vulnerability details Impact In LineOfCredit contract, both functions addCredit... and increaseCredit... require mutual consent between lender and borrower. If lender is tricked by borrower, or by mistake, lender ETH will be locked in the contract forever. function addCredit uint128...

Using keccak256(abi.encoded()) can result in collisions

Lines of code Vulnerability details Proof of Concept The code in mutualConsent makes use of keccak256abi.encodePacked but this can result in a collision when the arguments of abi.encodePacked are aligned in a way that gives the same result. Impact Having hash collisions in mutualConsent...

Missing ReEntrancy Guard to sendOutTokenOrETH function

Lines of code Vulnerability details Proof of Concept There is no re-entry risk on true ERC-20 tokens that work according to the spec i.e. audited, etc.. However you can write a malicious ERC-20 with custom transferFrom or approve that have re-entrancy hooks to attack a target. Furthermore ERC-777...

Having nonzero allowance to the exchange will prevent any future trades

Lines of code Vulnerability details Impact A borrower can prevent other lenders to call claimAndRepay, in other words the lenders can not use the revenue streams in the spigotedLine. Proof of Concept Suppose a borrower has a stream of revenue token, and also borrowed some tokens already. Then the...