code-423n4 / 2023-03-asymmetry-findings

14 stars 12 forks source link

First staker can break minting of shares #212

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L98 https://github.com/code-423n4/2023-03-asymmetry/blob/main/contracts/SafEth/SafEth.sol#L99

Vulnerability details

The attack vector and impact is similar to Trail of Bits Yearn Finance Audit - 3. Division rounding may affect issuance of shares, where users may not receive any shares when staking, if the total asset amount has been manipulated through a large “donation”.

Impact

The first staker can make it so that a few units of SafeEth are worth 1Eth of underlying value, effectively making most subsequent stakings mint 0 units of SafeEth to victims.

This would not only make stakers sometimes completely use their funds (because they would be minted 0 SafEth units), but attacker could also withdraw the underlying value added by the victims, since the total number of SafEth shares didn't change and attacker holds all of them.

Proof of Concept

Recommended Mitigation Steps

If Assymetry Finance notices this attack happening, they can immediately pause staking, blocking users from becoming victims, and also pause unstaking, blocking the attacker from unstaking their shares and getting back their 1 Reth investment.

An easy way to mitigate the vulnerability would be to require(mintAmount != 0, "Can not mint 0 shares").

Another way would be taking Uniswap example: Uniswap V2 solved this problem by sending the first 1000 LP tokens to the zero address. The same can be done in this case, send the first 1000 SafEth units to the zero address to enable share dilution.

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as duplicate of #715

c4-judge commented 1 year ago

Picodes marked the issue as satisfactory

c4-judge commented 1 year ago

Picodes changed the severity to 3 (High Risk)