Omniscia Sova Network Audit

ManagedWithdrawRWA Manual Review Findings

ManagedWithdrawRWA Manual Review Findings

MWR-01M: Incorrect Order of Operations

Description:

In contrast to the tRWA implementation, the IHook::onBeforeWithdraw implementations are invoked before the state is mutated in the ManagedWithdrawRWA::batchRedeemShares function creating a significant inconsistency and data discrepancy for hook implementations.

Impact:

Hooks that expect state changes to have been applied (for example, to enforce a minimum balance) will misbehave when the ManagedWithdrawRWA implementation is utilized.

Example:

src/token/ManagedWithdrawRWA.sol
108// Execute each individual withdrawal (includes hooks)
109for (uint256 i; i < len; ++i) {
110    uint256 userShares = shares[i];
111    uint256 userAssets = assets[i];
112    address recipient = to[i];
113    address shareOwner = owner[i];
114

115    // Call hooks (same logic as in _withdraw)
116    for (uint256 j; j < opHooks.length; ++j) {
117        IHook.HookOutput memory hookOut =
118            opHooks[j].hook.onBeforeWithdraw(address(this), strategy, userAssets, recipient, shareOwner);
119        if (!hookOut.approved) revert HookCheckFailed(hookOut.reason);
120        opHooks[j].hasProcessedOperations = true;
121    }
122

123    // Accounting and transfers (mirrors _withdraw logic sans nonReentrant)
124    if (strategy != shareOwner) _spendAllowance(shareOwner, strategy, userShares);
125    _beforeWithdraw(userAssets, userShares);
126    _burn(shareOwner, userShares);
127

128    SafeTransferLib.safeTransfer(asset(), recipient, userAssets);
129    emit Withdraw(strategy, recipient, shareOwner, userAssets, userShares);
130}

Recommendation:

We advise the code to mimic the order of operations in the tRWA::_withdraw function, executing the hooks after the burn operation but before the asset transfer toward the recipient.

Alleviation (e4d6885477438291b0d3ca477fab7e088522967b):

The function's statements were re-ordered per our recommendation, ensuring local state changes relating to the burn operation are applied prior to executing the relevant hooks that are meant to perform validation.

Prev

GatedMintEscrow.sol (GME-M)

Next

ManagedWithdrawRWAStrategy.sol (MWW-M)

On this page