Liquid Staking

Summary

Early depositors will unnecessarily lose shares

Vulnerability Details

Upon shares minting, the amount is calculated using these functions:

We can see that the protocol introduces the DEAD_SHARES = 10 ** 3 variable in order to protect the pool from inflation attack, which also deducts from the first depositor's amount. However if there is no such attack, the user will not get 1:1 minting ratio as it's stated in the documentation and later on the user can't recover them. But if we consider that there is a donation attack the current logic will still prevent such scenario without this line: _amount -= DEAD_SHARES, thus this results in honest early depositor unnecessarily to lose liquid staking tokens. These cases are explained in more detail below:

1. Case where there is no attack and the protocol functions normal

   • result: discrepancy between the documentation and the code, as the protocol promises 1:1 minting ratio, but early depositor gets less

   • further there is no logic for the user to recover these shares

2. Case where there is a donation attack

   • if we consider that this line is skipped: _amount -= DEAD_SHARES, the totalShares will increase in anyway to 10 ** 3 = 1000, so if a user deposits 1 LINK, which will cost him around 10.5 USD, amount * totalShares will be 1000e18 so it will require the attacker to donate 1001e18 which will make his costs ~10,500 USD, which makes the scenario nearly impossible

   • however even with this defence logic there is still a possible inflation attack scenario, only if user stakes 0.01 LINK, hence attacker will need to spend only ~105 USD to be able to truncate the shares of honest user to 0, but i assume this would be rare case as 1 LINK token costs only ~10 USD

Tools Used

Manual Review

Recommendations

Currently the protocol unessesarily mints the early depositor less shares, which causes discrepancy between the docs and the code, because users are not aware of this, but the protocol promises 1:1 ratio mints and causes loses which can't be acquired later.

My recommendation would be to increase the value of DEAD_SHARES to a greater number, hence this will nullify all the scenarios for inflation attacks, even if some users deposit small amounts.

Subsequently will allow this line _amount -= DEAD_SHARES to be removed also, which will not cause early depositor loss of shares.