Back to Audits
PlutusAudited by Bulwark Security2026-06-24v1.3

Security Assessment for OVaultRWA

Table of Contents

  1. Summary
  2. Scope
  3. Methodology
  4. Severity Classification
  5. Findings Summary
  6. First Mitigation Review 2026-07-06
  7. Second Mitigation Review 2026-07-13
  8. Third Mitigation Review 2026-07-15
  9. Detailed Findings
  10. Disclaimer

Summary

This report presents the findings of the security assessment conducted on the smart contracts of the OVaultRWA project, a cross-chain RWA vault system built around a hub-chain asynchronous vault, LayerZero V2 messaging, Stargate USDC transfers, and OFT-based vault share movement.

This report covers the initial assessment and subsequent mitigation/follow-up reviews:

Review PhaseDateCommitNotes
Initial assessment findings2026-06-24481db73Fresh manual review, triage of older automated findings in automated findings/, and validation of test/static-analysis status.
First mitigation review2026-07-061f322a7Review of fixes applied. Additional findings identified during the review.
Second mitigation review2026-07-13fa0a8a5Additional review of fixes applied. Additional follow-up findings.
Third mitigation review2026-07-156177fb5Additional review of fixes applied.

Assessment Overview

MetricValue
Total Issues Found21
Critical2
High2
Medium4
Low10
Informational3
Original Findings14
Later Review Findings7

Scope

The following files were included in the scope of this audit:

FileDescription
src/base/PlutusBaseVault.solBase vault contract with ERC4626, Ownable, Pausable, and UUPS inheritance
src/vault/PlutusAsyncVault.solMain async vault contract with deposits, redemptions, fees, NAV, and capital accounting
src/crosschain/OmniVaultComposer.solHub-chain LayerZero compose handler and cross-chain operation coordinator
src/crosschain/OmniVaultReceiver.solHub-chain OApp receiver/relay for cancel, claim, recover, pause, and status-update messages
src/factory/PlutusVaultFactory.solFactory for deploying and tracking vault, receiver, and composer proxy pairs
src/oft/ShareOFT.solSpoke-chain OFT share token
src/oft/ShareOFTAdapter.solHub-chain OFT adapter / lockbox for vault shares
src/routers/HubRouter.solHub-chain user router for direct deposits and redemptions
src/routers/SpokeRouter.solSpoke-chain router for cross-chain deposits, redemptions, cancellation, claims, and recovery
src/interfaces/*.solProtocol interfaces

The older reports under automated findings/ and root-level OVaultRWA audit reports were reviewed as supporting material. Findings from those reports were manually revalidated against the current source code before inclusion.

Repository: https://github.com/PlutusDao/OVaultRWA (private)
Assessed Commit: 481db73
First Mitigation Review Commit: 1f322a7
Second Mitigation Review Commit: fa0a8a5
Third Mitigation Review Commit: 6177fb5


Methodology

The audit process involved a combination of:

  1. Manual Code Review: Line-by-line inspection of factory deployment, access control, vault accounting, cross-chain message handling, recovery paths, bridge approvals, and upgrade authorization.
  2. Previous Finding Triage: Review and validation of older reports in automated findings/, including Pashov, Quill, Trail of Bits-style context outputs, and consolidated reports.
  3. Automated Testing: Running the full Foundry test suite.
  4. Automated Static Analysis: Running Slither with external dependencies, tests, scripts, and libraries filtered out of the reported result set.
  5. Manual Triage: Selecting findings based on current-code validity, practical exploitability, and consistency with the intended protocol design.

The automated component was used as review support rather than as a substitute for triage. Several older automated findings were found to be stale, fixed, or severity-inflated because the implementation has materially changed.

Code Excerpt Note: Some code snippets include shortened excerpts for readability. Line numbers refer to the assessed commit.


Severity Classification

LevelDescription
CriticalA vulnerability that can lead to significant loss of assets or contract state manipulation.
HighA vulnerability that can lead to loss of assets or contract state manipulation, but requires specific conditions, has limited scope, or is difficult to exploit.
MediumA vulnerability that can lead to contract state manipulation without the loss of assets, violates implementation requirements, or major deviations from best practices.
LowMinor issues, deviations from best practices, or risks that do not result in immediate loss of assets.
InformationalCode style and readability improvements, governance risks.

Findings Summary

Initial Assessment Findings — 2026-06-24

Finding IDTitleSeverityStatus
[C-01]Factory-deployed vaults leave critical vault roles assigned to the factoryCriticalResolved
[C-02]Cross-chain redeem shares can be stranded by insufficient PENDING status feeCriticalResolved
[H-01]Quote and helper payloads do not match executable cross-chain payloadsHighResolved
[M-01]Spoke status can be marked final before bridge delivery succeedsMediumResolved
[M-02]Composer admin setters allow zero critical bridge addressesMediumResolved
[M-03]Vault-level deposit and async redeem slippage bounds are missingMediumResolved
[L-01]Factory accepts zero or non-contract implementation addressesLowResolved
[L-02]ShareOFTAdapter.setComposer(address(0)) disables outbound share bridgingLowResolved
[L-03]liquidityBufferBps is stored but not enforcedLowResolved
[L-04]Claim and fulfill quote helpers use live share price instead of locked assetsLowResolved
[L-05]Admin and upgrade trust model is highly centralizedLowOpen
[I-01]Technical report is materially stale relative to the current implementationInformationalPartially Resolved
[I-02]ERC4626 first-deposit, rounding, and conversion-ordering exploit classes were reviewedInformationalMitigated / Not Present
[I-03]Drifting code style reduces readability and increases review riskInformationalOpen

First Mitigation Review Findings — 2026-07-06

The following findings were identified while performing the mitigation review against commit 1f322a7.

Finding IDTitleSeverityStatus
[H-02]Cross-chain redeem/cancel can be hijacked from another spoke with the same caller addressHighResolved
[M-04]returnNativeFee funding model is inconsistent across cross-chain flowsMediumResolved
[L-06]_handleRedeem() does not reject zero return addressesLowResolved
[L-07]Factory deployment should prevalidate critical deployment parametersLowResolved
[L-08]returnNativeFee overpayment is not consistently refunded or accountedLowResolved

Second Mitigation Review Findings — 2026-07-13

The following findings were identified during the second mitigation review against commit fa0a8a5. They were present in earlier reviewed versions, but the latest changes made the affected paths easier to isolate and verify.

Finding IDTitleSeverityStatus
[L-09]Vault accounting assumes non-fee-on-transfer underlying assetsLowOpen
[L-10]Accidental native ETH can become stuck in the vault during non-cross-chain fulfillmentLowResolved

First Mitigation Review 2026-07-06

This mitigation review was performed against commit 1f322a7. Automated tests were executed with forge test and passed with 424 tests passing and 0 failures. The fixes for the Critical and High severity findings were reviewed line by line to assess their effectiveness.

Finding IDStatusMitigation Review Notes
[C-01]ResolvedFactory deployment now grants DEFAULT_ADMIN_ROLE to the configured owner, keeps the operational OPERATOR_ROLE on the configured operator, transfers vault and receiver ownership to the owner, and renounces the factory's temporary admin role.
[C-02]Resolved_handleRedeem() now pre-quotes the PENDING status-update fee before creating the vault request and parks shares in recoverableShares instead of reverting when the embedded return fee is insufficient.
[H-01]ResolvedQuote helpers and composer encoding helpers now include the same payload fields used by executable paths, including minSharesOut, minAssetsOut, and returnNativeFee.
[M-01]ResolvedReturn bridge sends are now attempted before final status updates; bridge failures result in recoverable accounting and RECOVERABLE status updates.
[M-02]ResolvedsetShareOFTAdapter() and setStargatePool() now reject zero addresses, while explicit disable functions are available for intentional emergency disabling.
[M-03]Partially ResolvedDeposit flows now support minSharesOut, and async redemption requests now store and enforce minAssetsOut during fulfillment. No deadline/expiry protection was added.
[L-01]ResolvedFactory initialization and implementation setters now validate that implementation and infrastructure addresses are nonzero contracts.
[L-02]ResolvedShareOFTAdapter.setComposer() now rejects the zero address, and disabling outbound adapter sends requires the explicit disableComposer() function.
[L-03]ResolvedliquidityBufferBps was removed from the current vault implementation, eliminating the misleading unenforced configuration value.
[L-04]ResolvedQuote helpers now use req.lockedAssets for CLAIMABLE or CLAIMED requests and live conversion only for pending estimates.
[I-01]Partially ResolvedThe main technical and frontend-flow documentation has been substantially updated for the current minSharesOut / minAssetsOut, locked-assets, fee, and recovery model. However, backend-flow documentation still references removed roles and outdated status ordering, the vault interface still contains a stale share-burn comment, and older docs/v2_report.md / plans/ files contain stale implementation details.

All other findings remain open or unchanged as of commit 1f322a7.


Second Mitigation Review 2026-07-13

This mitigation review was performed against commit fa0a8a5. Automated tests were executed with forge test and passed with 447 tests passing and 0 failures. Slither was also executed with dependencies, tests, scripts, and libraries filtered and reported 0 findings.

Finding IDStatusMitigation Review Notes
[H-02]ResolvedhandleCancel() and handleClaim() now verify that the incoming source endpoint ID matches the original pendingRedeems[requestId].srcEid, preventing cross-spoke same-address claim or cancel hijacking. Regression tests were added for both claim and cancel from a different spoke.
[M-03]ResolvedRedemption requests now include a deadline; fulfillRedeem() rejects expired requests, and batchFulfillRedeem() / checkBatchFulfill() skip expired requests during preflight. A zero deadline is treated as no expiry.
[M-04]ResolvedSpokeRouter now validates LayerZero options with _validateOptions() and requires the options to deliver at least the declared returnNativeFee to the destination for deposit, redeem, cancel, claim, and recovery flows.
[L-06]Resolved_handleRedeem() now rejects returnAddress == address(0) before creating a vault request or storing pending redeem metadata.
[L-07]ResolvedPlutusVaultFactory.deployVault() now prevalidates asset, owner, operator, and fulfiller before deploying proxies. The asset must be nonzero and contain code.
[L-08]Resolved_handleRedeem() now spends exactly the quoted status-update native fee and credits excess ETH to gasFeeRefunds; related failure paths also credit unused ETH to refunds.
[I-01]Partially ResolvedBackend-flow documentation and the stale vault interface share-burn comment have been updated. Historical docs/v2_report.md and plans/ files are now marked as historical, but the current technical report still contains a stale note claiming the enum comment references share burn at claim.

Third Mitigation Review 2026-07-15

This third mitigation review was performed against commit 6177fb5. Automated tests were executed with forge test and passed with 451 tests passing and 0 failures. Slither was also executed with dependencies, tests, scripts, and libraries filtered and reported 0 findings.

Finding IDStatusMitigation Review Notes
[L-10]ResolvedfulfillRedeem() and batchFulfillRedeem() now refund accidental native ETH to msg.sender whenever no cross-chain notification is sent. Tests were added for successful refunds and refund-failure reverts.
[I-01]Partially ResolvedHistorical design/report files are now marked as historical, reducing the risk that operators treat stale design notes as current specification. One stale note remains in the current technical report regarding the enum comment for share-burn timing.

Detailed Findings

Critical Severity

[C-01] Factory-deployed vaults leave critical vault roles assigned to the factory

Review PhaseInitial assessment, 2026-06-24
Discovered In Commit481db73
StatusResolved

Update 2026-07-06: Fixed in commit 1f322a7. PlutusVaultFactory.deployVault() now grants the vault DEFAULT_ADMIN_ROLE to the configured owner, leaves the operational OPERATOR_ROLE with the configured operator, transfers vault and receiver ownership to the owner, and renounces the factory's temporary admin role. The deployment tests now assert that the factory no longer retains vault admin control after deployment.

Description: PlutusVaultFactory.deployVault() deploys each vault with the factory as the temporary owner_ passed into PlutusAsyncVault.initialize(). The vault initializer grants multiple AccessControl roles to owner_. The factory later transfers only Ownable ownership to operator, but it does not transfer the vault AccessControl roles.

This creates a split-brain administration state: the operator becomes owner() for onlyOwner functions, while the factory remains the holder of DEFAULT_ADMIN_ROLE, DEPLOYER_ROLE, and PAUSER_ROLE.

Note that the naming is actively misleading. operator in the factory is documented as the vault owner/admin, while operator_ in the vault is the fulfillment operator.

Location: src/factory/PlutusVaultFactory.sol:117-125, 151

// PlutusVaultFactory.deployVault()
vaultProxy = address(
    new ERC1967Proxy{ salt: vaultSalt }(
        vaultImplementation,
        abi.encodeCall(
            PlutusAsyncVault.initialize,
            (IERC20(asset), name, symbol, address(this), fulfiller, feeBps, liquidityBufferBps, treasury)
        )
    )
);

// Later, only Ownable ownership is transferred.
IOwnable(vaultProxy).transferOwnership(operator);

Location: src/vault/PlutusAsyncVault.sol:109-114

// PlutusAsyncVault.initialize()
_plutusBaseVaultInit(asset_, name_, symbol_, owner_);
__AccessControl_init();
_grantRole(DEFAULT_ADMIN_ROLE, owner_);
_grantRole(DEPLOYER_ROLE, owner_);
_grantRole(OPERATOR_ROLE, operator_);
_grantRole(PAUSER_ROLE, owner_);

Impact: Factory-deployed vaults can be severely operationally impaired. The operator owns the vault but cannot perform role-gated administrative actions such as:

  • grantOperator() / revokeOperator()
  • grantOracle() / revokeOracle()
  • grantPauser() / revokePauser()
  • deployCapital()
  • settleManagedCapital()
  • adminDeposit()
  • returnAllManagedAssets()
  • sweepUncontrolled()
  • pause() / unpause()

The factory retains those roles, but the factory does not expose forwarding functions to use them safely after deployment. Additionally, OmniVaultComposer.pauseAndBroadcast() calls vault.pause() / vault.unpause(), which require PAUSER_ROLE. The factory does not grant PAUSER_ROLE to the composer, so the intended cross-chain emergency pause flow can revert for factory-deployed vaults.

Proof of Concept: After a factory deployment, the following invariant fails:

assertEq(vault.owner(), operator);
assertFalse(vault.hasRole(vault.DEFAULT_ADMIN_ROLE(), operator));
assertFalse(vault.hasRole(vault.DEPLOYER_ROLE(), operator));
assertFalse(vault.hasRole(vault.PAUSER_ROLE(), operator));

The operator then cannot administer the vault roles:

vm.prank(operator);
vm.expectRevert();
vault.grantOracle(oracle);

Recommendation: During deployVault(), explicitly grant all required vault roles to the intended operational admin and revoke or renounce them from the factory. If the composer is expected to pause/unpause through pauseAndBroadcast(), also grant PAUSER_ROLE to the composer.

PlutusAsyncVault vault = PlutusAsyncVault(vaultProxy);

vault.grantRole(vault.DEFAULT_ADMIN_ROLE(), operator);
vault.grantRole(vault.DEPLOYER_ROLE(), operator);
vault.grantRole(vault.PAUSER_ROLE(), operator);
vault.grantRole(vault.PAUSER_ROLE(), composerProxy);

vault.renounceRole(vault.DEPLOYER_ROLE(), address(this));
vault.renounceRole(vault.PAUSER_ROLE(), address(this));
vault.renounceRole(vault.DEFAULT_ADMIN_ROLE(), address(this));

Also add a factory deployment test asserting that the final operator and composer have exactly the expected roles.


[C-02] Cross-chain redeem shares can be stranded by insufficient PENDING status fee

Review PhaseInitial assessment, 2026-06-24
Discovered In Commit481db73
StatusResolved

Update 2026-07-06: Fixed in commit 1f322a7. OmniVaultComposer._handleRedeem() now quotes the PENDING status-update fee before calling vault.requestRedeem(). If the embedded returnNativeFee is insufficient, the composer credits recoverableShares[returnAddress][srcEid], stores the failed message, emits a failure event, and returns without creating a vault request or reverting after shares have arrived on the hub.

Description: SpokeRouter.requestCrossChainRedeem() sends shares to the hub composer and encodes a user-provided returnNativeFee into the OFT compose payload. On the hub, OmniVaultComposer._handleRedeem() creates a vault redeem request and then forwards exactly returnNativeFee to OmniVaultReceiver.sendStatusUpdate() to notify the spoke that the request is PENDING.

If the embedded returnNativeFee is lower than the actual LayerZero fee required for the status update, sendStatusUpdate() reverts. The try/catch in _handleRedeem() only catches a revert from the external vault.requestRedeem() call. It does not catch a revert that occurs later inside the success block. As a result, the entire compose execution reverts after the ShareOFT transfer has already delivered shares to the hub composer.

Because the insufficient fee is embedded in the original compose payload, retrying the same compose message can continue to fail with the same insufficient fee. No reliable pendingRedeems or recoverableShares entry is created for the user.

Location: src/routers/SpokeRouter.sol:204-215

// SpokeRouter.requestCrossChainRedeem()
bytes memory composeMsg = abi.encode(OP_REDEEM, returnAddress, returnNativeFee);

SendParam memory sendParam = SendParam({
    dstEid: hubEid,
    to: addressToBytes32(hubComposer),
    amountLD: shares,
    minAmountLD: minAmountLD,
    extraOptions: combineOptions(hubEid, uint16(OP_REDEEM), redeemOptions),
    composeMsg: composeMsg,
    oftCmd: ""
});

Location: src/crosschain/OmniVaultComposer.sol:878-893

// OmniVaultComposer._handleRedeem()
try vault.requestRedeem(shares, address(this), address(this)) returns (uint256 requestId) {
    pendingRedeems[requestId] = PendingRedeemReturn({ srcEid: srcEid, returnAddress: returnAddress });

    bytes memory statusMsg = abi.encode(uint8(0x07), requestId, IPlutusVault.RedeemStatus.PENDING, returnAddress);
    bytes memory statusOptions = _buildOptions(statusUpdateGas);
    IOmniVaultReceiver(oAppReceiver).sendStatusUpdate{ value: returnNativeFee }(srcEid, statusMsg, statusOptions);

    emit CrossChainRedeemRequested(returnAddress, requestId, shares, srcEid);
} catch {
    recoverableShares[returnAddress][srcEid] += shares;
    failedMessages[guid] = message;
    emit CrossChainRedeemFailed(returnAddress, shares, "requestRedeem failed", guid);
}

Impact: A spoke user can lose practical access to their bridged shares if the status fee is stale or underquoted. The user may not be able to cancel, claim, or trigger the normal recovery path because the hub-side redeem metadata is not reliably persisted. Recovery may require direct administrative intervention, manual token rescue, or an implementation upgrade.

This is not theoretical. Cross-chain fee quotes are time-sensitive, and the current quote helpers underquote the executable payloads as described in [H-01].

Proof of Concept Scenario:

  1. User calls requestCrossChainRedeem() with valid shares and a stale returnNativeFee.
  2. ShareOFT burns shares on the spoke and credits hub vault shares to the composer.
  3. LayerZero calls lzCompose() on the hub.
  4. _handleRedeem() calls vault.requestRedeem() successfully.
  5. _handleRedeem() calls sendStatusUpdate{ value: returnNativeFee }().
  6. sendStatusUpdate() reverts because the actual fee is higher.
  7. The compose execution reverts.
  8. The same compose payload still contains the same insufficient returnNativeFee, so retries can repeatedly fail.

Recommendation: Quote and validate the required status-update fee before creating the vault request. If the available fee is insufficient, credit recoverableShares[returnAddress][srcEid] and return cleanly instead of reverting after the asset transfer.

bytes memory statusMsg = abi.encode(uint8(0x07), 0, IPlutusVault.RedeemStatus.PENDING, returnAddress);
bytes memory statusOptions = _buildOptions(statusUpdateGas);
MessagingFee memory statusFee = IOmniVaultReceiver(oAppReceiver).quoteStatusUpdate(srcEid, statusMsg, statusOptions);

if (msg.value < statusFee.nativeFee) {
    recoverableShares[returnAddress][srcEid] += shares;
    failedMessages[guid] = message;
    emit CrossChainRedeemFailed(returnAddress, shares, "insufficient status fee", guid);
    return;
}

Because the final requestId is not known before requestRedeem(), the exact status message may require either a conservative pre-check, a two-step fee model, or sending with the full available msg.value rather than only the embedded returnNativeFee. The important invariant is that an underfunded status update must never strand shares without a deterministic recovery bucket.


High Severity

[H-01] Quote and helper payloads do not match executable cross-chain payloads

Review PhaseInitial assessment, 2026-06-24
Discovered In Commit481db73
StatusResolved

Update 2026-07-06: Fixed in commit 1f322a7. The spoke quote helpers now encode the same message shapes as their corresponding execution functions. The composer encodeDeposit() and encodeRedeem() helpers were also updated to include vault-level slippage parameters and returnNativeFee.

Description: Several quote and helper functions encode shorter messages than the actual execution functions. The execution functions include returnNativeFee, while quote/helper functions omit it.

This discrepancy has two consequences:

  1. fee quotes can be lower than the fee needed for the actual transaction payload, and
  2. integrations using the helper encoders can produce payloads that revert during hub-side decoding.

Execution paths encode three or more fields:

Location: src/routers/SpokeRouter.sol:146, 204, 252, 287, 325

abi.encode(OP_DEPOSIT, controller, returnNativeFee);
abi.encode(OP_REDEEM, returnAddress, returnNativeFee);
abi.encode(OP_CANCEL, requestId, msg.sender, returnNativeFee);
abi.encode(OP_CLAIM, requestId, msg.sender, minAmountLD, returnNativeFee);
abi.encode(OP_RECOVER, controller, destination, returnNativeFee);

Quote/helper paths encode shorter messages:

Location: src/routers/SpokeRouter.sol:416, 441, 459, 483, 529

abi.encode(OP_DEPOSIT, controller);
abi.encode(OP_REDEEM, returnAddress);
abi.encode(OP_CANCEL, requestId, msg.sender);
abi.encode(OP_CLAIM, requestId, msg.sender, minAmountLD);
abi.encode(OP_RECOVER, controller, destination);

The composer helper functions also return two-field payloads:

Location: src/crosschain/OmniVaultComposer.sol:763, 770

abi.encode(OP_DEPOSIT, controller);
abi.encode(OP_REDEEM, returnAddress);

Impact: Frontends and integrations relying on these quote helpers can underfund cross-chain actions. In the redeem flow, underfunding directly increases the likelihood of [C-02], where shares can become stranded because the PENDING status update cannot be delivered.

Integrations relying on encodeDeposit() or encodeRedeem() as executable helper APIs will generate messages that cannot be decoded by the current lzCompose() path, which expects returnNativeFee.

Recommendation: Make all quote and helper functions encode the exact same payload shape used by execution. Add returnNativeFee parameters to quote helpers, or split the API into clearly named functions such as quoteOutboundOnly() and quoteOutboundWithReturnFee().

function quoteCrossChainRedeem(
    uint256 shares,
    uint256 minAmountLD,
    address returnAddress,
    bytes calldata redeemOptions,
    uint256 returnNativeFee
) public view returns (MessagingFee memory fee) {
    bytes memory composeMsg = abi.encode(OP_REDEEM, returnAddress, returnNativeFee);
    ...
}

The same change should be made for deposit, cancel, claim, and recovery quote paths.


[H-02] Cross-chain redeem/cancel can be hijacked from another spoke with the same caller address

Review PhaseFirst mitigation review, 2026-07-06
Discovered In Commit1f322a7
StatusResolved

Update 2026-07-13: Fixed in commit fa0a8a5. handleCancel() and handleClaim() now compare the incoming srcEid with the source endpoint ID stored in pendingRedeems[requestId] and revert with InvalidSourceEid on mismatch. Regression tests cover same-caller attempts from a different spoke for both cancel and claim.

Description: OmniVaultComposer.handleCancel() and OmniVaultComposer.handleClaim() authenticate a cross-chain cancel or claim by checking that the incoming caller equals the stored returnAddress. However, the functions do not verify that the message came from the same source endpoint ID that originally created the redeem request.

The request records both values:

Location: src/crosschain/OmniVaultComposer.sol:996

pendingRedeems[requestId] = PendingRedeemReturn({ srcEid: srcEid, returnAddress: returnAddress });

But cancellation and claim only enforce the address check:

Location: src/crosschain/OmniVaultComposer.sol:374, 469

if (ret.returnAddress != caller) revert NotReturnAddress(requestId, caller);

They then use the incoming srcEid supplied by the receiver path for the return leg. This means a request opened from spoke A can be cancelled or claimed from spoke B if the caller address matches.

Impact: Address equality across chains is not sufficient identity. Contract wallets, CREATE2 deployments, account-abstraction wallets, and multisigs may have the same address on multiple chains but different owners or code. An attacker who controls the same address on another registered spoke could redirect a claim or cancel return to that other spoke. This can result in theft of redeemed USDC or returned shares under realistic cross-chain address-collision conditions.

Recommendation: Bind every redeem lifecycle operation to the original source chain. Both handleCancel() and handleClaim() should verify ret.srcEid == srcEid before any vault action or return transfer. Use ret.srcEid for return sends instead of trusting the incoming source value.

if (ret.srcEid != srcEid) revert InvalidSourceEid(requestId, ret.srcEid, srcEid);

Add regression tests where a redeem is created from one spoke and a cancel/claim is attempted from another spoke with the same caller address.


Medium Severity

[M-01] Spoke status can be marked final before bridge delivery succeeds

Review PhaseInitial assessment, 2026-06-24
Discovered In Commit481db73
StatusResolved

Update 2026-07-06: Fixed in commit 1f322a7. The composer now sends the return bridge leg before emitting final status updates. If the Stargate or ShareOFTAdapter send fails, funds are moved into the appropriate recoverable bucket and the spoke is notified with RECOVERABLE status instead of a misleading final status.

Description: Some cross-chain flows send status updates before the corresponding return bridge leg is guaranteed to succeed. For example, onFulfilRedeem() prepares and sends a CLAIMED status update before the Stargate USDC return is attempted. If the Stargate send later fails, the assets are credited to recoverableAssets, but the spoke may already show the request as CLAIMED.

A similar state mismatch can occur in cancellation paths: the request can be reported as CANCELLED while the share return leg later fails and the shares are only recoverable.

Location: src/crosschain/OmniVaultComposer.sol:288-332

bytes memory statusMsg = abi.encode(uint8(0x07), requestId, IPlutusVault.RedeemStatus.CLAIMED, ret.returnAddress);
...
IOmniVaultReceiver(oAppReceiver).sendStatusUpdate{ value: statusFee.nativeFee }(ret.srcEid, statusMsg, statusOptions);
...
try IOFT(stargatePool).send{ value: fee.nativeFee }(sendParam, fee, payable(msg.sender)) {
    ...
} catch {
    recoverableAssets[ret.returnAddress][ret.srcEid] += assets;
    emit CrossChainRedeemFailed(ret.returnAddress, assets, "stargate bridge failed", bytes32(requestId));
}

Impact: Assets are generally recoverable, but spoke-chain state and user-facing UI can become misleading. A user may see CLAIMED or CANCELLED even though their assets or shares were not returned and require a separate recovery action. Frontends may hide or disable the recovery flow if they treat final statuses as complete.

Recommendation: Add explicit intermediate or failure statuses such as CLAIMING, RETURN_FAILED, or RECOVERABLE. Alternatively, only send final CLAIMED / CANCELLED status updates after the return bridge send succeeds. If a bridge send fails, send a RECOVERABLE status update so the spoke UI can direct users to recovery.


[M-02] Composer admin setters allow zero critical bridge addresses

Review PhaseInitial assessment, 2026-06-24
Discovered In Commit481db73
StatusResolved

Update 2026-07-06: Fixed in commit 1f322a7. setShareOFTAdapter() and setStargatePool() now reject zero addresses. Intentional emergency disabling is handled by explicit disableShareOFTAdapter() and disableStargatePool() functions.

Description: OmniVaultComposer.setShareOFTAdapter() and OmniVaultComposer.setStargatePool() allow the admin to set critical bridge addresses to address(0). Setting either value to zero can disable or break cross-chain share returns, USDC returns, claim flows, cancel flows, and recovery flows.

Location: src/crosschain/OmniVaultComposer.sol:686-693

function setShareOFTAdapter(address shareOFTAdapter_) external override onlyRole(DEFAULT_ADMIN_ROLE) {
    address oldAdapter = shareOFTAdapter;
    if (oldAdapter != address(0)) {
        IERC20(address(vault)).forceApprove(oldAdapter, 0);
    }
    shareOFTAdapter = shareOFTAdapter_;
    emit ShareOFTAdapterUpdated(oldAdapter, shareOFTAdapter_);
}

Location: src/crosschain/OmniVaultComposer.sol:735-742

function setStargatePool(address stargatePool_) external override onlyRole(DEFAULT_ADMIN_ROLE) {
    address oldPool = stargatePool;
    if (oldPool != address(0)) {
        IERC20(vault.asset()).forceApprove(oldPool, 0);
    }
    stargatePool = stargatePool_;
    emit StargatePoolUpdated(oldPool, stargatePool_);
}

Impact: This is an admin-only issue, but a single mistaken transaction can brick cross-chain return and recovery paths until corrected. If the zero setting occurs during an active failure or recovery event, users may be forced into additional retries or manual support.

Recommendation: Reject zero addresses unless disabling is an intentional emergency feature.

if (shareOFTAdapter_ == address(0)) revert InvalidConfig();
if (stargatePool_ == address(0)) revert InvalidConfig();

If emergency disabling is desired, create explicit functions such as disableShareBridge() or pauseBridgeReturns() and document their operational effect.


[M-03] Vault-level deposit and async redeem slippage bounds are missing

Review PhaseInitial assessment, 2026-06-24
Discovered In Commit481db73
StatusResolved

Update 2026-07-13: Fixed in commit fa0a8a5. Redemption requests now include a deadline parameter. The vault stores the deadline in each request, fulfillRedeem() rejects expired requests, and batch fulfillment preflight skips expired requests. A zero deadline is explicitly allowed as no expiry.

Update 2026-07-06: Partially fixed in commit 1f322a7. Deposits now support and enforce minSharesOut, including hub-router and cross-chain deposit flows. Redemption requests now store minAssetsOut, and fulfillment reverts if the locked assets are below that bound. The recommended deadline/expiry protection was not added, so the finding remains partially resolved.

Description: User-facing deposit and redemption flows include bridge-level slippage parameters in some paths, but they do not include vault-level conversion bounds. minAmountLD protects the amount moved by Stargate or OFT, not the number of vault shares minted by deposit() or the amount of assets locked during async redemption fulfillment.

Direct hub deposits, core vault deposits, and cross-chain deposits all eventually call vault.deposit() without a user-supplied minSharesOut. Async redemption requests specify only shares; the asset amount is calculated later during operator fulfillment using the live share price at that time. There is no user-supplied minAssetsOut or deadline stored with the request.

Location: src/vault/PlutusAsyncVault.sol:450-459

function deposit(uint256 assets, address receiver)
    public
    override(ERC4626Upgradeable, IPlutusVault)
    whenNotPaused
    nonReentrant
    returns (uint256 shares)
{
    shares = super.deposit(assets, receiver);
    userDeposits += assets;
}

Location: src/routers/HubRouter.sol:72

shares = IPlutusVault(vault).deposit(assets, receiver);

Location: src/crosschain/OmniVaultComposer.sol:801-803

try vault.deposit(assets, address(this)) returns (uint256 _shares) {
    shares = _shares;
}

Location: src/vault/PlutusAsyncVault.sol:221-232

function requestRedeem(uint256 shares, address controller, address owner_)
    external
    override
    whenNotPaused
    returns (uint256 requestId)
{
    ...
    _redeemRequests[requestId] = RedeemRequest({
        controller: controller,
        owner: owner_,
        shares: shares,
        lockedAssets: 0,
        fulfilledAt: 0,
        status: RedeemStatus.PENDING
    });
}

Location: src/vault/PlutusAsyncVault.sol:589

// Assets are locked later, at fulfillment, using the then-current conversion rate.
req.lockedAssets = convertToAssets(req.shares);

Impact: Depositors can receive fewer shares than expected if the vault ratio changes between signing and execution. This can happen through NAV updates, capital accounting changes, admin deposits, or MEV around public deposit transactions. For cross-chain deposits, the risk is amplified by message latency: the user protects the bridge amount with minAmountLD, but not the vault conversion rate at the hub.

For redemptions, users request redemption by shares and receive the asset value calculated at fulfillment time. If NAV decreases before fulfillment, the user has no on-chain minAssetsOut protection or expiry condition. This may be intended for an async RWA vault, but it should be explicit and user-bounded if users rely on quoted values.

This same minSharesOut protection would also prevent the user-loss side of first-deposit inflation-style attacks. Even though direct donation inflation is structurally mitigated in this codebase, a deposit-level minSharesOut remains the correct user protection against unexpectedly low share minting.

Recommendation: Add slippage-aware entrypoints and propagate the bounds through cross-chain payloads.

For deposits:

function deposit(uint256 assets, address receiver, uint256 minSharesOut)
    external
    returns (uint256 shares)
{
    shares = deposit(assets, receiver);
    if (shares < minSharesOut) revert SlippageExceeded();
}

For cross-chain deposits, include minSharesOut in the compose payload and check it after vault.deposit(). If the check fails, credit recoverableAssets[controller][srcEid] instead of returning fewer shares than expected.

For async redemptions, store minAssetsOut and optionally deadline in the redeem request and enforce them during fulfillment:

if (block.timestamp > req.deadline) revert Expired();
if (req.lockedAssets < req.minAssetsOut) revert SlippageExceeded();

[M-04] returnNativeFee funding model is inconsistent across cross-chain flows

Review PhaseFirst mitigation review, 2026-07-06
Discovered In Commit1f322a7
StatusResolved

Update 2026-07-13: Fixed in commit fa0a8a5. SpokeRouter now validates combined LayerZero options before sending messages. _validateOptions() parses Type 3 options and requires sufficient executor native value for the relevant receive or compose option, preventing callers from declaring returnNativeFee without delivering the corresponding destination value.

Description: Spoke-side functions encode a user-provided returnNativeFee into messages sent to the hub, and hub-side handlers require the delivered msg.value to be at least that amount. However, the spoke router does not itself prove that the LayerZero options actually deliver that native value to the hub receiver or composer.

For example, claim and recovery flows build messages containing returnNativeFee, quote the outbound endpoint fee, and send the outbound message. The actual destination funding depends on correctly constructed LayerZero options rather than a contract-enforced invariant.

Location: src/routers/SpokeRouter.sol:313-327, 355-369

bytes memory message = abi.encode(OP_CLAIM, requestId, msg.sender, minAmountLD, returnNativeFee);
...
MessagingFee memory fee = endpoint.quote(params, address(this));
if (msg.value < fee.nativeFee) revert InsufficientMsgValue();
// Send message and refund excess gas
MessagingReceipt memory r = endpoint.send{ value: fee.nativeFee }(params, payable(msg.sender));

Location: src/crosschain/OmniVaultReceiver.sol:230-236

if (msg.value < returnNativeFee) revert InsufficientFee();
IOmniVaultComposer(composer).handleClaim{ value: msg.value }(requestId, caller, minAmountLD, _origin.srcEid);

Impact: Users can submit transactions that appear to include enough native value on the spoke but do not deliver the expected native value to the hub handler. This can make cancel, claim, or recovery messages revert or push assets into recoverable paths. The result is primarily liveness failure and operational friction, but in a cross-chain vault this can look like stranded user funds until recovery is completed.

Recommendation: Make the fee model explicit and enforceable. Either remove user-supplied returnNativeFee and require a pre-funded hub composer model, or validate that the supplied LayerZero options include the expected destination native drop to the correct hub contract. Hub-side code should quote the actual required return/status fee and spend exactly the quoted amount.


Low Severity

[L-01] Factory accepts zero or non-contract implementation addresses

Review PhaseInitial assessment, 2026-06-24
Discovered In Commit481db73
StatusResolved

Update 2026-07-06: Fixed in commit 1f322a7. Factory initialization and implementation setters now reject zero addresses and addresses without code for implementation and infrastructure dependencies.

Description: PlutusVaultFactory.initialize() and the implementation setter functions accept implementation and infrastructure addresses without validating that they are nonzero and contain code.

Location: src/factory/PlutusVaultFactory.sol:198-201

function setVaultImplementation(address newImpl) external override onlyOwner {
    emit ImplementationUpgraded(vaultImplementation, newImpl);
    vaultImplementation = newImpl;
}

Impact: A misconfigured factory can brick future deployments or deploy proxies pointing to invalid logic contracts. This does not directly affect already deployed vault systems, but it increases operational risk and can produce confusing deployment failures.

Recommendation: Validate all implementation addresses and critical infrastructure addresses.

if (newImpl == address(0) || newImpl.code.length == 0) revert InvalidConfig();

Apply the same pattern to vaultImplementation, oAppReceiverImplementation, composerImplementation, lzEndpoint, and stargatePool during initialization.


[L-02] ShareOFTAdapter.setComposer(address(0)) disables outbound share bridging

Review PhaseInitial assessment, 2026-06-24
Discovered In Commit481db73
StatusResolved

Update 2026-07-06: Fixed in commit 1f322a7. setComposer() now rejects address(0). Emergency disabling remains possible only through the explicit disableComposer() function.

Description: ShareOFTAdapter.setComposer() allows the owner to set composer to address(0). The adapter's _debit() function only allows outbound transfers when msg.sender == composer. If composer is zero, no normal caller can initiate outbound share bridging through the adapter.

Location: src/oft/ShareOFTAdapter.sol:56-80

function setComposer(address composer_) external onlyOwner {
    composer = composer_;
    emit ComposerSet(composer_);
}
...
function _debit(address _from, uint256 _amountLD, uint256 _minAmountLD, uint32 _dstEid)
    internal
    override
    returns (uint256 amountSentLD, uint256 amountReceivedLD)
{
    if (msg.sender != composer) revert NotComposer();
    return super._debit(_from, _amountLD, _minAmountLD, _dstEid);
}

Impact: A configuration mistake can halt cross-chain share returns from the hub. This affects deposits, cancellations, and recoveries that need to return vault shares to spoke chains.

Recommendation: Reject zero composer addresses unless disabling outbound share bridging is an intentional emergency feature. If intentional, expose an explicit disable function and document that it halts outbound adapter sends.


[L-03] liquidityBufferBps is stored but not enforced

Review PhaseInitial assessment, 2026-06-24
Discovered In Commit481db73
StatusResolved

Update 2026-07-06: Fixed in commit 1f322a7. The misleading liquidityBufferBps configuration was removed from the current vault implementation, so there is no longer an unenforced liquidity-buffer parameter.

Description: liquidityBufferBps is stored in the vault but not enforced by deployCapital(). The deployer can deploy all idle user deposits as long as amount <= userDeposits.

Location: src/vault/PlutusAsyncVault.sol:325-334

function deployCapital(address to, uint256 amount) external override onlyRole(DEPLOYER_ROLE) {
    if (amount == 0) revert ZeroAmount();
    if (to == address(0)) revert InvalidConfig();
    if (amount > userDeposits) revert InsufficientLiquidity();
    uint256 id = ++deploymentCounter;
    userDeposits -= amount;
    totalAssetsManaged += amount;
    IERC20(asset()).safeTransfer(to, amount);
    emit CapitalDeployed(id, to, amount);
}

Impact: If users or operators believe liquidityBufferBps is a hard liquidity invariant, the vault can be configured in a way that does not match expectations. All idle liquidity can be deployed, leaving redemptions dependent on operator repayment during fulfillment.

The code currently documents the parameter as informational, so this is primarily a design and documentation issue.

Recommendation: Either enforce the buffer or rename/document it as purely informational. If it is intended to be enforced, add a check similar to:

uint256 minIdle = (totalVaultAssets() * liquidityBufferBps) / 10_000;
if (userDeposits - amount < minIdle) revert InsufficientLiquidityBuffer();

[L-04] Claim and fulfill quote helpers use live share price instead of locked assets

Review PhaseInitial assessment, 2026-06-24
Discovered In Commit481db73
StatusResolved

Update 2026-07-06: Fixed in commit 1f322a7. Quote helpers now use req.lockedAssets for requests that are already CLAIMABLE or CLAIMED, and only use live convertToAssets(req.shares) for pending estimates.

Description: Some quote helpers estimate claim or fulfillment return amounts using convertToAssets(req.shares). For already fulfilled requests, the payout amount is locked in req.lockedAssets at fulfillment time. If share price changes after fulfillment, these helpers can quote the wrong amount.

Location: src/routers/HubRouter.sol:256-259

IPlutusVault.RedeemRequest memory req = IPlutusVault(hubVault).getRedeemRequest(requestId);
uint256 grossAssets = IPlutusVault(hubVault).convertToAssets(req.shares);
uint256 fee_ = (grossAssets * IPlutusVault(hubVault).feeBps()) / 10_000;
uint256 assets = grossAssets - fee_;

Impact: Users or operators can receive inaccurate fee quotes for claim and fulfillment return legs. This can cause avoidable reverts, overpayment, or underpayment of cross-chain fees.

Recommendation: Use req.lockedAssets whenever req.status is CLAIMABLE or CLAIMED. Use live convertToAssets(req.shares) only for PENDING requests and label the result as an estimate.


[L-05] Admin and upgrade trust model is highly centralized

Review PhaseInitial assessment, 2026-06-24
Discovered In Commit481db73
StatusOpen

Description: OVaultRWA uses UUPS upgradeability and admin-controlled cross-chain wiring. Privileged accounts can upgrade implementations, change bridge addresses, set peers, update NAV in emergencies, and manage capital deployment roles.

This is not necessarily a code bug if the protocol is deployed with proper operational controls. However, the blast radius of a single privileged-key compromise is severe. For example, a compromised owner or admin can upgrade implementations, reconfigure cross-chain bridge endpoints, or grant roles that move idle vault assets. A compromised DEPLOYER_ROLE can call deployCapital() and transfer idle user deposits to an arbitrary address.

Impact: Exploitation requires operational failures such as assigning privileged roles to an EOA, losing a multisig threshold, misconfiguring deployment ownership, or approving an unsafe upgrade. Those conditions are less likely than a permissionless exploit, but if they occur the impact can be severe and may include full vault compromise, asset loss, NAV corruption, or cross-chain asset stranding.

The severity is Low under the assumption that privileged roles are held by a properly secured multisig and upgrade process. If any owner, admin, or deployer role is held by a single EOA in production, this risk should be treated as High.

Recommendation: Use multisigs and timelocks for owner/admin roles, monitor implementation slot changes, monitor role changes, and publish an emergency response runbook. For high-value deployments, require pre-announced upgrades with bytecode verification before execution. Add deployment-script assertions that verify no critical role is left with a single EOA unless explicitly accepted by governance.


[L-06] _handleRedeem() does not reject zero return addresses

Review PhaseFirst mitigation review, 2026-07-06
Discovered In Commit1f322a7
StatusResolved

Update 2026-07-13: Fixed in commit fa0a8a5. _handleRedeem() now reverts with InvalidConfig() when returnAddress == address(0), preventing zero-address pending redeem state from being created.

Description: OmniVaultComposer._handleRedeem() does not explicitly reject returnAddress == address(0). If a malformed compose message is accepted with a zero return address, the request can be stored with returnAddress equal to zero. Later handlers treat returnAddress == address(0) as meaning that no pending redeem exists.

Location: src/crosschain/OmniVaultComposer.sol:955-996

function _handleRedeem(
    address returnAddress,
    uint256 shares,
    uint256 minAssetsOut,
    uint32 srcEid,
    bytes32 guid,
    bytes calldata message,
    uint256 returnNativeFee
) internal {
    ...
    pendingRedeems[requestId] = PendingRedeemReturn({ srcEid: srcEid, returnAddress: returnAddress });

Impact: Malformed cross-chain redeem messages can create awkward or stuck state that normal claim/cancel paths cannot handle cleanly. The issue is most likely caused by integration or message construction error rather than a profitable attacker path.

Recommendation: Reject zero return addresses at the start of _handleRedeem().

if (returnAddress == address(0)) revert InvalidConfig();

[L-07] Factory deployment should prevalidate critical deployment parameters

Review PhaseFirst mitigation review, 2026-07-06
Discovered In Commit1f322a7
StatusResolved

Update 2026-07-13: Fixed in commit fa0a8a5. deployVault() now validates the asset, owner, operator, and fulfiller before deploying proxies. Invalid inputs revert before any CREATE2 deployment is attempted.

Description: PlutusVaultFactory.deployVault() relies on downstream initializers and role-management calls for some validation, but it should prevalidate critical deployment parameters before deploying proxies. Parameters such as asset, owner, operator, and fulfiller should be checked explicitly at the top of the deployment function.

Location: src/factory/PlutusVaultFactory.sol:111-122

function deployVault(
    address asset,
    string calldata name,
    string calldata symbol,
    address owner,
    address operator,
    address fulfiller,
    address shareOFTAdapter,
    uint16 feeBps,
    address treasury,
    bytes32 salt
) external override onlyOwner returns (address vaultProxy, address oAppReceiverProxy, address composerProxy) {

Impact: Bad deployment input can waste gas, create confusing partial failure modes, or leave operational roles misconfigured until an admin repair transaction is performed. This is primarily deployment-hardening risk rather than a direct runtime exploit.

Recommendation: Add explicit validation before any CREATE2 proxy deployment or role grant.

if (asset == address(0) || asset.code.length == 0) revert InvalidConfig();
if (owner == address(0)) revert InvalidConfig();
if (operator == address(0)) revert InvalidConfig();
if (fulfiller == address(0)) revert InvalidConfig();

[L-08] returnNativeFee overpayment is not consistently refunded or accounted

Review PhaseFirst mitigation review, 2026-07-06
Discovered In Commit1f322a7
StatusResolved

Update 2026-07-13: Fixed in commit fa0a8a5. The redeem compose path now sends exactly the quoted status-update fee to the receiver and credits any excess native value to gasFeeRefunds[returnAddress][srcEid]. Failure paths also credit unused ETH to the refund mapping.

Description: Some paths forward user-provided returnNativeFee or msg.value into status-update calls instead of always spending the exact quoted native fee and deterministically refunding or crediting the remainder. This creates inconsistent behavior between deposit, redeem, cancel, claim, recovery, and fulfillment flows.

One representative path forwards the embedded returnNativeFee to the status update instead of spending the quoted status fee exactly:

Location: src/crosschain/OmniVaultComposer.sol:993-1004

try vault.requestRedeem(shares, address(this), address(this), minAssetsOut) returns (uint256 requestId) {
    // If successful, store the pending return details for later fulfillment.
    pendingRedeems[requestId] = PendingRedeemReturn({ srcEid: srcEid, returnAddress: returnAddress });
    // Send status update back to the spoke to set status to PENDING.
    // The real requestId is now known; build the actual status message and send it.
    bytes memory statusMsg = abi.encode(uint8(0x07), requestId, IPlutusAsyncVault.RedeemStatus.PENDING, returnAddress);
    IOmniVaultReceiver(oAppReceiver).sendStatusUpdate{ value: returnNativeFee }(srcEid, statusMsg, statusOptions);
    emit CrossChainRedeemRequested(returnAddress, requestId, shares, srcEid);
}

The safer invariant is:

  1. quote the required native fee on the hub,
  2. send exactly that amount to the external messaging call, and
  3. credit or refund every excess wei.

Impact: Overpayments can become difficult to reason about and may accumulate in unexpected contracts or paths. This is not a direct fund-drain issue, but it creates accounting ambiguity for native fee refunds and makes frontend/operator fee handling more error-prone.

Recommendation: Standardize all return-fee handling across the cross-chain flows. For every status update or bridge send, spend exactly the quoted nativeFee. Any excess should be either returned immediately or credited to gasFeeRefunds[user][srcEid] with an event. Add tests for underpayment, exact payment, and overpayment on every cross-chain action.


[L-09] Vault accounting assumes non-fee-on-transfer underlying assets

Review PhaseSecond mitigation review, 2026-07-13
Discovered In Commitfa0a8a5
StatusOpen

Note: This issue was present in earlier reviewed versions. It became easier to identify during the second mitigation review because the review focused on residual accounting assumptions after the redemption and cross-chain fee fixes.

Description: PlutusAsyncVault accounts deposits and capital returns using the requested transfer amount rather than the actual received token balance delta. This is safe for standard ERC20 tokens such as USDC, but it is unsafe for fee-on-transfer, rebasing, or otherwise non-standard tokens.

Deposit and mint accounting credit userDeposits by the requested or computed asset amount:

Location: src/vault/PlutusAsyncVault.sol:494-518

shares = super.deposit(assets, receiver);
if (shares < minSharesOut) revert SlippageExceeded();
userDeposits += assets;
...
assets = super.mint(shares, receiver);
userDeposits += assets;

Operational capital-return paths use the same requested-amount accounting pattern:

Location: src/vault/PlutusAsyncVault.sol:373-388

IERC20(asset()).safeTransferFrom(msg.sender, address(this), amountReturned);
totalAssetsManaged -= amountReturned;
userDeposits += amountReturned;
...
IERC20(asset()).safeTransferFrom(msg.sender, address(this), amount);
userDeposits += amount;

If the underlying token charges a transfer fee, the vault may receive less than the amount it records internally. Since totalAssets() is based on internal accounting, the vault can overstate assets and inflate share price relative to its actual token balance.

Impact: For the intended USDC-based deployment model, this is mainly an asset-assumption issue. If the factory is used to deploy vaults for arbitrary ERC20 tokens, a fee-on-transfer asset can break vault accounting and cause share price distortion.

Recommendation: Explicitly restrict supported vault assets to non-rebasing, non-fee-on-transfer ERC20s. Ideally enforce this through an approved asset allowlist or require asset to match the configured Stargate-compatible USDC asset. Document this assumption clearly in deployment and integration docs.


[L-10] Accidental native ETH can become stuck in the vault during non-cross-chain fulfillment

Review PhaseSecond mitigation review, 2026-07-13
Discovered In Commitfa0a8a5
StatusResolved

Update 2026-07-15: Fixed in commit 6177fb5. fulfillRedeem() and batchFulfillRedeem() now refund attached native ETH to msg.sender when no cross-chain notification is sent. The test suite includes successful refund cases and refund-failure reverts.

Note: This issue was present in earlier reviewed versions. The latest batch-fulfillment changes make it easier to trigger and detect because invalid cross-chain requests can now be skipped instead of reverting, allowing a payable batch transaction to succeed without forwarding msg.value to the composer.

Description: PlutusAsyncVault.fulfillRedeem() and batchFulfillRedeem() are payable because cross-chain fulfillments forward msg.value to the composer for LayerZero status updates. However, if no cross-chain notification is sent, attached ETH is not refunded.

In fulfillRedeem(), ETH is only forwarded if the request controller is the composer:

Location: src/vault/PlutusAsyncVault.sol:238-247

_fulfillRedeem(requestId);

if (_redeemRequests[requestId].controller == composer) {
    uint256[] memory ccRequestIds = new uint256[](1);
    ccRequestIds[0] = requestId;
    IOmniVaultComposer(composer).notifyFulfillment{ value: msg.value }(ccRequestIds, msg.sender);
}

If the request is hub-direct and msg.value > 0, the ETH remains in the vault.

In batchFulfillRedeem(), ETH is only forwarded if at least one claimable request is cross-chain:

Location: src/vault/PlutusAsyncVault.sol:252-289

uint256 ccCount;
for (uint256 i; i < requestIds.length; i++) {
    if (isClaimable[i] && _redeemRequests[requestIds[i]].controller == composer) {
        ccCount++;
    }
}
...
if (ccCount > 0) {
    ...
    IOmniVaultComposer(composer).notifyFulfillment{ value: msg.value }(ccRequestIds, msg.sender);
}

If the batch contains only hub-direct requests, or if all cross-chain requests are skipped by preflight, msg.value remains in the vault. The vault has no native ETH withdrawal or refund path.

Impact: This does not affect ERC20 vault funds, but operators can accidentally strand native ETH in the vault. The latest batch-fulfillment behavior makes this more reachable because invalid cross-chain requests can now be skipped instead of reverting, allowing a transaction with attached ETH to succeed without forwarding or refunding that ETH.

Recommendation: Refund msg.value whenever no cross-chain notification is sent, or reject native ETH in non-cross-chain fulfillment paths.

if (ccCount == 0 && msg.value > 0) {
    (bool ok,) = msg.sender.call{ value: msg.value }("");
    if (!ok) revert EthRefundFailed();
}

Similarly, in fulfillRedeem(), if the request controller is not the composer and msg.value > 0, either refund it or revert.


Informational Severity

[I-01] Technical report is materially stale relative to the current implementation

Review PhaseInitial assessment, 2026-06-24
Discovered In Commit481db73
StatusPartially Resolved

Update 2026-07-15: Partially fixed in commit 6177fb5. Historical design/report files are now marked as historical, reducing the risk that stale implementation notes are treated as current specification. The finding remains partially resolved because docs/PLUTUS_OVAULT_TECHNICAL_REPORT.md still contains a stale note stating that the IPlutusAsyncVault.RedeemStatus.CLAIMED enum comment references share burn at claim, while the interface comment has been updated to state that shares are burned at fulfillRedeem.

Update 2026-07-13: Partially fixed in commit fa0a8a5. Backend-flow documentation and the stale vault interface share-burn comment have been updated. Some historical docs/v2_report.md and plans/ files were explicitly marked as historical.

Update 2026-07-06: Partially fixed in commit 1f322a7. The main technical report and frontend-flow documentation now reflect much of the current implementation, including minSharesOut, minAssetsOut, locked redemption assets, share burn at fulfillment, and the separate recovery buckets.

Description: At the time of the initial review, docs/PLUTUS_OVAULT_TECHNICAL_REPORT.md did not match the implementation in several material areas, including vault accounting, redemption burn timing, fee handling, capital settlement, recovery structure, role model, composer fulfiller model, and cross-chain fee responsibility.

Most of those discrepancies have since been corrected in the current technical and flow documentation. The remaining current-documentation issue identified during the latest review is a stale note in docs/PLUTUS_OVAULT_TECHNICAL_REPORT.md that states the IPlutusAsyncVault.RedeemStatus.CLAIMED enum comment still references share burn at claim. The interface comment has already been corrected to say shares are burned at fulfillRedeem, so the note itself is now stale.

Older docs/v2_report.md and plans/ materials still contain historical implementation details such as recoverableIsShares, ComposerEthLow, and minEthBalance, but these files are now explicitly marked as historical design/report material rather than the authoritative specification.

Impact: Integrators and operators following the report can build incorrect fee flows, incorrect recovery UIs, or incorrect operational runbooks. Documentation drift is a real security issue in a cross-chain system because mistakes often result in stuck messages or stranded assets.

Recommendation: Remove or update the remaining stale note in the current technical report, and keep the historical warnings on legacy design/report files. Treat the updated report as a versioned specification and add a documentation-consistency review to the release process.


[I-02] ERC4626 first-deposit, rounding, and conversion-ordering exploit classes were reviewed

Review PhaseInitial assessment, 2026-06-24
Discovered In Commit481db73
StatusMitigated / Not Present

Description: Several common ERC4626 vault exploit classes were reviewed against the current OVaultRWA implementation:

  1. first-deposit inflation via attacker dust deposit plus direct donation;
  2. user-favorable rounding loops that drain vault dust;
  3. conversion calculations that read post-mint, post-burn, or post-transfer state in the same call.

The classic first-deposit direct-donation attack is structurally mitigated because PlutusAsyncVault.totalAssets() does not read the raw ERC20 balance. It returns internal accounting instead:

Location: src/vault/PlutusAsyncVault.sol:513-516

function totalVaultAssets() public view override returns (uint256) {
    return userDeposits + totalAssetsManaged;
}

A direct USDC transfer to the vault therefore does not increase the ERC4626 conversion denominator. The inherited OpenZeppelin ERC4626 implementation also uses virtual asset/share math:

Location: node_modules/@openzeppelin/contracts-upgradeable/token/ERC20/extensions/ERC4626Upgradeable.sol:270-278

return assets.mulDiv(totalSupply() + 10 ** _decimalsOffset(), totalAssets() + 1, rounding);
return shares.mulDiv(totalAssets() + 1, totalSupply() + 10 ** _decimalsOffset(), rounding);

Rounding is also vault-favorable through OpenZeppelin defaults: previewDeposit() rounds shares down, previewMint() rounds assets up, previewWithdraw() rounds shares up, and previewRedeem() rounds assets down. The standard synchronous ERC4626 withdraw() and redeem() paths are disabled in PlutusAsyncVault, which further removes the usual deposit/withdraw loop surface.

Conversion ordering is handled correctly in the core paths. Deposits compute shares through OpenZeppelin preview logic before transfer and mint, and async redemption fulfillment computes req.lockedAssets = convertToAssets(req.shares) before burning shares or mutating accounting buckets. Claims then use req.lockedAssets instead of recomputing live conversion.

Impact: These three exploit classes were not found as active vulnerabilities in the reviewed code. However, the lack of minSharesOut and minAssetsOut described in [M-03] remains relevant: slippage bounds are still needed to protect users and integrations from unexpectedly poor conversion rates, even when the classic first-deposit donation attack is structurally mitigated.

Recommendation: Keep the internal-accounting totalAssets() model and OpenZeppelin ERC4626 conversion behavior. Add regression tests for:

  • direct donations before the first deposit;
  • deposits that would mint zero shares;
  • full supply redemption with dust remaining;
  • conversion ordering during deposit, fulfillment, and claim.

Also implement [M-03] so users can set minSharesOut, minAssetsOut, and deadlines where appropriate.


[I-03] Drifting code style reduces readability and increases review risk

Review PhaseInitial assessment, 2026-06-24
Discovered In Commit481db73
StatusOpen

Description: The codebase contains inconsistent formatting patterns across and within files. This includes inconsistent multiline wrapping, struct literals that are sometimes one-field-per-line and sometimes compressed into a single long line, long function signatures using different wrapping styles, and minor whitespace drift.

One clear example is PlutusVaultFactory.deployVault(), where three near-identical proxy deployments are formatted in three different ways:

Location: src/factory/PlutusVaultFactory.sol:116-142

vaultProxy = address(
    new ERC1967Proxy{ salt: vaultSalt }(
        vaultImplementation,
        abi.encodeCall(
            PlutusAsyncVault.initialize,
            (IERC20(asset), name, symbol, address(this), fulfiller, feeBps, liquidityBufferBps, treasury)
        )
    )
);

oAppReceiverProxy = address(
    new ERC1967Proxy{ salt: receiverSalt }(
        oAppReceiverImplementation, abi.encodeCall(OmniVaultReceiver.initialize, (address(1), address(this)))
    )
);

composerProxy = address(
    new ERC1967Proxy{ salt: composerSalt }(
        composerImplementation,
        abi.encodeCall(
            OmniVaultComposer.initialize, (lzEndpoint, oAppReceiverProxy, vaultProxy, stargatePool, shareOFTAdapter, address(this))
        )
    )
);

The issue is not that this formatting directly breaks execution. The problem is auditability. When equivalent logic is formatted differently, reviewers spend more effort proving that the code paths are actually equivalent. This increases the chance that developers introduce bugs and that auditors miss meaningful differences or vulnerabilities. In cross-chain code, where small payload or parameter differences can strand funds, readability is part of the security posture.

Additional representative examples include src/routers/SpokeRouter.sol:137, src/routers/SpokeRouter.sol:191-195, src/routers/SpokeRouter.sol:530-533, and src/crosschain/OmniVaultReceiver.sol:222.

Impact: Inconsistent style can hide subtle differences in parameter ordering, message encoding, access-control checks, and bridge configuration. This does not create a direct exploit by itself, but it increases the probability of future implementation mistakes and review misses.

Recommendation: Enforce formatting in CI with:

forge fmt --check src

Adopt consistent rules for:

  • one field per line in struct literals;
  • one argument per line for long calls;
  • consistent multiline function signatures;
  • no trailing whitespace;
  • consistent grouping of repeated deployment or message-construction patterns.

Disclaimer

This report is provided for informational purposes only and does not constitute investment advice. The security assessment was conducted using currently available tools and knowledge. While every effort has been made to identify potential vulnerabilities, this report does not guarantee that the smart contracts are free from all bugs or security risks. The auditor assumes no liability for any loss of funds or damages resulting from the use of the audited code.