[Security] CEI Violation in removeStrategy() - Reentrancy Allows Unlimited OUSD Minting

[laolaoqi]

Summary

Severity: Medium-High
PoC: Available
Impact: Unlimited OUSD token minting without backing, supply inflation attack

Description

In VaultAdmin.sol, the removeStrategy() function violates the Checks-Effects-Interactions (CEI) pattern. The external call strategy.withdrawAll() is executed BEFORE the strategy state is updated to isSupported = false.

function removeStrategy(address _addr) external onlyGovernor {
    require(strategies[_addr].isSupported, "Not supported");
    IStrategy(_addr).withdrawAll();       // INTERACTION - BEFORE state update
    strategies[_addr].isSupported = false; // EFFECT - TOO LATE
}

If the strategy is mint-whitelisted, its withdrawAll() can call back into the vault via mintForStrategy() to mint OUSD tokens while still being whitelisted. This allows:

1. Attacker-controlled strategy calls withdrawAll()
2. During withdrawal, strategy re-enters vault to mint OUSD
3. Since strategy is still whitelisted, mint succeeds
4. Attacker drains underlying assets with unbacked OUSD

Full PoC

Complete Foundry PoC available upon request.

Fix

function removeStrategy(address _addr) external onlyGovernor {
    require(strategies[_addr].isSupported, "Not supported");
    strategies[_addr].isSupported = false;  // Move BEFORE withdrawAll
    IStrategy(_addr).withdrawAll();
}

Reported by: laolaoqi