Unbounded loop can run out of gas in withdraw and unstake operations

Lines of code Vulnerability details Unbounded loop can run out of gas in withdraw and unstake operations Impact There are no bounds on the number of orders in the loop, this can run out of gas due to cost of the operations that can vary by the time. Proof Of Concept function withdrawLPTokensLPTok...

batchDepositETHForStaking in GiantMevAndFeesPool.sol can be ticked to steal all ETH in the pool

Lines of code Vulnerability details Impact All Eth can be drained by fake vault addresses. Proof of Concept In batchDepositETHForStaking, stakingFundsVault is checked for its validity through StakingFundsVault sfv = StakingFundsVaultpayablestakingFundsVaulti; require...

batchRotateLPTokens in GiantSavETHVaultPool can be used to steal LPTokens

Lines of code Vulnerability details Impact real LPTokens can be transferred out of GiantSavETHVaultPool through fake stakingFundsVaults provided by an attacker. Proof of Concept batchRotateLPTokens takes in stakingFundsVaults, oldLPTokens, newLPTokens and rotate amounts from old to new tokens. Th...

HIGH: Vault or pool funds can be stolen by any user who holds LP tokens.

Lines of code Vulnerability details Description The distributeETHRewardsToUserForToken is implemented in SyndicateRewardsProcessor and used in derived contracts: GiantMevAndFeePool and StakingFundsVault. It sends out rewards to the given user according to the current accumulatedETHPerLPShare and...

Node operators cannot withdraw their ETH when they rage quit

Lines of code Vulnerability details Node operators can opt for Rage Quit, after the BLS public key is staked. In the current configuration, they will not be able to retrieve their staked ETH: withdrawETHForKnot will revert here if the lifecycle status is not INITIALSREGISTERED. The lifecycle stat...

HIGH : The Giant pools can be drained by any user.

Lines of code Vulnerability details Description In GiantSavETHVaultPool.sol, batchDepositETHForStaking is used to deposit held funds to savETHPool vault: function batchDepositETHForStaking address calldata savETHVaults, uint256 calldata ETHTransactionAmounts, bytes calldata blsPublicKeys, uint256...

Medium: Node runner whitelisting functionality is broken

Lines of code Vulnerability details Description isNodeRunnerWhitelisted manages whitelisting of nodeRunners. If whitelisting is enabled, this mapping is checked in isNodeRunnerValid: function isNodeRunnerValidaddress nodeRunner internal view returns bool requirenodeRunner != address0, "Zero...

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

bringUnusedETHBackIntoGiantPool in GiantMevAndFeesPool can be used to steal LPTokens

Lines of code Vulnerability details Impact real LPTokens can be transferred out of GiantMevAndFeesPool through fake stakingFundsVaults provided by an attacker. Proof of Concept bringUnusedETHBackIntoGiantPool takes in stakingFundsVaults, oldLPTokens, newLPTokens and rotate amounts from old to new...

batchRotateLPTokens in GiantMevAndFeesPool can be used to steal LPTokens

Lines of code Vulnerability details Impact real LPTokens can be transferred out of GiantMevAndFeesPool through fake stakingFundsVaults provided by an attacker. Proof of Concept batchRotateLPTokens takes in stakingFundsVaults, oldLPTokens, newLPTokens and rotate amounts from old to new tokens. The...

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

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

RETURN STATEMENT OF THE transferFrom FUNCTION ALWAYS RETURN TRUE EVEN THE TRANSFER IS FAILED . IF RETURN TRUE FOR ALL TRANSACTIONS IT WILL CAUSE A PROBLEM AND LOSE OF AMOUNT

Lines of code Vulnerability details Impact When ever we calling transfer from function it will return true even the transfer failed. So as per function we think the transfer is success. But in real that transfer may or may not be failed. We don't get the exact status of the transfer Proof of...

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

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

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

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

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

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 is not initialized correctly

Lines of code Vulnerability details Impact Pool has no owner and will be un-upgradeable. Proof of Concept Pool does not provide an initialize interface to initialize the owner, so the owner will never be set. Pool as a UUPSUpgradeable can not be upgraded without a valid owner. Tools Used n/a...

Upgraded Q -> M from #348 [1668468155840]

Judge has assessed an item in Issue 348 as M risk. The relevant finding follows: L01 Wrong parameter order inside beforeTokenTransfer in LBtoken::burn call should be beforeTokenTransferaccount, address0, id, amount; --- The text was updated successfully, but these errors were encountered: All...

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

Counterparty-exchange owner may alter policy manager in execution callback, altering the price of subsequent order matching

Lines of code Vulnerability details Impact In the context of bulkExecute, with a sqeuence of executions requested, transferring ETH, ERC721 or ERC1155 may result in a hook/callback to a counterparty on receipt of ETH or one of these tokens. If this counterparty is also the exchange owner, or is...

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

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

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 #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 #75 [1668466949988]

Judge has assessed an item in Issue 75 as M risk. The relevant finding follows: Line 237, beforeTokenTransferaddress0, account, id, amount; should be beforeTokenTransferaccount, address0, id, amount; --- The text was updated successfully, but these errors were encountered: All reactions...

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

Anyone can take the remaining ETH in the Exchange (proxy) contract

Lines of code Vulnerability details Impact Anyone can take the remaining ETH in the Exchange proxy contract Proof of Concept Because returnDust just send all ETHs of the Exchange balance to caller, anyone can call bulkExecuteexecutions with 1wei and an empty executions list to drain the contract:...

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

Susceptible to reorg attack

Lines of code Vulnerability details Impact When reorg happens, it is possible that the cancelled order by the taker be executed by the maker. Proof of Concept Suppose userA is seller and userB is buyer, seller is maker, and buyer is taker. After some time, userB the buyer cancels the order by...

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

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

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

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 #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 does not revert ether transfer failure

Lines of code Vulnerability details Impact In function returnDust the return value of call is not checked for success. If a user had sent excess ether or if an order in a bulk order had failed, and if the call fails ether will remain in the contract. Users who execute orders later will be able to...

Upgraded Q -> M from #403 [1668467487185]

Judge has assessed an item in Issue 403 as M risk. The relevant finding follows: 1. No upper limit check on flash loan fee No upper limit check on setFlashLoanFee. Although only admin can set this, human error can lead to higher percente swap, leading to excess swap and gas fee. Or admin can rug...

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

Trader can still execute the order even after cancelling the order

Lines of code Vulnerability details Impact There are no checks to verify whether the order has been cancelled or not in execute function. This will enable traders to place the order even after cancellation Proof of Concept Execute cancelOrder function with Order data Include the Order in input of...

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

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

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

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