code-423n4 / 2023-04-eigenlayer-findings

1 stars 1 forks source link

Depositor get's extra shares to keep #366

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/core/StrategyManager.sol#L696-L697

Vulnerability details

Impact

The number of shares of depositor should have reduced after this step but as code is not properly implemented depositor get's to keep free shares.

Proof of Concept

In the _removeShares function of Strategy Manager,after the calculation of userShares it should have been subtracted from the shares of depositor's existing shares for the particular strategy and the same is said in the comment of the function: // subtract the shares from the depositor's existing shares for this strategy but subtraction part has been forgotten by the developer,which might lead to serious logic flaws and calculation problems in terms of shares distributed.

Tools Used

Manual Audit

Recommended Mitigation Steps

Change the code in line: https://github.com/code-423n4/2023-04-eigenlayer/blob/main/src/contracts/core/StrategyManager.sol#L697

to

stakerStrategyShares[depositor][strategy] -= userShares;

Assessed type

Math

0xSorryNotSorry commented 1 year ago

The shares are decreased in below lines and the mapping updated accordingly.

        unchecked {
            userShares = userShares - shareAmount;
        }

Invalid assumption.

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as low quality report

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Invalid