Bandit - First Depositor Can Inflate Share Value to Cause Extreme Rounding Loss And Prohibitively Expensive Mints

[sherlock-admin]

Bandit

high

First Depositor Can Inflate Share Value to Cause Extreme Rounding Loss And Prohibitively Expensive Mints

Summary

The first depositor can mint shares then inflate the value of the shares by depositing tokens and then sync the balance to the internal accounting of the system. This will lead to severe rounding losses such as in the sellShares function meaning that users get significantly less than they should, and is especially punishing for early withdrawers.

Another way it can be used is to make the first depositor mint 0 shares.

Vulnerability Detail

Note: the well known ERC4626 inflation frontrunning attack sequence where attacker mints 1 share and victimn gets 0 shares does not work because victims transaction will revert if they don't mint more than 1000 shares. This is a different variation on that concept.

If the first deposit using buyShares, the logic in the if (totalSupply == 0) is executed. Consider the situation when the shares = quoteBalance < DecimalMath.mulFloor(baseBalance, _I_) is false. Then, the shares will be set to the baseBalance.

This means that the value of a share would have the same decimal denomination as a the baseToken. Since the value of 1 wei of baseToken is small, the rounding of both shares and baseToken will be insignificant in future calculations.

Note that the minimum amount of shares minted is 1000, but this number can actually by reduced/bypassed by first depositing with buyShares and then withdrawing with sellShares.

Lets say that the attacker mints 1000 shares and then withdraws such that there is only 1 share left. Then, they can deposit 1e20 of tokens to the GSPFunding contract and then call sync to make the donated tokens apply to the internal accounting. Thus the 1 share is worth 1e20 base tokens and the corresponding quote tokens. Lets say the base token is DAI, which has 18 decimals and each token after decimal scaling is $1. 1e20 DAI = $100. They also donate the matching amount of quote token. This would mean that there is up to $200 USD of rounding error (1 share is $100 DAI and $100 worth of quote token) per withdrawal. Note that in this scenario the cost of the attacker's donation was $200.

The rounding loss comes in these lines:

baseAmount = baseBalance * shareAmount / totalShares;

quoteAmount = quoteBalance * shareAmount / totalShares;

This also means that with $200 per share the minimum mint (which is 1000 shares) becomes $200,000 which is far too much for most users do deposit.

The alternate way to use a first-deposit donation attack is to frontrun the first depositor. This is the sequence of steps:

Impact

First depositor can manipulate the share price to have inflated value rather than matching the base/quote token values. This can lead to extreme rounding errors when sellShares is called leading to loss of funds for users.

The profit incentive for the attacker is that the lost lost withdrawals end up distributed to existing stakers as the share reduction is not matched with the corresponding quote and base token reduction. Since the attacker is aware of the share inflation they caused themselves, they can tailor their share withdrawal amounts to never have any rounding loss.

Code Snippet

https://github.com/sherlock-audit/2023-12-dodo-gsp/blob/main/dodo-gassaving-pool/contracts/GasSavingPool/impl/GSPFunding.sol#L56-L65

Tool used

Manual Review

Recommendation

There could be a virtual share and asset offset as implemented in a fix to a similar attack on ERC4626 vaults: https://blog.openzeppelin.com/a-novel-defense-against-erc4626-inflation-attacks

Duplicate of #55