Description

The ConcreteMultiStrategyVault contract inherits from OpenZeppelin's ERC-20 implementation, which uses type(uint256).max as a sentinel value to represent infinite allowance. The standard internal _spendAllowance function preserves this infinite approval by not decreasing the allowance when it is set to type(uint256).max.

function _spendAllowance(address owner, address spender, uint256 value) internal virtual {
    uint256 currentAllowance = allowance(owner, spender);
    if (currentAllowance != type(uint256).max) {
        if (currentAllowance < value) {
            revert ERC20InsufficientAllowance(spender, currentAllowance, value);
        }
        unchecked {
            _approve(owner, spender, currentAllowance - value, false);
        }
    }
}

However, the _withdraw function in ConcreteMultiStrategyVault does not use _spendAllowance. Instead, it manually calculates and reduces the allowance when an approved spender (msg.sender != owner_) executes a withdrawal. This approach does not check for the infinite allowance sentinel value.

function _withdraw(uint256 assets_, address receiver_, address owner_, uint256 shares, uint256 feeShares) private {
    // [...]
    if (msg.sender != owner_) {
        _approve(owner_, msg.sender, allowance(owner_, msg.sender) - shares);
    }
    // [...]
}

Impact

When token owners grant spenders infinite allowance, the allowance decreases if spenders withdraw assets from the vault on their behalf. This behavior is inconsistent with the transferFrom function, where infinite allowances remain unchanged after transfers.

Recommendations

Consider replacing the _approve call with the _spendAllowance internal function, which checks for infinite allowance before reducing the allowance.

function _withdraw(uint256 assets_, address receiver_, address owner_, uint256 shares, uint256 feeShares) private {
    // [...]
    if (msg.sender != owner_) {
-        _approve(owner_, msg.sender, allowance(owner_, msg.sender) - shares);
+        _spendAllowance(owner_, msg.sender, shares);
    }
    // [...]
}

Remediation

This issue has been acknowledged by Blueprint Finance, and a fix was implemented in commit 44114dac↗.