Description

The deposit() and withdraw() functions of the Vault contract accept two arguments:

    function deposit(uint256 amountIn, address receiver)
        public
        override
        nonReentrant
        ensureFeesAreCollected
        returns (uint256 shares)
    {
        /// checks for only batcher deposit
        onlyBatcher();

        ...
    }

function withdraw(uint256 sharesIn, address receiver)
        public
        override
        nonReentrant
        ensureFeesAreCollected
        returns (uint256 amountOut)
    {
        /// checks for only batcher withdrawal
        onlyBatcher();
        ...
    }

Both of the functions call onlyBatcher() to check and enforce the validity of msg.sender:

function onlyBatcher() internal view {
    if (batcherOnlyDeposit) {
        require(msg.sender == batcher, "ONLY_BATCHER");
    }
}

Both of the functions perform no other checks of the validity of msg.sender.

By default (batcherOnlyDeposit = true), only the Batcher contract can deposit and withdraw funds on behalf of the receiver.

The governance can change batcherOnlyDeposit to false. When batcherOnlyDeposit = false, the deposit() and withdraw() functions perform no msg.sender validity checks whatsoever, allowing any third-party user to trigger deposits and withdrawals on behalf of any receiver.

Impact

A third party can trigger unwanted deposits and withdrawals on behalf of another user. This can lead to the users' confusion, lost profits, and even potentially to a loss of funds.

Recommendations

Consider adding if (!batcherOnlyDeposit) { require(msg.sender == receiver); } checks to the deposit() and withdraw() functions.

Remediation

The issue has been fixed in commit 32d30c8↗.