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

1 stars 1 forks source link

`deposit()` function is calculation wrong value of `newShares' #427

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/Layr-Labs/eigenlayer-contracts/blob/dbeb85bcd0476e06b8feebf07e33f8a53d54c029/src/contracts/strategies/StrategyBase.sol#L99

Vulnerability details

Impact

deposit() function is calculating wrong value of newShare which will cause hyper inflation of newShare

Proof of Concept

If priorTokenBalance is 1, then newShares = (amount * totalShares) / priorTokenBalance will amount*totalShares/1 and the newShare will be amount*totalShares

Here amount will be the input and totalShares will be the number of extant shares in thie Strategy so newshares could be more then intended.

Tools Used

Manual Review

Recommended Mitigation Steps

maybe you could use if condition to check if priorTokenBalance is 1

Assessed type

Math

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as duplicate of #193

c4-judge commented 1 year ago

GalloDaSballo marked the issue as partial-50

GalloDaSballo commented 1 year ago

Poor description and no coded POC, but does explain the vulnerability

c4-judge commented 1 year ago

GalloDaSballo changed the severity to 2 (Med Risk)

c4-judge commented 1 year ago

GalloDaSballo marked the issue as duplicate of #343

c4-judge commented 1 year ago

GalloDaSballo marked the issue as partial-25

d3e4 commented 1 year ago

There is no issue here. If priorTokenBalance is 1, then amount*totalShares is the correct amount of shares to mint in order to withdraw the same amount as was deposited. Withdrawing gives _tokenBalance() * amountShares / priorTotalShares which in our case if we withdraw our shares again is (1 + amount) * (amount*totalShares) / (totalShares + amount*totalShares) = amount.

c4-judge commented 1 year ago

GalloDaSballo changed the severity to QA (Quality Assurance)