Mitigation of M-06: See comments

Mitigation of M-06: See comments Link to Issue: code-423n4/2023-03-asymmetry-findings770 Comments Sponsor decided not to mitigate the issue with the following comment: This is as expected I agree that the issue is TOO broad and some of the described scenarios don't make sense at all e.g. the...

Mitigation of M-12: mitigation error, see comments

MITIGATION IS NOT CONFIRMED MITIGATION IS NOT CONFIRMED Mitigation of M-12: mitigation error, see comments Link to Issue: code-423n4/2023-03-asymmetry-findings150 Comments While the proposed change correctly mitigates the issue, in the sense that it introduces a user controlled slippage for stake...

Mitigation Confirmed for NEW

Note: Issue has not actually been resolved but for some reason I can't get my issues to submit without "Mitigation confirmed no new vulnerabilities detected" checked so I am doing this as a work around Severity High Lines of code Impact Derivative will become broken and all funds lost even if the...

Mitigation of M-11: Issue not mitigated, mitigation error

MITIGATION IS NOT CONFIRMED MITIGATION IS NOT CONFIRMED Mitigation of M-11: Issue not mitigated, mitigation error Link to Issue: code-423n4/2023-03-asymmetry-findings152 Comments Even though the sponsor followed the warden's recommendation in issue M-11, I don't think the proposed change properly...

Mitigation Confirmed for Mitigation of H-06: Issue mitigated with error

Mitigated issue H-06: WstEth derivative assumes a 1=1 peg of stETH to ETH. The issue was that WstEth.withdraw and WstEth.ethPerDerivative assume a perfect peg between stETH and ETH, which may cause the slippage to be inaccurately evaluated. Mitigation review The issue has been mitigated by using...

StrategyBase.sharesToUnderlying() cannot be overridden to intended mutability

Lines of code Vulnerability details Impact An implementation of sharesToUnderlying, as inherited from StrategyBase.sol, cannot contrary to intentions make state modifications. This implies that StrategyBase.sol may become useless as a base contract to inherit from. Proof of Concept StrategyBase.s...

Context not implemented as per need of the code

Lines of code Vulnerability details Impact Logical and mathematical flaws might occur due to lack of the check . Proof of Concept As per comment in: //check if the withdrawal occured after mostRecentWithdrawalBlockNumber but no check is involved in the code which would cause mathematical flaws in...

Upgraded Q -> 3 from #549 [1683219263333]

Judge has assessed an item in Issue 549 as 3 risk. The relevant finding follows: L-02 Unsafe casting of uints Downcasting from uint256 in Solidity does not revert on overflow. This can easily result in undesired exploitation or bugs, since developers usually assume that overflows raise errors...

StrategyBase.underlyingToShares() cannot be overridden to intended mutability

Lines of code Vulnerability details Impact An implementation of underlyingToShares, as inherited from StrategyBase.sol, cannot contrary to intentions make state modifications. This implies that StrategyBase.sol may become useless as a base contract to inherit from. Proof of Concept StrategyBase.s...

Users can queue a withdrawal and potentially withdraw completely if PAUSED_EIGENPODS_VERIFY_OVERCOMMITTED = false

Lines of code Vulnerability details Impact Users can queue a withdrawal and potentially withdraw completely if PAUSEDEIGENPODSVERIFYOVERCOMMITTED = false Proof of Concept We need to look at two functions. The first one is function verifyOvercommittedStake uint40 validatorIndex,...

Loss of funds on deposit when totalShares > 0 && priorTokenBalance == 0

Lines of code Vulnerability details Impact Withdrawing one's shares may return far less tokens than one deposited. Proof of Concept Shares returned when depositing is calculated in StrategyBase.deposit as if totalShares == 0 newShares = amount; else uint256 priorTokenBalance = tokenBalance -...

State variables are initialized in an upgradeable contract + there is constructor

Lines of code Vulnerability details Impact Due to a requirement of the proxy-based upgradeability system, no constructors can be used in upgradeable contracts. State variables are initialized in an upgradeable contract Proof of Concept See -upgradeableavoid-initial-values-in-field-declarations...

Upgraded Q -> 3 from #853 [1683219256548]

Judge has assessed an item in Issue 853 as 3 risk. The relevant finding follows: LC-02 USE THE SAFECAST LIBRARY FOR CASTING VALUES TO AVOID OVERFLOW/UNDERFLOW File: virtualBaseTokenReserves += uint128netInputAmount - feeAmount - protocolFeeAmount; virtualNftReserves -= uint128weightSum; --- The...

Access Control Unauthorized access to restricted functions setWithdrawalDelayBlocks

Lines of code Vulnerability details Impact By exploiting the owner's role through social engineering, an attacker could theoretically gain indirect control over any functions that require owner authorization. Specifically, the ability to manipulate withdrawal delays and other critical security...

Zero address pauser assignment

Lines of code Vulnerability details Impact By allowing any address to be assigned as the pauser, the StrategyBase contract leaves itself vulnerable to losing critical functionality that controls token transfers in and out. Assigning a zero address would result in no valid pauser, preventing the...

Upgraded Q -> 3 from #316 [1683219136420]

Judge has assessed an item in Issue 316 as 3 risk. The relevant finding follows: L-04 USE THE OPENZEPPELIN SAFECAST LIBRARY FOR CRITICAL FUNCTIONS 2 --- The text was updated successfully, but these errors were encountered: All reactions...

Attacker can make his stake immune to verifyOvercommittedStake.

Lines of code Vulnerability details Impact verifyOvercommittedStake for an EigenPod will be DOSed, and Attacker's shares will never be reduced even when his stake on BeaconChain gets slashed, and someone attempts to record the overcommitment. Proof of Concept Here is the...

verifyWithdrawalCredentialsAndBalance does not verify that oracleBlockNumber is the latest block number.

Lines of code Vulnerability details Impact After participating in Ethereum Staking, you may receive shares based on your balance when it was 32 ETH, even though you may have suffered a certain level of slashing at 32 ETH. Until some conscientious watcher proves the slashing in the Consensus Layer...

Assuming a 1-1 peg of Liquid Staked Tokens like stETH and rETH to ETH is dangerous

Lines of code Vulnerability details Impact The price of ETH staking derivatives may not be pegged 1-1 to ETH which affect staking conditions. Proof of Concept To stake eth, a user calls depositBeaconChainETH. The amount parameter is passed into the addShares function. In addShares, the amount...

Slashing can be frontrunned

Lines of code Vulnerability details Proof of Concept When attempting to withdraw funds, the user calls queueWithdrawal first. queueWithdrawal checks that the caller is not frozen, then marks the withdrawal as pending. function queueWithdrawal uint256 calldata strategyIndexes, IStrategy calldata...

Upgraded Q -> 3 from #344 [1683218670048]

Judge has assessed an item in Issue 344 as 3 risk. The relevant finding follows: To ensure that there is no overflow when converting uint256 to uint128,and the totalNetInputAmount can be extracted so that it does not need to be calculated again later virtualBaseTokenReserves +=...

Skipping indices of malicious strategies does not work

Lines of code Vulnerability details Impact In src/contracts/core/StrategyManager.solL536 parameter indicesToSkip per documentation: """exists so that, e.g., if the slashed QueuedWithdrawal contains a malicious strategy in the strategies array which always reverts on calls to its 'withdraw'...

Upgraded Q -> 3 from #635 [1683219244120]

Judge has assessed an item in Issue 635 as 3 risk. The relevant finding follows: Unsafe downcasting PrivatePool.sol - 230, 231 --- The text was updated successfully, but these errors were encountered: All reactions...

Upgraded Q -> 3 from #13 [1683219131959]

Judge has assessed an item in Issue 13 as 3 risk. The relevant finding follows: L-6 Consider using OpenZeppelin’s SafeCast library to prevent unexpected errors 3 --- The text was updated successfully, but these errors were encountered: 👍 1 sathishpic22 reacted with thumbs up emoji All reactions 👍...

The values for strategyIndexes are not enforced

Lines of code Vulnerability details Proof of Concept strategyIndexes is used to indicate which strategies the caller will withdraw 100% of his shares, but it can contain any value when calling StrategyManager.queueWithdrawal and StrategyManager.slashShares. These two functions will reuse...

Gas stipend of 2300 in staticcall function causes contract failure

Lines of code Vulnerability details Impact The Merkle library's staticcall function contains a vulnerability that could lead to the failure of the entire operation if the invoked external contract exceeds the provided 2300 gas stipend. An attacker could leverage this vulnerability to launch a...

anyone can call initialize() functions which can cause lost of funds and contract ownership

Lines of code Vulnerability details Impact Most of the smart contracts have an initialize function that anyone can call as initialize function visibility is either external or public. This could lead to a race condition when the contract is deployed. At that moment a hacker or attacker could call...

Integer Overflow Large numbers of strategies/deposits could overflow arrays stakerStrategyList

Lines of code Vulnerability details Impact Overflows of the staker StrategyList and StrategyShares arrays could allow an attacker to manipulate a staker's data by reusing keys. Once the arrays have wrapped around, depositing into a large number of strategies or requesting withdrawals for the same...

[H-02] Owner cannot freeze and thus cannot slash a queued withdraw that has the delegatedAddress being the 0 address.

Lines of code Vulnerability details canSlash checks to see if the block number is less than whitelistedContractDetailstoBeSlashedslashingContract, which will be 0 if a user has not delegated an address. This will revert freezeOperatorand not allow an owner/watcher to freeze the address, and thus...

The length of proofs.slotProof is not checked in the verifyWithdrawalProofs function, allowing a malicious EigenPod Owner to be issued only shares via StrategyManager and withdraw all their money

Lines of code Vulnerability details Impact Below is a portion of the verifyAndProcessWithdrawal function. // Verifying the withdrawal as well as the slot BeaconChainProofs.verifyWithdrawalProofsbeaconStateRoot, withdrawalProofs, withdrawalFields; // Verifying the validator fields, specifically th...

deposit will cause erros if totalShares is 1e9-1 or less

Lines of code Vulnerability details Impact user funds will be stuck Proof of Concept User will not be able to withdraw for tokens that are less then 1e9-1 like usdt This is a warning in the code WARNING: In order to mitigate against inflation/donation attacks in the context of ERC4626, this...

The value of MAX_WITHDRAWAL_DELAY_BLOCKS is constant which shouldn't be

Lines of code Vulnerability details Impact The value of MAXWITHDRAWALDELAYBLOCKS is constant which shouldn't be. It is stated that at the moment MAXWITHDRAWALDELAYBLOCKS is 50400 and it is adjustable, but it isn't in this case as it is constant and once deployed it can't be change Proof of Concep...

Upgraded Q -> 3 from #903 [1683219188466]

Judge has assessed an item in Issue 903 as 3 risk. The relevant finding follows: L-2. Use SafeCast Library Description Downcasting from uint256/int256 in Solidity does not revert on overflow. This can easily result in undesired exploitation or bugs, since developers usually assume that overflows...

Use of block.timestamp

Lines of code Vulnerability details Impact Block timestamps have a purpose for a variety of applications, such as entropy for random numbers see the Entropy Illusion for further details, locking funds for periods of time, and various state-changing conditional statements that are time-dependent...

A Malicious validator can frontrun 32ETH deposit

Lines of code Vulnerability details Impact Frontrunning by malicious validator actor to change the withdrawal credentials and gain the withdrawal ETH value. Proof of Concept A malicious validator can frontrun stake transaction with the same pubkey and deposit 1 ether for different withdrawal...

_deployPod() is using hardcoded salt value which can cause issues

Lines of code Vulnerability details Impact deployPod is deploying pod but there is a hardcoded salt value of 0 which can cause issue Proof of Concept There is a risk of address collisions if two different users call the deployPod function with the same input parameters at the same time. This coul...

Upgraded Q -> 3 from #359 [1683219147605]

Judge has assessed an item in Issue 359 as 3 risk. The relevant finding follows: L-18 UNSAFE CAST Description: Keep in mind that the version of solidity used, despite being greater than 0.8, does not prevent integer overflows during casting, it only does so in mathematical operations. It is...

Overflow Excessive claims could overwhelm storage _userWithdrawals mapping

Lines of code Vulnerability details Impact By submitting an excessive number of claims through a compromised user, an attacker could theoretically overwhelm the storage used for mapping users to their delayed withdrawals. If critical data is overwritten, the contract would be rendered unusable...

Strategy owner can steal staker funds.

Lines of code Vulnerability details Impact The functions StrategyManager.depositIntoStrategy and StrategyManager.depositIntoStrategyWithSignature doesn't check if the msg.sender != strategy. Hence, a strategy owner can deposit into his own strategy and specify the staker to his own EOA account, a...

BEACON CHAIN VALIDATOR COULD SELF RESCUE WHEN OPERATOR IS FROZEN

Lines of code Vulnerability details Impact The modifier onlyNotFrozen is intuitive such that the staker will be frozen when the delegated operator is frozen. However, not utilizing it in recordOvercommittedBeaconChainETH and undelegate could allow the Beacon Chain validator to undelegate from the...

Temporary blocking withdrawals because of slashQueuedWithdrawal function incorrectness

Lines of code Vulnerability details Temporary blocking withdrawals because of slashQueuedWithdrawal function incorrectness Impact The incorrectness of the slashQueuedWithdrawalcan block withdraw operations till queuedWithdrawal argument will be changed to exclude strategies with PAUSEDWITHDRAWALS...

Upgraded Q -> 3 from #826 [1683219178977]

Judge has assessed an item in Issue 826 as 3 risk. The relevant finding follows: 12. Consider using OpenZeppelin’s SafeCast library to prevent unexpected overflows when casting from uint256. PrivatePool.solL2 File: src/PrivatePool.sol 231: virtualNftReserves -= uint128weightSum;...

Upgraded Q -> 3 from #421 [1683219158450]

Judge has assessed an item in Issue 421 as 3 risk. The relevant finding follows: 1. Unchecked Cast May Overflow As of Solidity 0.8 overflows are handled automatically; however, not for casting. For example uint324294967300 will result in 4 without reversion. Consider using OpenZepplin's SafeCast...

Medium Access Control Unauthorized access to restricted functions

Lines of code Vulnerability details Impact compromising the owner or whitelister roles could allow an attacker to manipulate critical whitelist and configuration logic, potentially damaging the use of the protocol. By taking control of privileged access, the attacker could enable rapid withdrawal...

Users can avoid getting their queuedWithdrawal slashed because of the wrong implementation.

Lines of code Vulnerability details Impact Users can avoid getting their queuedWithdrawal slashed because of the wrong implementation. Proof of Concept Let's take a look at the following code snippet from StrategyManagerslashQueuedWithdrawal. // keeps track of the index in the indicesToSkip array...

High Reentrancy Withdrawals can be frontrun

Lines of code Vulnerability details Impact A reentrancy attack on the withdrawal functions could allow an attacker to drain the contract of all funds by repeatedly calling the functions faster than transactions can complete. By calling completeQueuedWithdrawal and withdrawBeaconChainETH multiple...

Attacker can operate as a staker/operator on eigenLayer without risking any funds

Lines of code Vulnerability details Impact Attacker would get shares in StrategyManager without staking any real funds. This would allow him to earn rewards or act maliciously without fear of getting slashed. Proof of Concept Here is the verifyWithdrawalCredentialsAndBalance function: L175-L226...

Upgraded Q -> 3 from #326 [1683219203160]

Judge has assessed an item in Issue 326 as 3 risk. The relevant finding follows: L-05 Integer overflow by unsafe casting Description Keep in mind that the version of solidity used, despite being greater than 0.8, does not prevent integer overflows during casting, it only does so in mathematical...

Zero address Pauser assignment

Lines of code Vulnerability details Impact By exploiting the unpauser role's access to call setPauser with any address input, an attacker could permanently disable a core functionality pausing/unpausing the token by assigning a zero address as the pauser. No pauser would mean no ability to freeze...

Slot and block number proofs not required for verification of withdrawal (multiple withdrawals possible)

Lines of code Vulnerability details Impact Since this is a vulnerability which involves multiple in-scope contracts and leads to more than one impact, let's start with a bug desciption from bottom to top. Library Merkle The methods verifyInclusionSha256proof, root, leaf, index and...