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

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

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

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

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

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

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

Potential Reentrancy at multiple places

Handle 0v3rf10w Vulnerability details Impact Potential Reentrancy at multiple places Proof of Concept L2Migrator.finalizeMigrateDelegatorIMigrator.MigrateDelegatorParams contracts/L2/gateway/L2Migrator.sol130-188: L1Escrow.approveaddress,address,uint256 contracts/L1/escrow/L1Escrow.sol21-28...

L2Migrator calls wrong function on bondingManager

Handle harleythedog Vulnerability details Impact In L2Migrator, the function bondFor calls the function "bondForWithHint" on the bondingManager. This function does not exist anywhere in the protocol: the correct function name is simply "bondWithHint". This is a run-time issue the contracts will...

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-M4] Unable to use L2GatewayRouter to withdraw LPT from L2 to L1, as L2LPTGateway does not implement L2GatewayRouter expected method

Handle WatchPug Vulnerability details Per the document: The following occurs when LPT is withdrawn from L2 to L1: The user initiates a withdrawal for X LPT. This can be done in two ways: a. Call outboundTransfer on L2GatewayRouter which will call outboundTransfer on L2LPTGateway b. Call...

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

Fund loss when insufficient call value to cover fee

Handle gzeon Vulnerability details Impact Fund can be lost if the L1 call value provided is insufficient to cover maxSubmissionCost, or stuck if insufficient to cover maxSubmissionCost + maxGas gasPriceBid. Proof of Concept outboundTransfer in L1LPTGateway does not check if the call value is...

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

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

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

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

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

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

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

Unbounded iteration over all pools

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

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

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

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

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

If Vault contains tokens that charge a fee on transfer the internally kept balance will be wrong

Handle Ruhum Vulnerability details Impact Some tokens charge a fee for each transfer. USDT, for example, has the possibility of enabling fees at any time. If the vault is used for that kind of token, the internal balance keeping will be wrong. The vault will think that it owns more tokens than it...

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

the first depositor to a pool 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...

[WP-H29] Vault#setController() owner of the Vault contracts can drain funds from the Vault

Handle WatchPug Vulnerability details function setControlleraddress controller public override onlyOwner requirecontroller != address0, "ERRORZEROADDRESS"; if addresscontroller != address0 controller.migrateaddresscontroller; controller = IControllercontroller; else controller =...

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

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

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

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

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

System Debt Is Not Handled When Insurance Pools Become Insolvent

Handle leastwood Vulnerability details Impact If an incident has occurred where an insurance policy is to be redeemed. The market is put into the MarketStatus.Payingout mode where the insurance.insured account is allowed to redeem their cover and receive a payout amount. Upon paying out the...

[WP-M17] Vault.sol Tokens with fee on transfer are not supported

Handle WatchPug Vulnerability details There are ERC20 tokens that charge fee for every transfer / transferFrom. Vault.soladdValue assumes that the received amount is the same as the transfer amount, and uses it to calculate attributions, balance amounts, etc. While the actual transferred amount c...

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 from a long list of storage can impact other people paying more gas than it used to

Handle Fitraldys Vulnerability details Impact In the it will loop through an entire indexList array this doesnt immedietely impact other user, however when there is many user call allocateCredit this function will add another indexlist if the user didnt exist in the first place. Lets say the time...

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

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

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

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-H32] PoolTemplate.sol Attacker can call Factory#createMarket() and transfer funds from another user's wallet to the pool

Handle WatchPug Vulnerability details function initialize string calldata metaData, uint256 calldata conditions, address calldata references external override require initialized == false && bytesmetaData.length 0 && references0 != address0 && references1 != address0 && references2 != address0 &&...