Summary
Type: Layer 2 & Rollups
Timeline: From 2026-02-11 → To 2026-02-13
Languages: Solidity
Findings
Total issues: 18 (15 resolved)
Critical: 0 (0 resolved) · High: 0 (0 resolved) · Medium: 1 (1 resolved) · Low: 3 (3 resolved)
Notes & Additional Information
14 notes raised (11 resolved)
OpenZeppelin performed an audit of the Cubewire/vault-contracts repository at commit fd22a03.
In scope were the following files:
contracts
└── Vault
├── ERC4626FeesUpgradeable.sol
├── GatedTrustedForwarder.sol
├── VaultUpgradeable.sol
├── Whitelist.sol
├── interfaces
│ ├── IVaultUpgradeable.sol
│ └── IWhitelist.sol
├── overrides
│ └── ERC2771ForwarderKeyedNonce.sol
└── proxy
└── UUPSProxy.sol
The Vault system is an upgradeable, ERC-4626-inspired tokenized vault designed for regulated fund management. It enables controlled asset deposits and share token issuance with oracle-based pricing, configurable fees, whitelist access control, ERC20Permit for gasless approvals, gasless transaction support via ERC-2771 meta-transactions, and multicall functionality.
The system consists of three primary contracts:
VaultUpgradeable: The core vault contract that manages deposits, redemptions, share token minting/burning, and integrates all system features including fee collection, oracle pricing, and access control.Whitelist: An external, immutable, and independently deployed contract that maintains an enumerable set of addresses permitted to hold, transfer, or receive vault shares. The vault enforces whitelist checks on all share movements.GatedTrustedForwarder: An ERC-2771 meta-transaction forwarder with role-restricted relay execution, enabling gasless operations for whitelisted users.While the vault implements much of the ERC-4626 interface, it is not EIP-4626 compliant. Notable deviations include:
totalAssets/totalSupply ratio. The price is always expressed in 18 decimals regardless of the underlying asset's decimals.mint and withdraw functions are disabled and always revert. Users must use deposit (specifying assets) and redeem (specifying shares) instead.deposit and redeem functions have been overloaded with a slippage parameter to prevent unfavorable execution due to oracle price changes. The standard ERC-4626 deposit and redeem functions remain accessible for compatibility but lack slippage protection; users and integrators should use the slippage-protected variants.previewDeposit, previewRedeem, _convertToShares, and _convertToAssets functions revert when the price is zero or stale, rather than returning a value.totalAssets: Returns the oracle-implied total value of all outstanding shares (totalSupply * price / 1e18) rather than the vault's actual token balance, since the TRANSFER_MANAGER_ROLE can move assets out of the vault for fund operations. Unlike the conversion functions, totalAssets does not revert when the price is zero or stale — it returns 0 or the stale NAV respectively.PriceIsStale if the latest oracle price update exceeds the configured staleLimit.The vault operates exclusively in async mode (the isAsync flag is hardcoded to true). In this mode, all deposit and redemption operations must be submitted through the trusted forwarder — direct calls to these functions revert with AsyncForwarderOnly. Share token transfers between whitelisted addresses remain able to be operated directly (without meta-transactions), and administrative functions can operate independently of the async setting. This design enables:
ForwardRequestData messages off-chain, and authorized relayers submit these on their behalf.The GatedTrustedForwarder extends OpenZeppelin's ERC-2771 forwarder pattern with keyed nonces and role-gated execution. NoncesKeyed is used instead of sequential nonces, enabling users to submit multiple transactions in parallel without nonce conflicts by choosing different key values. Additionally, only addresses with RELAYER_ROLE can execute forwarded transactions, preventing unauthorized parties from submitting meta-transactions.
The forwarder validates signatures using ECDSA recovery, which restricts meta-transaction signers to EOA wallets; contract wallets such as Safe multisigs or ERC-4337 accounts cannot initiate deposits or redemptions through the forwarder without explicit ERC-1271 signature support.
The vault charges configurable entry and exit fees, specified in basis points. Fees are transferred to a configurable feeRecipient address. If the fee recipient is set to the vault itself, fees accumulate in the vault balance and can be extracted via transferAssets by the TRANSFER_MANAGER_ROLE. A maxFeeBasisPoints governance parameter limits how high entry and exit fees can be set (up to a maximum of 10000 BPS), providing protection against excessive fees.
When a whitelist contract is configured (non-zero address), the vault enforces whitelist checks on all token movements. All share transfers require both sender and receiver to be whitelisted, share approvals require both owner and spender to be whitelisted, minting requires the receiver to be whitelisted, and burning requires the share owner to be whitelisted (bypassed during compliance recovery via recoverShares). The whitelist can be shared across multiple vaults, replaced by the vault administrator, or disabled by setting it to zero.
The Vault system operates under a managed trust model appropriate for regulated financial products. The security architecture relies on role-based access control, external contract dependencies, and trust assumptions regarding off-chain infrastructure.
The VaultUpgradeable contract uses the UUPS proxy pattern, allowing future upgrades while preserving state. The Whitelist and GatedTrustedForwarder contracts are not upgradeable and are deployed as standard contracts.
All three contracts use OpenZeppelin's AccessControl for role management. The DEFAULT_ADMIN_ROLE in each contract can grant and revoke any role, including itself. This role represents a single point of failure — if renounced without reassignment, the contract becomes orphaned with no ability to modify roles or (for the vault) authorize upgrades.
The vault stores a reference to an external whitelist contract and trusts it to correctly report address membership. The whitelist contract address can be changed by FUND_ADMIN_ROLE to any arbitrary address, meaning a malicious or compromised admin could point the vault to a contract that returns true for attacker-controlled addresses. The whitelist contract itself is immutable once deployed, but its entries are mutable by addresses holding WHITELIST_ROLE. Whitelist changes take effect immediately, affecting transfer and approval capabilities instantly, and are not subject to the vault's pause state since the whitelist is an independent contract. Additionally, since share transfers are not restricted to the async forwarder path, users may monitor the public mempool for pending whitelistRemove transactions and front-run removal by transferring shares to another whitelisted address. Finally, the whitelist design prevents permissionless DeFi integrations unless external protocols are explicitly approved.
In async mode, the vault depends entirely on the trusted forwarder for deposit and redemption operations. A malicious, compromised, or vulnerable forwarder could impersonate any privileged role by appending incorrect context to calls. The forwarder address can be changed by FUND_ADMIN_ROLE, but cannot be set to zero in async mode. Relayers control transaction timing and ordering, can observe signed transactions, and could potentially delay or refuse service. Since relayers control execution timing, signed requests targeting the standard ERC-4626 entrypoints (which lack slippage protection) could be delayed and executed after adverse oracle price updates. Users cannot cancel or invalidate nonces after signing, even if transactions remain unsubmitted by relayers.
The keyed nonce system in ERC2771ForwarderKeyedNonce allows parallel transaction submission but requires careful relayer configuration. The forwarder does not enforce on-chain restrictions on target contracts, function selectors, or nonce key values. Relayers should implement appropriate off-chain protections including but not limited to: validating the target is the vault contract, allowlisting permitted function selectors, restricting permitted nonce key values to prevent unbounded storage growth, enforcing gas caps, implementing rate limits, and checking whitelist status before submission.
The vault implements an RWA (Real World Asset) pricing model where the oracle-set price reflects the real-world NAV of underlying assets, independent of on-chain token balances and share supply. This is fundamentally different from typical ERC-4626 vaults where pricing derives from totalAssets/totalSupply. Consequently, operations such as recoverShares, transferAssets, and fee accumulation do not directly affect the oracle price. The on-chain token balance can diverge significantly from the oracle-implied NAV, and off-chain accounting should accurately reconcile these operations. If the vault's token balance falls below the amount required for a redemption, redemptions will revert until sufficient liquidity is restored.
The oracle is fully trusted to provide accurate pricing, and the setPrice function performs no validation on the price value. Setting the price to zero halts all vault operations, while extreme price values could enable share inflation or deflation attacks. If sync mode were re-enabled, price updates would be susceptible to front-running from users.
The vault includes stale price protection to mitigate risks from outdated prices, which can be disabled by setting it to zero. According to the documentation, the oracle key is expected to be protected by hardware security modules and transaction approval policies (HSM + TAP), but these protections exist entirely off-chain and are not enforced by the contract.
The vault assumes the underlying ERC-20 asset is a standard, well-behaved token. The following token types are explicitly not supported and will cause accounting errors or unexpected behavior: fee-on-transfer tokens that deduct fees on transfer/transferFrom, rebasing tokens with elastic supply that adjust balances automatically, and tokens with transfer hooks implementing deflationary, inflationary, or callback mechanisms. Additionally, when using tokens with administrative transfer restrictions (such as USDC or USDT blocklists), if either the feeRecipient or the vault itself becomes restricted, deposits and redemptions will revert, preventing assets from moving into or out of the vault.
DEFAULT_ADMIN_ROLE: Can grant and revoke all roles, including itself. Required for role management but has no direct operational capabilities. This is a critical single point of failure — if renounced without reassignment, the contract becomes orphaned with no further role modifications or upgrades possible, thus it must be safeguarded with multisig, HSM, governance structure and/or timelock.UPGRADER_ROLE: Can authorize upgrades to new implementation contracts via the UUPS proxy pattern. A compromised upgrader could replace the implementation with malicious code, extracting all funds or modifying any behavior.ORACLE_ROLE: Can set share pricing via setPrice, which also updates the lastPriceUpdate timestamp. This role has full control over share/asset valuations and can single-handedly extract value from the vault by manipulating the price — for example, setting the price extremely low before depositing, then high before redeeming.FUND_ADMIN_ROLE: Can update infrastructure contracts (whitelist, forwarder), set operational limits (maxDepositAssets, maxWithdrawAssets), set maxFeeBasisPoints, update feeRecipient, and configure staleLimit. Can redirect the vault to malicious whitelist or forwarder contracts, set max fee up to 100% potentially siphoning all assets from interactions, and disable stale price protection. Notably, replacing the forwarder with a malicious contract could allow impersonation of any privileged role, including DEFAULT_ADMIN_ROLE.FEE_MANAGER_ROLE: Can set entry and exit fees within the maxFeeBasisPoints limit. Since the maximum can be configured up to 100%, this role could potentially extract all assets from user operations through fees.TRANSFER_MANAGER_ROLE: Can transfer underlying assets out of the vault to arbitrary addresses via transferAssets. Has direct, unconstrained access to all vault funds. A compromised transfer manager could drain the entire vault balance.PAUSER_ROLE: Can pause and unpause vault operations. When paused, all transfers, approvals, deposits, redemptions, asset transfers, and share recovery are blocked. Cannot directly extract funds but can freeze the vault indefinitely, preventing all user interactions.RECOVERY_ROLE: Can burn shares of de-whitelisted users via recoverShares, only callable for users not on the whitelist. Intended for compliance such as recovering shares from sanctioned addresses. Does not transfer underlying assets and must coordinate with TRANSFER_MANAGER_ROLE to move the corresponding assets.DEFAULT_ADMIN_ROLE: Can grant and revoke WHITELIST_ROLE and manage the role hierarchy. If renounced, no whitelist managers can be added or revoked.WHITELIST_ROLE: Can add and remove addresses from the whitelist via whitelistAdd and whitelistRemove, controlling who can hold and transfer vault shares.GatedTrustedForwarder ContractDEFAULT_ADMIN_ROLE: Can grant and revoke RELAYER_ROLE and manage the role hierarchy. If renounced, no relayers can be added or revoked.RELAYER_ROLE: Can execute meta-transactions on behalf of users via execute and executeBatch, and can control transaction submission and timing.The whitelisting mechanism in VaultUpgradeable is designed to restrict vault share ownership and transfers to approved addresses only. However, users holding infinite approvals (type(uint256).max) can bypass these controls even after being removed from the whitelist. The _update function only validates that the from and to addresses are whitelisted. When a user calls transferFrom, the caller (spender) is not checked. Normally, _spendAllowance would invoke _approve to decrement the allowance, which would trigger a whitelist check on the spender. However, for infinite approvals, the allowance update is skipped entirely.
Consider a scenario where User A (whitelisted) grants an infinite approval (type(uint256).max) to User B (whitelisted). User B is subsequently removed from the whitelist due to compliance reasons. Despite being de-whitelisted, User B can still execute transferFrom(userA, userC, amount) to transfer User A's shares to a colluding whitelisted User C. Alternatively, User B can call redeem(shares, userB, userA) to burn User A's shares and extract the underlying assets; even though User B is no longer whitelisted. While manual off-chain filtering or relayer oversight could catch this action in practice, the on-chain contract logic fails to prevent it.
Furthermore, this vulnerability is compounded by a separate issue that blocks owners from revoking approvals for de-whitelisted spenders. Since any attempt by User A to reset the allowance to 0 will revert, User B will permanently retain this infinite approval, allowing them to attempt these unauthorized transfers indefinitely. This effectively renders the whitelist controls ineffective for any user who has previously been granted an infinite approval, undermining the KYC/AML compliance guarantees the whitelist is intended to provide.
Consider ensuring that whitelist restrictions remain enforceable at all times, preventing de-whitelisted spenders from transferring or redeeming vault shares, regardless of prior approvals.
Update: Resolved in pull request #1 at commit 317f947.
The _approve function in VaultUpgradeable enforces whitelist checks on both the owner and the spender. This creates a problematic scenario when a user attempts to revoke an existing approval. If User A has previously approved User B to spend their shares, and User B is subsequently removed from the whitelist, User A becomes unable to revoke this approval. Any call to approve(userB, 0) will revert with NotOnWhitelist(userB) due to the spender whitelist check.
While a de-whitelisted spender cannot immediately exploit non-infinite existing approvals, since _spendAllowance also calls _approve internally, the approval remains active and unrecoverable. If the de-whitelisted user is later re-added to the whitelist, they would regain the ability to use this approval that the owner could not revoke.
Consider modifying the _approve function to skip the spender whitelist check when the approval value is zero, allowing owners to always revoke existing approvals, as a protective action.
Update: Resolved in pull request #2 at commit 0110637.
FUND_ADMIN_ROLE Can Escalate Privileges by Setting a Malicious Trusted ForwarderThe VaultUpgradeable contract relies on _msgSender from ERC2771ContextUpgradeable for all role-based access control. When msg.sender equals the trusted forwarder, the sender address is extracted from the calldata suffix rather than using the actual caller.
The vault is hardcoded to async-only mode during initialization and enforces meta-transaction execution via the trusted forwarder using the asyncForwarderOnly modifier on _deposit and _withdraw. The forwarder address can be updated by FUND_ADMIN_ROLE via setTrustedForwarder. If FUND_ADMIN_ROLE is compromised, it can set the trusted forwarder to a malicious contract that can append an arbitrary address as the calldata suffix and impersonate any role holder or user.
The role architecture is designed to enforce separation of duties, with distinct roles for fund administration, asset transfers, upgrades, and overall governance. This separation allows organizations to distribute responsibilities across different parties, ensuring that no single operational role can unilaterally perform all critical actions. The ability to set an arbitrary forwarder undermines this security model, as FUND_ADMIN_ROLE can effectively impersonate any role in the system and bypass all intended access control boundaries.
Consider preventing forwarded calls from reaching administrative functions restricted to privileged roles. Additionally, consider treating trustedForwarder as a governance-critical dependency, restricting updates to DEFAULT_ADMIN_ROLE and enforcing a timelock and/or other measures to ensure its integrity.
Update: Resolved in pull request #3 at commit c458875. Privileged (role-guarded) functions in the Vault are now restricted to direct msg.sender calls only. Execution via the trusted forwarder (meta-transactions) has been explicitly disabled for these functions.
The VaultUpgradeable contract combines UUPS upgrades with ERC-2771 meta-transaction support. When a call arrives through the trusted forwarder, the forwarder appends the original sender to the calldata, and _msgSender extracts it from the last 20 bytes when msg.sender equals the trusted forwarder.
In a UUPS upgradeToAndCall flow, the authorization check onlyRole(UPGRADER_ROLE) in _authorizeUpgrade correctly uses the outer calldata to recover the forwarded sender. However, the subsequent delegatecall executes with only the ABI-decoded data parameter and the ERC-2771 sender suffix is not propagated. Inside the delegatecall, msg.sender remains the trusted forwarder, but msg.data contains only the setup data without the original sender suffix. Any setup function that calls _msgSender would extract 20 bytes from the end of the setup data, yielding an unintended address or reverting if the data is too short.
The current initialize function assigns roles from explicit parameters. However, if a future reinitializer function relies on _msgSender for critical operations such as role assignment, the role could be granted to an arbitrary address derived from the trailing bytes of the setup data, resulting in privilege escalation or loss of administrative control.
Consider preventing upgrades through the trusted forwarder or ensuring that upgrade setup functions do not rely on _msgSender.
Update: Resolved in pull request #3 at commit c458875. Privileged (role-guarded) functions in the Vault are now restricted to direct msg.sender calls only. Execution via the trusted forwarder (meta-transactions) has been explicitly disabled for these functions.
immutableThe isAsync state variable in the VaultUpgradeable contract is hardcoded to true during initialization and is never modified thereafter. Since the value is constant across all deployments and never reassigned, declaring it as immutable or as a constant would more accurately convey the intended semantics.
Consider declaring isAsync as immutable and assigning it in the constructor, or as a constant if the value will always be true.
Update: Acknowledged, not resolved. The Cubewire team stated:
Keeping
isAsyncas a regular storage variable preserves the flexibility to assign it at initialization time, which may be required in future upgrades. Additionally, changing the variable to immutable would shift the storage slot layout of the contract, introducing risk to the upgradeability of the proxy if we bring the variable back at a later point.
The VaultUpgradeable contract inherits from multiple parent contracts that define the same functions. When overriding these functions, the contract uses super.functionName to call the parent implementation, which relies on Solidity's C3 linearization to determine which parent's function is invoked. While this works correctly, it introduces ambiguity in the code that makes it harder to understand and audit.
The following functions use ambiguous super calls:
_msgSender function overrides both ContextUpgradeable and ERC2771ContextUpgradeable._msgData function overrides both ContextUpgradeable and ERC2771ContextUpgradeable._contextSuffixLength function overrides both ContextUpgradeable and ERC2771ContextUpgradeable.decimals function overrides ERC20Upgradeable, ERC4626Upgradeable, and IERC20Metadata.Consider explicitly specifying which parent contract's function should be called (e.g., ERC2771ContextUpgradeable._msgSender) to improve code clarity and reduce the risk of unintended behavior if the inheritance hierarchy changes in future upgrades.
Update: Resolved in pull request #4 at commit de823df.
The recoverShares function allows the RECOVERY_ROLE to burn shares from de-whitelisted users for compliance purposes. However, since the function calls _burn, which routes through _update, and _update is protected by whenNotPaused, recovery burns revert whenever the vault is paused.
The interface documentation for pause states that "deposits, redeems, transfers, and approvals are blocked" but does not mention recoverShares. In emergency compliance scenarios, it may be legitimate to pause the vault while simultaneously needing to recover shares from a de-whitelisted user.
Consider explicitly documenting that recoverShares is blocked while paused. If recovery during pause is a desired capability for emergency response, consider moving pause enforcement to user-facing entry points or adding a narrowly scoped bypass for recovery burns.
Update: Resolved in pull request #5 at commit bcd3fab. The documentation now clearly specifies that recoverShares is affected by the contract's pausability controls.
maxRedeem Reverts on Stale Prices Contradicting the DocumentationThe maxRedeem function's NatSpec states that it "does not check whether the oracle price is stale" and that it may return a non-zero value even when redeem would revert with PriceIsStale. However, when _maxWithdrawAssets < type(uint256).max / 1e18, the implementation calls _convertToShares, which in turn calls _checkPriceNotStale and reverts when the price is stale. The behavior contradicts the documented behavior, which could mislead integrators relying on the NatSpec. Furthermore, there is an inconsistency with maxDeposit, which does not rely on the conversion function and therefore does not enforce any staleness checks.
Consider either updating maxRedeem to match its documented behavior, or updating the NatSpec to accurately reflect the fact that the function can revert under stale price conditions when _maxWithdrawAssets is finite.
Update: Resolved in pull request #6 at commit bdaa8eb. The maxRedeem function has been updated to no longer revert on stale prices, aligning its behavior with the documented NatSpec.
key Parameter in ForwardRequestDataThe _execute function in GatedTrustedForwarder includes detailed NatSpec documentation describing each field of the ForwardRequestData struct. However, the key field is omitted from this documentation. The key parameter is a significant addition to the standard ERC-2771 forwarder, as it enables keyed nonces that allow users to submit multiple transactions in parallel without nonce conflicts. This is explicitly noted as a change from the original OpenZeppelin implementation in ERC2771ForwarderKeyedNonce.
Consider updating the NatSpec for _execute to include documentation for the key field.
Update: Resolved in pull request #7 at commit 2d16fea.
The _execute function in ERC2771ForwarderKeyedNonce emits the ExecutedForwardRequest event with a nonce parameter that is a packed value combining both the key and the sequential nonce. Off-chain indexers must unpack this value to interpret the key and sequence correctly.
Consider documenting the packing format in the event's NatSpec to aid off-chain consumers and reduce the risk of misinterpretation.
Update: Resolved in pull request #8 at commits 914c3b9 and f566bc3.
The VaultUpgradeable and ERC4626FeesUpgradeable contracts use traditional sequential storage layout with __gap [1] [2] variables for upgrade safety. While storage gaps provide reserved space for future state variables, this approach requires careful management during upgrades. EIP-7201 namespaced storage eliminates these concerns by placing each contract's storage at a deterministic, collision-resistant slot derived from a namespace identifier, providing greater flexibility and safety for future upgrades.
Consider migrating to EIP-7201 namespaced storage layout or ensuring that upgrade procedures include thorough storage layout verification to prevent slot collisions.
Update: Acknowledged, not resolved. The Cubewire team stated:
The existing layout is well-understood and tested, we agree that in the event of an upgrade thorough storage layout verification should be undertaken.
The Whitelist contract implements IWhitelist and inherits from AccessControl, which provides ERC-165 support via supportsInterface. However, the contract does not override supportsInterface to advertise type(IWhitelist).interfaceId.
Similarly, the VaultUpgradeable contract implements IVaultUpgradeable but does not advertise this interface through ERC-165.
Consider overriding supportsInterface in both contracts to return true for their respective interface IDs, or documenting the fact that ERC-165 is not implemented for these interfaces.
Update: Resolved in pull request #9 at commit a8bc56a.
feeRecipient_ as Zero-Address CompatibleThe initialize function's NatSpec documents feeRecipient_ as accepting zero address to accumulate fees in the vault. However, _setFeeRecipient explicitly reverts with InvalidFeeRecipient when the address is zero. The actual mechanism for accumulating fees in the vault is to set feeRecipient to address(this), as the fee transfer is skipped when feeRecipient == address(this) [1] [2].
Consider updating the NatSpec to accurately state that feeRecipient_ cannot be zero and that address(this) should be used to accumulate fees in the vault.
Update: Resolved in pull request #10 at commit 672c77b.
The _approve function includes the whenNotPaused modifier, preventing all allowance changes while the vault is paused, including defensive revocations via approve(spender, 0). While compromised spenders cannot act during the pause, this creates a race condition upon unpause where previously approved spenders could act before owners can revoke.
Consider allowing approval revocations while paused or documenting this as expected behavior during emergency scenarios.
Update: Resolved in pull request #11 at commits e959085 and c185432. The documentation has been updated to clarify that allowance changes, including revocations, are disabled while the contract is paused.
maxDeposit and maxRedeem Do Not Account for Pause and Whitelist StateThe maxDeposit and maxRedeem functions only check whether _price is set, but do not reflect pause state or whitelist restrictions that are enforced during execution. When the vault is paused or when the receiver/owner is not whitelisted, these functions can return non-zero values even though the corresponding deposit or redeem call would revert.
The ERC-4626 specification expects max* functions to return the effective maximum that can actually be deposited or redeemed for a given account. The NatSpec documents the staleness deviation but does not mention pause or whitelist considerations. Integrators relying on these view functions for preflight checks may submit transactions guaranteed to revert.
Consider returning zero from maxDeposit and maxRedeem when the vault is paused or when the receiver/owner is not whitelisted. Alternatively, consider documenting these deviations alongside the existing staleness deviation note.
Update: Resolved in pull request #12 at commit 823ee8e. The documentation for maxDeposit and maxRedeem has been updated to note that these functions do not account for pause state, or whitelist restrictions.
totalAssets Returns Stale NAV Without Enforcing Staleness ChecksThe totalAssets function returns an oracle-implied NAV calculated as totalSupply * _price / 1e18. Unlike _convertToShares and _convertToAssets, which enforce freshness via _checkPriceNotStale, totalAssets does not verify that the oracle price is current, silently returning a potentially outdated NAV with no indication of its staleness. Similarly, the getPrice function returns the raw stored price without staleness validation. As such, integrators should use lastPriceUpdate and staleLimit to verify freshness.
Consider documenting the staleness behavior in the NatSpec, or reverting with PriceIsStale.
Update: Resolved in pull request #13 at commit 37a54e7. The totalAssets and getPrice functions now enforce staleness checks and will revert with PriceIsStale if the oracle price is outdated.
The slippage-protected deposit and redeem functions perform redundant price conversions due to their wrapper pattern over the parent ERC-4626 implementation.
The VaultUpgradeable.deposit function first calls ERC4626FeesUpgradeable.previewDeposit for slippage checking, which calculates the fee via _feeOnRaw and calls _convertToShares enforcing a price validation and staleness check. After slippage validation passes, it calls super.deposit, which is the ERC4626Upgradeable.deposit function. This parent function calls previewDeposit again internally, repeating the fee calculation and conversion. Finally, ERC4626FeesUpgradeable._deposit calculates the fee a third time via _feeOnRaw to perform the actual fee transfer.
The redeem function first calls previewRedeem for slippage checking, which calls _convertToAssets and calculates the fee. After slippage validation, it calls super.redeem, which is the ERC4626Upgradeable.redeem function. This parent function calls maxRedeem, which conditionally calls _convertToShares to convert _maxWithdrawAssets to shares. Then it calls previewRedeem again, repeating the conversion and fee calculation. Finally, ERC4626FeesUpgradeable._withdraw calls _convertToAssets to recalculate the gross assets for the fee computation.
Each conversion involves storage reads and a mulDiv operation, with the redeem flow particularly inefficient — performing up to four conversions, three of which produce the same shares-to-assets value.
Consider simplifying the deposit and redeem flows to improve code readability, reduce complexity, and eliminate redundant computation.
Update: Acknowledged, not resolved. The Cubewire team stated:
Addressing this finding would require overriding core OpenZeppelin ERC-4626 library functions, adding additional lines of code and increasing the surface area for bugs in exchange for minor gas savings. We prefer to maintain the readability and minimise the vulnerable surface area.
The IVaultUpgradeable interface does not declare the fee-related events, errors or setter functions that are part of the vault's public API via ERC4626FeesUpgradeable. While the contract behaves correctly, the interface does not serve as a complete reference for the vault's capabilities, which may cause front-ends and indexers to miss fee-related events.
Consider thoroughly documenting the vault's public API in IVaultUpgradeable for completeness.
Update: Resolved in pull request #9 at commit a8bc56a.
The Vault system is a well-architected, upgradeable tokenized vault designed for regulated fund management, implementing oracle-based pricing, configurable fees, whitelist access control, gasless approvals via permit, multicall functionality, and gasless transactions through ERC-2771 meta-transactions. The modular design cleanly separates concerns across the core vault, whitelist, and forwarder contracts, while the multi-role access control architecture enables fine-grained permission management appropriate for institutional use cases.
The system operates under a managed trust model, where security relies heavily on off-chain infrastructure and privileged role holders. The oracle is fully trusted to provide accurate pricing, the relayer infrastructure governs all deposit and redemption flows, and multiple roles possess the ability to extract or redirect user funds if compromised. While these privileged roles are expected to be protected via hardware security modules, transaction-approval policies, governance structures, and/or timelocks, such protections exist entirely off-chain and are not enforced by the contracts. Thus, organizations deploying this system must implement robust operational security around privileged keys and relayer infrastructure.
The audit identified an edge case in whitelist enforcement for infinite approvals, a privilege boundary consideration related to trusted forwarder configuration, minor documentation gaps, and several other areas for improvement in an otherwise solid codebase. Overall, the codebase demonstrates exceptional documentation quality, featuring comprehensive NatSpec comments, clear disclosures of trust assumptions, and embedded architectural diagrams. This level of documentation substantially facilitated the audit process and reflects a mature, professional approach to smart contract development.
The Cubewire team is appreciated for their collaboration and responsiveness throughout the audit process.
It should be noted that this audit only covered the asynchronous mode, where all user-vault interactions go through the trusted forwarder and are executed by the relayer role. Enabling a synchronous mode in the future may introduce new risk vectors and would require a further audit.
OpenZeppelin classifies smart contract vulnerabilities on a 5-level scale:
This classification is applied when the issue’s impact is catastrophic, threatening extensive damage to the client's reputation and/or causing severe financial loss to the client or users. The likelihood of exploitation can be high, warranting a swift response. Critical issues typically involve significant risks such as the permanent loss or locking of a large volume of users' sensitive assets or the failure of core system functionalities without viable mitigations. These issues demand immediate attention due to their potential to compromise system integrity or user trust significantly.
These issues are characterized by the potential to substantially impact the client’s reputation and/or result in considerable financial losses. The likelihood of exploitation is significant, warranting a swift response. Such issues might include temporary loss or locking of a significant number of users' sensitive assets or disruptions to critical system functionalities, albeit with potential, yet limited, mitigations available. The emphasis is on the significant but not always catastrophic effects on system operation or asset security, necessitating prompt and effective remediation.
Issues classified as being of medium severity can lead to a noticeable negative impact on the client's reputation and/or moderate financial losses. Such issues, if left unattended, have a moderate likelihood of being exploited or may cause unwanted side effects in the system. These issues are typically confined to a smaller subset of users' sensitive assets or might involve deviations from the specified system design that, while not directly financial in nature, compromise system integrity or user experience. The focus here is on issues that pose a real but contained risk, warranting timely attention to prevent escalation.
Low-severity issues are those that have a low impact on the client's operations and/or reputation. These issues may represent minor risks or inefficiencies to the client's specific business model. They are identified as areas for improvement that, while not urgent, could enhance the security and quality of the codebase if addressed.
This category is reserved for issues that, despite having a minimal impact, are still important to resolve. Addressing these issues contributes to the overall security posture and code quality improvement but does not require immediate action. It reflects a commitment to maintaining high standards and continuous improvement, even in areas that do not pose immediate risks.