Possibly reentrancy attacks in _distributeETHRewardsToUserForToken function

Lines of code Vulnerability details Author: rotcivegaf Impact The root of the problem are in the distributeETHRewardsToUserForToken who makes a call to distribute the ether rewards. With this call the recipient can execute an reentrancy attack calling several times the different function to steal...

withdrawETHForKnot is vulnerable to reentrancy attack

Lines of code Vulnerability details Impact The withdrawETHForKnot is vulnerable to reentrancy because the transfer is done before an important state change. Proof of Concept function withdrawETHForKnotaddress recipient, bytes calldata blsPublicKeyOfKnot external requirerecipient != address0, "Zer...

bringUnusedETHBackIntoGiantPool in GiantSavETHVaultPool can be used to steal LPTokens

Lines of code Vulnerability details Impact real LPTokens can be transferred out of GiantSavETHVaultPool through fake savETHVaults provided by an attacker. Proof of Concept bringUnusedETHBackIntoGiantPool takes in savETHVaults, lpTokens, and burns certain amount olpTokens. The tokens are thoroughl...

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

Multi reentrancy issues leveragin on _distributeETHRewardsToUserForToken method

Lines of code Vulnerability details Impact Multiple reentrancy issues mainly trigger by a low call ETH transfer. Proof of Concept Everytime you use the distributeETHRewardsToUserForToken method on SyndicateRewardsProcessor.solL67 there are many reentrancy opportunities for an attacker. For exampl...

batchDepositETHForStaking in GiantSavETHVaultPool.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, savETHVault is checked for its validity through SavETHVault savETHPool = SavETHVaultsavETHVaultsi; require...

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

Medium: Attacker may withdraw arbitrary amount from smart wallet, even if state checks would not normally allow it

Lines of code Vulnerability details Description withdrawETHForKnot in LiquidStakingManager suffers from reentrancy attack. function withdrawETHForKnotaddress recipient, bytes calldata blsPublicKeyOfKnot external requirerecipient != address0, "Zero address";...

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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