Depositor can send a positive token amount to vault but mint 0 shares when calling ReaperVaultV2._deposit function

[code423n4]

Lines of code

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperVaultV2.sol#L59-L73
https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperVaultERC4626.sol#L110-L112
https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperVaultV2.sol#L302-L304
https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperVaultV2.sol#L313-L315
https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperVaultV2.sol#L319-L338

Vulnerability details

Impact

When an EOA or contract with the DEPOSITOR role calls the following ReaperVaultERC4626.deposit, ReaperVaultV2.depositAll, and ReaperVaultV2.deposit functions, the ReaperVaultV2._deposit function is called.

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperVaultV2.sol#L59-L73

    /**
     * Reaper Roles in increasing order of privilege.
     * {DEPOSITOR} - Role conferred to EOAs/contracts that are allowed to deposit in the vault.
     ...
     */
    bytes32 public constant DEPOSITOR = keccak256("DEPOSITOR");

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

    function deposit(uint256 assets, address receiver) external override returns (uint256 shares) {
        shares = _deposit(assets, receiver);
    }

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperVaultV2.sol#L302-L304

    function depositAll() external {
        _deposit(token.balanceOf(msg.sender), msg.sender);
    }

https://github.com/code-423n4/2023-02-ethos/blob/main/Ethos-Vault/contracts/ReaperVaultV2.sol#L313-L315

    function deposit(uint256 _amount) external {
        _deposit(_amount, msg.sender);
    }

Calling the ReaperVaultV2._deposit function below then executes shares = (_amount * totalSupply()) / freeFunds. Because totalSupply() can be less than freeFunds, such as after profits of the vault's strategies are harvested, it is possible that _amount * totalSupply() is less than freeFunds. In this situation, the depositor, who calls the ReaperVaultV2._deposit function with such _amount input value, would send _amount tokens to the vault but mint 0 shares in return. As a result, this depositor loses the _amount tokens that are sent.

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

    function _deposit(uint256 _amount, address _receiver) internal nonReentrant returns (uint256 shares) {
        _atLeastRole(DEPOSITOR);
        require(!emergencyShutdown, "Cannot deposit during emergency shutdown");
        require(_amount != 0, "Invalid amount");
        uint256 pool = balance();
        require(pool + _amount <= tvlCap, "Vault is full");

        uint256 freeFunds = _freeFunds();
        uint256 balBefore = token.balanceOf(address(this));
        token.safeTransferFrom(msg.sender, address(this), _amount);
        uint256 balAfter = token.balanceOf(address(this));
        _amount = balAfter - balBefore;
        if (totalSupply() == 0) {
            shares = _amount;
        } else {
            shares = (_amount * totalSupply()) / freeFunds; // use "freeFunds" instead of "pool"
        }
        _mint(_receiver, shares);
        emit Deposit(msg.sender, _receiver, _amount, shares);
    }

Proof of Concept

The following steps can occur for the described scenario.

1. At this moment, the vault has 1e6 total shares and 1e9 tokens as the free funds.
2. The depositor owns 0.9e3 of such token and calls the ReaperVaultV2.depositAll function.
3. Because (_amount * totalSupply()) / freeFunds = (0.9e3 * 1e6) / 1e9 rounds down to 0, the depositor mints 0 shares after the ReaperVaultV2._deposit function is called.
4. Since the depositor sends 0.9e3 tokens to the vault but mints 0 shares, it loses the 0.9e3 tokens.

Tools Used

VSCode

Recommended Mitigation Steps

The ReaperVaultV2._deposit function can be updated to ensure that calling it can revert when the calculated shares to be minted is 0.

[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)