Liquid Staking

Summary

"In Solidity, division rounds towards zero". However, this doesn't always favor the protocol. StakingPool.sol is an ERC4626-like contract and rounding directions should be favoring the protocol, because dust amounts eventually pile up to a considerable amount.

Vulnerability Details

OpenZeppelin's ERC4626 withdraw function which has an input of assets which converts to shares, rounds up during the calculation. This is not the case here, all conversions round down:

For example, let's say Bob has 1e18 stLink (assets) and 1e18 shares, while at the StakingPool.sol the totalShares = 2e18 and totalStaked = 2e18 + 1 because let's say someone has donated or there are 1 wei from rewards. When Bob want's to withdraw half of his assets, so 0.5e18, the getSharesByStake function will get called to determine how many of his shares will get burnt:

0.5e18 * 2e18 / (2e18 + 1) = 499999999999999999 while rounding down.

While Bob is withdrawing exactly half of his assets, he burns less than half of his shares, benefiting from the division rounding. The above example is one in the simpliest form to demonstrate the problem. Small rounding errors not favoring the protocol will happen in every withdrawal.

The protocol should NEVER be losing on these transactions, even the lowest amount of dust, because it slowly accumulates.

Impact

The protocol loses dust amounts on every withdrawal, which in the long-run will always accumulate to a considerable amount.

Tools Used

Manual review

Recommendations

Like the OpenZeppelin's vault, import the Math library which allows to choose the direction of the rounding. Always round up during the withdrawals when converting from assets to shares.