migrateETH will not work

Handle gzeon Vulnerability details Impact migrateETH does not send the withdrawn ETH to L2 causing fund to stuck in the L1Migrator contract. Proof of Concept When migrateETH is called, it would withdraw all ETH from bridgeMinter, and then use sendTxToL2 create a L2 retryable ticket to call the...

Incorrect erc20 interface

Handle 0v3rf10w Vulnerability details Impact Incorrect return values for ERC20 functions results in failure when interacting with other contract , as the return value is missing. Proof of Concept ApproveLike contracts/L1/gateway/L1Migrator.sol67-69 has incorrect ERC20 function...

Anyone can freeze fund in BridgeMinter

Handle gzeon Vulnerability details Impact Anyone can call migrateETH and migrateLPT in L1Migrator with arbitrary maxSubmissionCost. For example when migrateLPT is called, it would withdraw all LPT from bridgeMinter, and then create a L2 retryable ticket to call the L2Migrator using...

[WP-M0] MINTER_ROLE can be granted by the deployer of L2LivepeerToken and mint arbitrary amount of tokens

Handle WatchPug Vulnerability details function mintaddress to, uint256 amount external override onlyRoleMINTERROLE mintto, amount; emit Mintto, amount; Using the mint function of L2LivepeerToken, an address with MINTERROLE can burn an arbitrary amount of tokens. If the private key of the deployer...

Incorrect erc20 interface

Handle 0v3rf10w Vulnerability details Impact Incorrect erc20 implementation can lead to failure when interacting with contarcts Proof of Concept IBridgeMinterToken contracts/token/BridgeMinter.sol5-13 has incorrect ERC20 function interface:IBridgeMinterToken.transferaddress,uint256...

[WP-M2] DEFAULT_ADMIN_ROLE can approve arbitrary address to spend any amount from the L1Escrow contract

Handle WatchPug Vulnerability details function approve address token, address spender, uint256 value public onlyRoleDEFAULTADMINROLE ApproveLiketoken.approvespender, value; emit Approvetoken, spender, value; L1Escrow.solapprove allows an address with DEFAULTADMINROLE can approve an arbitrary amou...

Use safeTransferFrom consistently instead of transferFrom

Handle Jujic Vulnerability details Impact It is good to add a require statement that checks the return value of token transfers, or to use something like OpenZeppelin’s safeTransferFrom unless one is sure the given token reverts in case of a failure. Failure to do so will cause silent failures of...

Admin can rug L2 Escrow tokens leading to reputation risk

Handle harleythedog Vulnerability details Impact The L1Escrow contract has the function approve that is callable by the admin to approve an arbitrary spender with an arbitrary amount so they can steal all of the escrow's holdings if they want. Even if the admin is well intended, the contract can...

l1CirculatingSupply may not give correct value after a mass withdrawal from L2

Handle hubble Vulnerability details Impact In the event of a mass withdrawal from L2, the l2SupplyFromL1 is set to 0 in the function decreaseL2SupplyFromL1. Now if LPT is moved again from L1 to L2, some of them may be the excess LPT withdrawn from above case. So the value of l1CirculatingSupply...

Griefing attack on migrateLPT can prevent token transfer to L2

Handle harleythedog Vulnerability details Impact In L1Migrator.sol the function migrateLPT can be called by anyone. A malicious user can call migrateLPT with a small maxSubmissionCost argument to intentionally make the retryable ticket creation fail. This will lock the LPT in the L1 escrow. Now,...

[WP-H3] L1Migrator.sol#migrateETH() Improper implementation of L1Migrator causing migrateETH() always reverts, can lead to ETH in BridgeMinter getting stuck in the contract

Handle WatchPug Vulnerability details uint256 amount = IBridgeMinterbridgeMinterAddr .withdrawETHToL1Migrator; L1Migrator.solmigrateETH will call IBridgeMinterbridgeMinterAddr.withdrawETHToL1Migrator to withdraw ETH from BridgeMinter. However, the current implementation of L1Migrator is unable to...

[WP-H5] L1Migrator.sol#migrateETH() dose not send bridgeMinter's ETH to L2 causing ETH get frozen in the contract

Handle WatchPug Vulnerability details Per the arb-bridge-eth code: all msg.value will deposited to callValueRefundAddress on L2 uint256 seqNum = inbox.createRetryableTicketvalue: l1CallValue target, l2CallValue, maxSubmissionCost, from, from, maxGas, gasPriceBid, data ; At L308-L309, ETH held by...

[WP-M1] BURNER_ROLE can burn any amount of L2LivepeerToken from an arbitrary address

Handle WatchPug Vulnerability details function burnaddress from, uint256 amount external override onlyRoleBURNERROLE burnfrom, amount; emit Burnfrom, amount; Using the burn function of L2LivepeerToken, an address with BURNERROLE can burn an arbitrary amount of tokens from any address. We believe...

fundDepositAndReserveFor function does not exist in protocol

Handle harleythedog Vulnerability details Impact The L2Migrator contract makes use of the function fundDepositAndReserveFor on the ticket broker. In the commit hash for the contest seen from this snippet from the contest page: git clone https://github.com/livepeer/protocol git checkout...

LivepeerToken.burn function could burn tokens of any user

Handle cccz Vulnerability details Impact Same as code-423n4/2021-11-overlay-findings22, the burner could burn any amount of tokens of any user. Proof of Concept Tools Used Manual analysis Recommended Mitigation Steps Update burn function for only owner can burn his tokens. --- The text was update...

L2Migrator allows a user to migrate once through claimStake() and once through finalizeMigrateDelegator()

Handle Ruhum Vulnerability details Impact There are two ways to migrate from L1 to L2. Either through the cross-chain or the snapshot migration, as specified here But, a user is able to migrate twice by using both options. Proof of Concept The issue is that the migratedDelegator map is not used...

transfer return value of a general ERC20 is ignored

Handle pants Vulnerability details Need to use safeTransfer instead of transfer. As there are popular tokens, such as USDT that transfer/trasnferFrom method doesn’t return anything. The transfer return value has to be checked as there are some other tokens that returns false instead revert, that...

L1Migrator.migrateLPT` can be used to take away protocol's access to LPT tokens in BridgeMinter

Handle Ruhum Vulnerability details Vulnerability details Impact Same thing as the ETH issue I reported earlier. I wasn't sure if those are supposed to be a single issue or not. The concept is the same. But, now you lose LPT tokens. The L1Migrator.migrateLPT function can be called by anyone. It...

Unchecked returns in multiple place

Handle 0v3rf10w Vulnerability details Impact Multiple checks needed Proof of Concept L1LPTGateway.outboundTransferaddress,address,uint256,uint256,uint256,bytes contracts/L1/gateway/L1LPTGateway.sol80-123 ignores return value by TokenLikel1Token.transferFromfrom,l1LPTEscrow,amount...

approve return value is ignored

Handle pants Vulnerability details Some tokens don't correctly implement the EIP20 standard and their approve function returns void instead of a success boolean. Calling these functions with the correct EIP20 function signatures will always revert. Tokens that don't correctly implement the latest...

L1Migrator.migrateETH can be used to take away protocol's access to funds

Handle Ruhum Vulnerability details Impact The L1Migrator.migrateETH function can be called by anyone. It pulls all the ETH from the BridgeMinter contract and starts the process of moving the funds to L2. First of all, this function is only executable once. The RetryableTicket created with the fir...

Wrong logic in L2ArbitrumMessenger

Handle 0x1f8b Vulnerability details Impact Current logic doesn't work. Proof of Concept The method sendTxToL1 inside the contract L2ArbitrumMessenger has a wrong logic, it convert the value 100 to an address, in order to call sendTxToL1 method, but this converted address will never work, so the...

Duplicate total in getMigrateUnbondingLocksParams

Handle 0x1f8b Vulnerability details Impact Wrong total computation. Proof of Concept The method getMigrateUnbondingLocksParams inside the contract L1Migrator doesn't check that the array unbondingLockIds provided by the user has duplicate ids, if the user provide duplicate ids the total will be...

No reentrancy guards on functions using .call

Handle jayjonah8 Vulnerability details Impact In BridgeMinter.sol the migrateToNewMinter and withdrawETHToL1Migrator both use the .call function without adding reentrancy guard modifiers to the functions. This is important when using .call as functions can be reentered before execution is complet...

L1LPTGateway.sol does not make use of safeTransferFrom

Handle jayjonah8 Vulnerability details Impact In the L1LPTGateway.sol transferFrom is used in several parts of the file. Tokens that don’t correctly implement the latest EIP20 spec will be unusable in the protocol as they revert the transaction because of the missing return value. Proof of Concep...

No check that _to and from are different addresses in outboundTransfer() function

Handle jayjonah8 Vulnerability details Impact In L1LPTGateway.sol the outboundTransfer function transfers the l1Token from the msg.sender to the l1LPTEscrow contract. It also takes in the to argument which is set in the outboundCalldata variable. This function does not check if the msg.sender and...

Unbounded iteration over all indexes

Handle Dravee Vulnerability details Impact The transactions could fail if the array get too big and the transaction would consume more gas than the block limit. This will then result in a denial of service for the desired functionality and break core functionality. Proof of Concept Tools Used VS...

Looping over dynamic array might result in DoS through the block gas limit

Handle Ruhum Vulnerability details Impact Because of the block gas limit, looping over a dynamic array that grows over time might result in a DoS at some point. Both the PoolTemplate and the IndexTemplate have such dynamic arrays. Both don't have any functionality to decrease the size. Meaning, i...

Unbounded iteration over all indexes (2)

Handle Dravee Vulnerability details Impact The transactions could fail if the array get too big and the transaction would consume more gas than the block limit. This will then result in a denial of service for the desired functionality and break core functionality. Proof of Concept Tools Used VS...

Out of gas.

Handle Jujic Vulnerability details Impact There is no upper limit on allMarkets, it increments each time when a new market is added. Eventually, as the count of markets increases, gas cost of smart contract calls will raise until reaching an "Out of Gas" error or a "Block Gas Limit" in the worst...

[WP-M35] PoolTemplate#applyCover Unbounded for loops allows an attacker to malfunction applyCover(), making it impossible to change the marketStatus of the Pool to Payingout status

Handle WatchPug Vulnerability details function applyCover uint256 pending, uint256 payoutNumerator, uint256 payoutDenominator, uint256 incidentTimestamp, bytes32 merkleRoot, string calldata rawdata, string calldata memo external override onlyOwner requirepaused == false, "ERROR: UNABLETOAPPLY";...

No check that DEFAULT_ADMIN_ROLE is not the LivepeerToken contract itself

Handle jayjonah8 Vulnerability details Impact In LivepeerToken.sol the constructor sets the DEFAULTADMINROLE but does not ensure that the msgSender is not the contract itself. This is an important check to make in order to avoid costly mistakes during deployment. Proof of Concept LivepeerToken.so...

Malicious Market Creators Can Steal Tokens From Unsuspecting Approved Reference Accounts

Handle leastwood Vulnerability details Impact The current method of market creation involves calling Factory.createMarket with a list of approved conditions and references accounts. If a registered template address has templatesaddresstemplate.isOpen == true, then any user is able to call...

[WP-H24] Wrong design/implementation of permission control allows malicious/compromised Registry or Factory admin to steal funds from users' wallet balances

Handle WatchPug Vulnerability details The current design/implementation allows a market address registered on registry to call VaultaddValue and transfer tokens from an arbitrary address to a specified beneficiary up the approved amount at any time, and the beneficiary can withdraw the funds by...

backdoor in withdrawRedundant

Handle cmichel Vulnerability details The Vault.withdrawRedundant has wrong logic that allows the admins to steal the underlying vault token. function withdrawRedundantaddress token, address to external override onlyOwner if token == addresstoken && balance 0 // @audit they can rug users. let's sa...

Vault.withdrawRedundant() allows the owner to accidentally take out the vault's whole balance

Handle Ruhum Vulnerability details Impact The Vault.withdrawRedundant allows the owner to withdraw funds that are not accounted for. The function has a check that is supposed to stop the owner from withdrawing funds of the vault's underlying token that the vault "knows" about. But, there's an edg...

Vaults don't work with fee-on transfer tokens

Handle cmichel Vulnerability details Certain ERC20 tokens make modifications to their ERC20's transfer or balanceOf functions. One type of these tokens is deflationary tokens that charge a certain fee for every transfer or transferFrom. Impact The Vault.addValueBatch functions will recive less...

Initial pool deposit can be stolen

Handle cmichel Vulnerability details Note that the PoolTemplate.initialize function, called when creating a market with Factory.createMarket, calls a vault function to transfer an initial deposit amount conditions1 from the initial depositor references4: // PoolTemplate function initialize string...

[WP-H30] A malicious/compromised Registry or Factory admin can drain all the funds from the Vault contracts

Handle WatchPug Vulnerability details modifier onlyMarket require IRegistryregistry.isListedmsg.sender, "ERRORONLYMARKET" ; ; function borrowValueuint256 amount, address to external onlyMarket override debtsmsg.sender += amount; totalDebt += amount; IERC20token.safeTransferto, amount; The current...

[WP-H39] PoolTemplate.sol#resume() Wrong implementation of resume() will compensate overmuch redeem amount from index pools

Handle WatchPug Vulnerability details Root Cause Wrong arithmetic. uint256 deductionFromIndex = debt totalCredit MAGICSCALE1E6 / totalLiquidity; uint256 actualDeduction; for uint256 i = 0; i 0 uint256 shareOfIndex = credit MAGICSCALE1E6 / totalCredit; uint256 redeemAmount = divCeil...

Index compensate is 0 when totalLiquidity() is enough to cover the whole amount

Handle pauliax Vulnerability details Impact In IndexTemplate, function compensate, When amount value, and = totalLiquidity, the value of compensated is not set, so it gets a default value of 0: if value = amount ... compensated = amount; else ... if totalLiquidity amount ... compensated = value +...

[WP-H33] IndexTemplate.sol Wrong implementation allows lp of the index pool to resume a locked PayingOut pool and escape the responsibility for the compensation

Handle WatchPug Vulnerability details Based on the context, the system intends to lock all the lps during PayingOut period. However, the current implementation allows anyone, including LPs to call resume and unlock the index pool. It allows a malicious LP to escape the responsibility for the...

[WP-H27] IndexTemplate.sol#compensate() will most certainly fail

Handle WatchPug Vulnerability details Root Cause Precision loss while converting between the amount of shares and the amount of underlying tokens back and forth is not handled properly. uint256 shortage; if totalLiquidity amount //Insolvency case shortage = amount - value; uint256 cds =...

no check that DEFAULT_ADMIN_ROLE is not the contract itself in L1Escrow.sol

Handle jayjonah8 Vulnerability details Impact In L1Escrow.sol the constructor sets up the DEFAULTADMINROLE without requiring that the msgSender is not the L1Escrow contract itself. Proof of Concept constructor setupRoleDEFAULTADMINROLE, msgSender; Tools Used Manual code review Recommended...

unlock function is callable by anyone

Handle Fitraldys Vulnerability details Impact In the the unlock function is callable by anyone, and there is no check if the caller is the correct insured address or not, this can be dangerous because it relies on parameters.getGracemsg.sender, which if the msg.sender didnt have any grace, it wil...

[WP-H20] Wrong implementation of withdrawRedundant() allows the Vault owner to drain all the funds

Handle WatchPug Vulnerability details Based on the context, withdrawRedundant intends to disallow the owner to withdraw more Vault tokens than the surplus amount. However, the current implementation is wrong, which allows the Vault owner to drain all the funds. function withdrawRedundantaddress...

Expired insurance status set incorrectly after unlock of funds

Handle ye0lde Vulnerability details Impact Expired insurance status set incorrectly after unlock of funds The insurance status is not set to false and the unlock function can be called over and over driving the lockedAmount to 0. The distorted lockedAmount will then cause liquidity and utilizatio...

[WP-H36] Admin of the index pool can withdrawCredit() after applyCover() to avoid taking loss for the compensation paid for a certain pool

Handle WatchPug Vulnerability details In the current implementation, when an incident is reported for a certain pool, the index pool can still withdrawCredit from the pool, which in the best interest of an index pool, the admin of the index pool is preferred to do so. This allows the index pool t...

applyCover() Does Not Enforce Index Market Lock

Handle leastwood Vulnerability details Impact The applyCover function is called by the insurance pool owner and intends to store data related to an insurance incident. Upon function execution, applyCover iterates over all available index markets and calls lock, denying all deposits and withdrawal...

the first depositor to an index can drain all users

Handle danb Vulnerability details if there is no liquidity in the pool, the first deposit determines the total liquidity, if the amount is too small the minted liquidity for the next liquidity providers will round down to zero. Impact An attacker can steal all money from liquidity providers. Proo...