[A23-6] [LOW] LibFlow.flow: reentrancy surface not enumerated in NatSpec

[thedavidmeister]

Description

LibFlow.flow's NatSpec says:

/// 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.

The "DOES NOT prevent reentrancy attacks. This is the responsibility of the caller" line is correct — the only concrete caller, Flow.flow in src/concrete/Flow.sol:144-149, applies nonReentrant. But the NatSpec does not name the specific reentrancy surface that the caller must guard against:

1. ERC721 safeTransferFrom invokes onERC721Received on the recipient.
2. ERC1155 safeTransferFrom invokes onERC1155Received on the recipient.
3. ERC20 safeTransfer/safeTransferFrom are non-reentrant for compliant tokens, but ERC777 (which presents an ERC20 interface) invokes tokensToSend on the sender and tokensReceived on the recipient via the ERC1820 registry.
4. interpreterStore.set is an external call to a contract whose address is determined at flow-deploy time by the deployer.

A future contract that uses LibFlow directly without nonReentrant on its entrypoint would be exploitable via any of these. The current NatSpec ("responsibility of the caller") gestures at this but does not enumerate it, which means a future caller has to re-derive the surface from scratch.

This is a documentation defect, not a code defect — Flow.sol is correctly guarded. But for a security-critical library marked "DOES NOT prevent reentrancy attacks", the surface needs to be explicit.

File:Line

src/lib/LibFlow.sol:134-150

Proposed fix

Diff:

     /// 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.
+    /// interpreter store. Then processes the ERC20, ERC721 and ERC1155 transfers
+    /// in the flow. Guarantees ordering of the transfers but DOES NOT prevent
+    /// reentrancy attacks. The caller MUST apply a reentrancy guard around any
+    /// entrypoint that reaches this function. The reentrancy surface is:
+    /// 1. `interpreterStore.set` — external call to an arbitrary store
+    ///    contract chosen by the flow deployer.
+    /// 2. ERC721 `safeTransferFrom` — invokes `onERC721Received` on a contract
+    ///    recipient.
+    /// 3. ERC1155 `safeTransferFrom` — invokes `onERC1155Received` on a
+    ///    contract recipient.
+    /// 4. ERC20 `safeTransfer` / `safeTransferFrom` — non-reentrant for
+    ///    compliant ERC20s, but ERC777 (which presents an ERC20 interface)
+    ///    invokes `tokensToSend` on the sender and `tokensReceived` on the
+    ///    recipient via the ERC1820 registry.
     /// @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.