[A23-3] [MEDIUM] LibFlow.flow: asymmetric namespace qualification trust boundary with interpreter store

[thedavidmeister]

Description

LibFlow.flow passes DEFAULT_STATE_NAMESPACE (an unqualified StateNamespace) directly to interpreterStore.set:

function flow(FlowTransferV1 memory flowTransfer, IInterpreterStoreV2 interpreterStore, uint256[] memory kvs)
    internal
{
    if (kvs.length > 0) {
        interpreterStore.set(DEFAULT_STATE_NAMESPACE, kvs);
    }
    ...
}

This is correct given the IInterpreterStoreV2 contract: set is documented as taking the unqualified namespace and the store implementation MUST qualify it by msg.sender internally. The store qualifies by hashing (stateNamespace, msg.sender) (see LibNamespace.qualifyNamespace), and msg.sender from the store's perspective is the Flow contract — so per-Flow-clone isolation holds.

However, on the read path in Flow._flowStack, the Flow contract itself explicitly qualifies the namespace before passing it to interpreter.eval2:

DEFAULT_STATE_NAMESPACE.qualifyNamespace(address(this)),

The asymmetry — the read path qualifies in-contract, the write path delegates qualification to the store — is correct under the current IInterpreterV2.eval2 / IInterpreterStoreV2.set interface (eval2 takes a FullyQualifiedNamespace, set takes a StateNamespace). But it is non-obvious from LibFlow.flow alone that the store will qualify by address(this) rather than by the EOA caller. A reviewer chasing the trust boundary has to fetch the store interface and then trust every concrete store implementation to obey the qualification contract.

This is not a bug today (the interface mandates the qualification, and the in-tree LibNamespace usage in Flow._flowStack is consistent), but it is a fragile boundary: any future store implementation that fails to qualify by msg.sender would silently leak state across Flow clones, with no defence-in-depth in LibFlow.

File:Line

src/lib/LibFlow.sol:141-150 (the set call)
src/concrete/Flow.sol:258-265 (the asymmetric eval2 qualification)

Proposed fix

Add a NatSpec line in LibFlow.flow that names the trust assumption explicitly, and consider passing an already-qualified namespace via a future IInterpreterStoreV3.set(FullyQualifiedNamespace, kvs) so the asymmetry is removed at the type level.

Short-term diff (NatSpec only — does not remove the underlying fragility, only flags it):

     /// Processes a flow transfer. Firstly sets state for the interpreter on the
     /// interpreter store. Then processes the ERC20, ERC721 and ERC1155 transfers
     /// in the flow. Guarantees ordering of the transfers but DOES NOT prevent
     /// reentrancy attacks. This is the responsibility of the caller.
+    /// Passes the unqualified `DEFAULT_STATE_NAMESPACE` to `interpreterStore.set`.
+    /// Per the `IInterpreterStoreV2` contract, the store MUST qualify this with
+    /// the caller (this Flow contract clone) before persisting — this is what
+    /// gives per-clone state isolation. Stores that do not qualify will leak
+    /// state across all callers using `DEFAULT_STATE_NAMESPACE`. The matching
+    /// read-side qualification is performed in `Flow._flowStack` via
+    /// `LibNamespace.qualifyNamespace(address(this))`.
     /// @param flowTransfer The `FlowTransferV1` to process.
     /// @param interpreterStore The `IInterpreterStoreV1` to set state on.
     /// @param kvs The key value pairs to set on the interpreter store.
     function flow(FlowTransferV1 memory flowTransfer, IInterpreterStoreV2 interpreterStore, uint256[] memory kvs)

Long-term: bump IInterpreterStoreV to take FullyQualifiedNamespace on set, mirroring get/eval2, and have LibFlow.flow call DEFAULT_STATE_NAMESPACE.qualifyNamespace(address(this)) before set. This removes the cross-contract trust requirement entirely.

[thedavidmeister]

Dismissed: the asymmetry is the security design and is already documented in the upstream IInterpreterStoreV2 NatSpec — set takes an unqualified StateNamespace precisely so the store can auto-qualify by msg.sender and prevent callers from writing to namespaces they don't control; get / eval2 take FullyQualifiedNamespace because reads across namespaces are non-destructive and intentionally allowed. Pre-qualifying writes in LibFlow.flow would re-introduce the cross-caller spoofing surface the unqualified-write design eliminates. IInterpreterStoreV3 upstream keeps the same shape.