ERC20 return values not checked

Lines of code Vulnerability details Impact 'token' is a ERC20 from LPToken.sol The ERC20.transfer and ERC20.transferFrom functions return a boolean value indicating success. This parameter needs to be checked for success. Some tokens do not revert if the transfer failed but return false instead...

Slashing can prevent Protected Staking Pool depositors from redeeming their ETH

Lines of code Vulnerability details The documentation states that: The Protected Staking Pool is free from any slashing and leaking risk. While the penalties affect the node operator first, then the Fees and MEV Pool stakers, if the slashing amount is higher, it will also impact the Protected...

EOAs and system contracts can be blocked from some actions by continuously transferring them zero LP tokens

Lines of code Vulnerability details Impact LPToken contains the map lastInteractedTimestamp which maps addresses to timestamps and is updated for from and to addresses after a token transfer. Many operations will check the last interaction time of an address and revert if it's too recent to comba...

Medium: Users receive an incorrect account of their accumulated ETH in GiantMevAndFeesPool.

Lines of code Vulnerability details Description GiantMEVAndFeesPool exposes previewAccumulatedETH for users to view how much ETH they have accumulated through the vault: function previewAccumulatedETH address user, address calldata stakingFundsVaults, LPToken calldata lpTokens external view retur...

Users can block other users from redeeming their ETH in Vaults

Lines of code Vulnerability details The burnLPToken of a protected vault allow users to burn LP tokens in exchange of ETH or dETH. In the case of ETH, ie when the BLS key has not had its derivatives minted yet, the function checks the liquidity is not fresh by checking...

Single-step process for critical ownership transfer/renounce is risky

Lines of code Vulnerability details Single-step process for critical ownership transfer/renounce is risky Impact The following contracts and functions, allow owners to interact with core functions such as: execute, rawExecute and setApproval in OwnableSmartWallet registerKnotsToSyndicate,...

Medium: Giant pools are prone to user griefing, preventing their holdings from being staked.

Lines of code Vulnerability details Description batchRotateLPTokens in GiantMevAndFeesPool allows any user to rotate LP tokens of stakingFundsVaults around. function batchRotateLPTokens address calldata stakingFundsVaults, LPToken calldata oldLPTokens, LPToken calldata newLPTokens, uint256 callda...

SLOT owners lose half their value when there is no sETH deposited to the Syndicate.

Lines of code Vulnerability details Description The Syndicate contract uses total balance as collateral for both sETH accumulatedETHPerFreeFloatingShare and SLOT rewards accumulatedETHPerCollateralizedSlotPerKnot. They are updated in the following function: function updateAccruedETHPerShares publ...

Variable claimed its being resetting instead of adding

Lines of code Vulnerability details Impact The variable claimed that keeps tracking of the total amount claimed per user per token its being being resetting with a wrong value. This impacts on the line due calculation on SyndicateRewardsProcessor.solL61 uint256 due = accumulatedETHPerLPShare...

Any user can block other users from interacting with LPTokens by transferring tiny amounts and resetting their lastInteractedTimestamp

Lines of code Vulnerability details Impact LPTokens in several different contracts have an afterTokenTransfer hook, which updates the last interacted timestamp. Because users can send LPTokens to any other user, this resets their last interacted timestamp which is used for several key actions whi...

dETH are branded as slash proof, but ETH2 slashing could make 32 deposit drop much below 24 (down to 0), making dETH undercollateralized

Lines of code Vulnerability details Description dETH is advertised as fault proof , slash proof ETH However, ETH2 staked deposit can be slashed from 32 down to 0, not just to 24 as would be expected24 dETH printed. This means dETH is undercollateralized, and indeeds bears the risk of being "paper...

Upgraded Q -> M from #164 [1668687728737]

Judge has assessed an item in Issue 164 as M risk. The relevant finding follows: 01 Lack of check if dust ether transfer is successful --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> M from #209 [1668684399391]

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

Upgraded Q -> M from #74 [1668688523897]

Judge has assessed an item in Issue 74 as M risk. The relevant finding follows: Low-7 EXCHANGE & SWAP are able to transfer any user funds to other addresses --- The text was updated successfully, but these errors were encountered: All reactions...

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

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

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

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

The setupExecution is reentrancy attack vulnerable

Lines of code Vulnerability details Impact The setupExecution can be re-entered by calling bulkExecute inside an execution. Because the global state remainingETH and isInternal are modified reset to 0 and flase after the latter reentrant call, the previous call is affected: 1. all subsequent...

Direct theft of buyers ETH funds.

Lines of code Vulnerability details Impact Most severe issue: A Seller or Fee recipient can steal ETH funds from the buyer when he is making a single or bulk execution. Direct theft of funds. Additional impacts that can be caused by these bugs: 1. Seller or Fee recipient can cause next in line...

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

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

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 #381 [1668467789168]

Judge has assessed an item in Issue 381 as M risk. The relevant finding follows: There is no limit for FlashLoanFee function setFlashLoanFeeuint256 flashLoanFee external override onlyOwner uint256 oldFlashLoanFee = flashLoanFee; if oldFlashLoanFee == flashLoanFee revert...

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

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

Upgraded Q -> M from #205 [1668464751854]

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

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

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

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

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 #449 [1668465456097]

Judge has assessed an item in Issue 449 as M risk. The relevant finding follows: Error when calling beforeTokenTransfer in the LBToken burn function | Low | 1 --- The text was updated successfully, but these errors were encountered: All reactions...

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

Recipients Can Steal Remaining ETH from Exchange

Lines of code Vulnerability details Impact This vulnerability allows recipients to reenter the Exchange contract, stealing remaining ETH. When a batch of orders are matched using the bulkExecute method a malicious recipient of assets may reenter the bulkExecute method, stealing ETH that would've...

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

The function _execute could be called externally

Lines of code Vulnerability details Author: rotcivegaf Impact The execute use a modifier to only can called internally, also specified in the documentation of the function: Must be called internally. But this modifier can be pass if a contract call the execute or bulkExecute and in the returnDust...

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

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

Exchange owner can consume all orders at arbitrary price

Lines of code Vulnerability details Impact The choice of policy to use for a transaction is determined by the listingTime. The listingTime can be supplied by the caller of execute/bulkExecute and can be arbitrary as along as it passes validation. And the policy of a given order is used to determi...

Attacker can spoof remainingETH and double-spend their input ETH to Exchange

Lines of code Vulnerability details Description remainingETH is an important state variable in Exchange.sol, which keeps track of how many ETH have yet to be used as payment from the current msg.value. The setupExecution modifier sets the value before and after execution: modifier setupExecution...

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 #471 [1668464969039]

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

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

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

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

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

A malicious user can steal all the excess balance on the pool by calling the execute function with higher amount than the selling amount

Lines of code Vulnerability details Impact When an order is matched, the Buyer has the option to pay in either ETH, WETH or via the Pool contract. The Exchange smart contract implements a function returnDust which returns the extra ETH to the user, if she overpays. The function is implemented in...

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

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