Kiln Phase 3

Description

The FeeManager is partially incompatible with the TransferMode.BLOCK_TRANSFER and the cShares fee type.

The FeeManager receives two types of fees:

• cShares when performance/management fees are minted or when the unlock action is performed for DEPOSIT operations
• assets when the unlock action is performed for REDEEM operations

The FeeManager acts like a "bucket" that then needs to "distribute" all the fees collected to the fee recipient.

The current logic of the FeeManager to perform such a task is to execute FeeManager.dispatchERC20 which will dispatch an arbitrary asset to all the fee recipients it has been configured with.

When the asset (input parameter) is the cShare, and the Conduit is configured with TransferMode.BLOCK_TRANSFER the FeeManager.dispatchERC20 operation will revert.

This means that all the cShares minted to the FeeManager as fees are locked forever into the FeeManager

Recommendation

Kiln must brainstorm a solution to solve this incompatibility between the FeeManager and the Conduit's transfer mode TransferMode.BLOCK_TRANSFER.

Note that the same problem arises when the Conduit is configured with the transfer mode TransferMode.ACCOUNT_LIST and the fee recipient of the FeeManager has not been configured into AccountList.

Kiln

Fixed in commit 70c70bd2

cShare fees accrue to the FeeManager as the conduit's own ERC20 (ongoing fees and transactional deposit fees), and FeeManager.dispatchERC20(cShare) dispatches them via conduit.transfer(FeeManager -> recipient). Before this fix, the only system-move bypass in the transfer gate exempted the conduit itself but not the FeeManager, so the dispatch was gated by _transferAllowed: it reverted while transferEnabled == false, and under AccountList screening when the recipient/FeeManager was blocked/sanctioned or (in STRICT) not allow-listed — stranding the accrued fee cShares until transfers were enabled / the recipient made transfer-eligible. The fix: cShare transfer compliance is now enforced at the public transfer/transferFrom entrypoints (the _update counterparty gate is removed), and the FeeManager is exempt as a SENDER only (from == feeManager). FeeManager.dispatchERC20(cShare) performs conduit.transfer(FeeManager -> recipient) with from == FeeManager, so it now passes regardless of transferEnabled/AccountList; screening rests on keeping the fee recipients clean (not blocked/sanctioned; allow-listed in STRICT). All conduit-internal fee accrual/distribution uses the internal _mint/_transfer and is unaffected (it never traversed the gate). The exemption is deliberately sender-only: a sanctioned/blocked holder cannot transfer/transferFrom cShares TO the FeeManager — or to the conduit — to move them unscreened, closing the symmetric to == feeManager / to == conduit bypass a first-pass _update counterparty exemption would have left open.

Spearbit

While the fix above resolves the original stranding issue, the sender-only exemption (from == feeManager) raises a new concern: it bypasses recipient-side compliance screening on fee dispatches, shifting it to an operational assumption about fee recipients. This is covered in the fix-review report. Kiln has since submitted a follow-up fix (PR 459, commit ef8e1d0e) that respects !ready() and screens the destination; it has not been reviewed by Spearbit and should be verified in the upcoming engagement.

Description

The current behavior of the OwnerRegistry.wrap, OwnerRegistry._update and Conduit.isTransferable does not perform any validation on the Query that is wrapped or transferred to another account.

Without any validation the owner of the query could be able to wrap it and "sell/transfer" the NFT (wrapped Query) to another user that would not be able to retrieve any cShare or asset given that the query is already in the final state (FINALIZED or REJECTED).

Recommendation

Kiln should perform the following refactoring and validations:

• OwnerRegistry.wrap should revert if the query state is in a final state
• OwnerRegistry._update should pass to conduit.isTransferable the inputs needed to validate the query
• Conduit.isTransferable should revert if the wrapped query (the NFT) is in a final state

Note: we are not suggesting to validate the EMPTY state only because right now it's not possible to wrap a query that is in such a state. To wrap it, the query must have been first registered by the Conduit via the registerOwner function which is only called by the Conduit when a query has been successfully created on the underlying vehicle.

Kiln

Fixed in commit e3b2d69b

Fixed on-chain. We reversed the earlier off-chain-exchange position and now ensure a finalized query never has a live, transferable wrapped NFT. OwnerRegistry.wrap takes the Query and reverts NonWrappableState if the query's vehicle state is EMPTY/SETTLED/REJECTED, so an already-finalized query cannot be wrapped. A new OwnerRegistry.unwrap(Id) — namespace-keyed on the calling conduit (computeHashId(msg.sender, queryId), so a caller can only affect its own queries' NFTs) — burns the wrapped NFT, and Conduit._process calls it when a query reaches SETTLED/REJECTED, after the output is distributed to the holder, so a query that finalizes while wrapped has its NFT burned (OwnerRegistry._update exempts burns from the transfer gate). Net effect: wrapping a finalized query reverts, and an in-flight wrapped query's NFT is destroyed on finalization, so the spent claim cannot be transferred or sold. We kept IConduit.isTransferable(address,address) unchanged (no query/Id threading) because the protection is achieved by the wrap-revert + burn-on-finalize rather than by gating transfers. This closes the finalized/terminal case; the partially-unlocked case is tracked separately as #10 (non-terminal — would require on-chain query-completion views) and remains acknowledged.

Spearbit

The commit e3b2d69b performs the following changes:

• the OwnerRegistry.wrap function reverts if the query's state (fetched from the vehicle) is EMPTY or in a final state (SETTLED or REJECTED): the caller cannot wrap a non-existing or already finalized query and then transfer it
• the Conduit._process function calls OwnerRegistry.unwrap once the progressed query is in a finalized state (SETTLED or REJECTED). The OwnerRegistry.unwrap function will "silently" (without reverting if the NFT does not exist) burn the NFT. By performing this operation no one will be able to transfer the NFT once the query is finalized.

⚠️ Note: because the OwnerRegistry.unwrap function does burn the NFT and reset the $.queries and $.tokenToHashId data structure for the relative query burned, we are also losing the "history" of ownership of the wrapped query and all the data stored in those mappings. The OwnerRegistry.getOwner will return address(0) as the owner even if the query was registered in the past.

Description

After reviewing the Conduit behavior relative to the query creation (deposit/redeem operations), the Conduit's share transfer and the Query NFT transfer (OwnerRegistry) we have identified multiple conflicts between the implementation behavior and the defined specifications.

Kiln has provided the following new specifications that the Conduit should follow

1. Scenario transfer mode == ACCOUNT_LIST

• if AccountList is in ALLOW mode
  • in create, msg.sender needs to be in the list and not in sanction list
  • in create, receiver needs to be in the list and not in sanction list
  • in transfers, from needs to be in the list and not in sanction list
  • in transfers, to needs to be in the list and not in sanction list
  • system transfers are allowed (address(0) <-> CONDUIT)
• if AccountList is in BLOCK mode
  • in create, msg.sender must not be in the list or sanction list
  • in create, receiver must not be in the list or sanction list
  • in transfers, from must not be in the list and not in sanction list
  • in transfers, to must not be in the list and not in sanction list
  • system transfers are allowed (address(0) <-> CONDUIT)

2. Scenario transfer mode == BLOCK_TRANSFER
   • in create, msg.sender must be equal to receiver
   • in transfers, we only allow system transfers (address(0) <-> CONDUIT)
3. Scenario transfer mode == ALLOW_TRANSFER: in this scenario everything is allowed

Note: the above scenarios apply to:

• Conduit's shares (direct transfer of Conduit's shares or create in DEPOSIT mode)
• Underlying assets (create in REDEEM mode)
• OwnerRegistry NFT transfer

We suggest the following changes:

Structure and Natspec

1. Rename the transferMode state variable name to allowMode (or something similar). The value could manage both the permission relative to the creation of the Conduit's query, the transferability of the Query's NFT ownership and the transferability of the Conduit's shares or underlying assets.
2. Rename the TransferMode struct to AllowMode (or something similar)
3. Rename TransferMode.ALLOW_TRANSFER to TransferMode.ALLOW_ALL
4. Update all the natspec relative to the transferMode variable, the TransferMode struct and everywhere the mint/burn/transfer logic is mentioned

constructor

In the constructor function apply these changes/refactor:

When params.transferMode == ConduitStructs.TransferMode.ACCOUNT_LIST, revert if address(params.accountList) == address(0) When params.transferMode != ConduitStructs.TransferMode.ACCOUNT_LIST, revert if address(params.accountList) != address(0)

_validateUserPermission function

This new functions will be used in both the create, Conduit's share transfer and NFT transfer

function _isAllowed(ConduitStore.Storage storage $, bool fromCreate, address sender, address receiver) internal returns (bool) {        ConduitStructs.TransferMode _transferMode = $.transferMode;
        // in ALLOW_TRANSFER everything is allowed        if( _transferMode == ConduitStructs.TransferMode.ALLOW_TRANSFER ) return true;        // in BLOCK_TRANSFER create is allowed when `sender == receiver`, transfer never allowed        if( _transferMode == ConduitStructs.TransferMode.BLOCK_TRANSFER ) {            if( fromCreate ) {                return sender == receiver;            }
            // transfers are never allowed            return false;        }
        // in ALLOW_LIST both the `sender` and `receiver` must be verified        // note 1: that both the `create` operation and ERC20/ERC721 standards allows         // to have `sender == receiver`        // note 2: in ALLOW_LIST mode `accountList` is always defined (see constructor requirements)
        // validate that the sender is allowed        if( !_accountList.isAllowed(sender) ) return false;        // if the receiver is not the sender, validate that also the receiver is allowed        if( sender != receiver ) {            if( !_accountList.isAllowed(receiver) ) return false;        }
        // sender was already allowed, receiver was the same as sender or was also allowed        return true;    }

create

Apply the following changes

-	if (!_ready() && msg.sender != $.deployer) {	// when the Conduit is not ready, only the deployer can create a Query and the deployer must also be the receiver+	if (!_ready() && (msg.sender != $.deployer || receiver != $.deployer)) {        revert ErrorLib.DisabledConduit();    }-	if (!_isAllowed($, msg.sender, receiver)) {+	if (!_isAllowed($, true, msg.sender, receiver)) {	    revert ConduitErrors.NotAllowed(msg.sender, receiver);	}

Conduit's share transfer

The _update function is called by the ERC20Upgradeable in many flows, including _mint and _burn.

Given that we have already validated msg.sender and receiver during the create execution, we can simply override the transfer and transferFrom functions and apply the validation directly there

function transfer(address to, uint256 value) public override returns (bool) {        address from = _msgSender();        // Note: we don't need to verify that `msg.sender != $.deployer` because we have enforced that         // the deployer is also the owner of the query in the `_create` flow        if( _ready() ) {	        if (!_isAllowed(ConduitStore.getStorage(), false, from, to)) {	            revert ConduitErrors.NotAllowed(from, to);	        }        }        return super.transfer(to, amount);    }        function transferFrom(address from, address to, uint256 value) public override returns (bool) {        if (!_isAllowed(ConduitStore.getStorage(), false, from, to)) {            revert ConduitErrors.NotAllowed(from, to);        }        return super.transferFrom(from, to, amount);    }

Now the _update function can be fully removed.

We can also remove all those logics that are minting/burning and transferring the shares

1. In _create instead of performing _transfer + _burn, just perform _burn(msg.sender, _burnAmount)
2. In _mintOngoingFees instead of performing _mint + _transfer, just perform _mint(address(feeManager), sharesToMint)
3. In _distributeAsset instead of performing _mint + _transfer, just perform _mint(recipient, _mintAmount)

isTransferable

The isTransferable must be changed to

function isTransferable(address from, address to) external view returns (bool) {-        return _isAllowed(ConduitStore.getStorage(), from, to);+        return _isAllowed(ConduitStore.getStorage(), false, from, to);    }

⚠️ Note: if the DEPLOYER it's not a trusted entity, the isTransferable function should add a validation to revert when _ready() == false. Maybe a better solution (to avoid direct impacts on the Conduit gas consumption) is to revert the OwnerRegistry.wrap function when conduit.ready() == false

⚠️ Important notes

Assumption on the Fee Manager

We are assuming that the feeManager is a TRUSTED actor. The permission validation will not be applied to the Conduit's shares (given the changes proposed) when _mintOngoingFees and _handleUnlocking are executed to mint/distribute the Conduit's shares.

If the assumption cannot be made we could apply this solution:

1. Update _mintOngoingFees to this logic

if (address(feeManager) != address(0) && ($.transferMode != TransferMode.ACCOUNT_LIST || _accountList.isAllowed(address(feeManager))) ) {

2. Update _handleUnlocking to this logic

if (address(feeManager) != address(0) && ($.transferMode != TransferMode.ACCOUNT_LIST || _accountList.isAllowed(address(feeManager))) ) {

Note that we have left unchanged both the feeManager.onUpdate and $.queryFeesConfigIds logics.

Async queries and edge cases

When the Conduit is configured with transferMode == TransferMode.ACCOUNT_LIST and the query is ASYNC, we could encounter this edge case: the receiver (the owner of the query returned by _ownerOf(...)) was allowed during the create(...) execution but is not allowed anymore when the query is unlocked/recovered.

The above suggested implementation would allow the receiver to finalize the query and receive the shares/assets. If the receiver should be blocked and the transaction reverted we can suggest the following change in the process function

if( $.transferMode == TransferMode.ACCOUNT_LIST && !_accountList.isAllowed(_owner) ) {	revert ConduitErrors.NotAllowed(_owner, _owner);}

⚠️ Note 1: It's important to note that this would now allow the query to be finalized to the SETTLED/REJECTED state. ⚠️ Note 2: It's possible that the now sanctioned user could, for the same query, be able to unlock/recover part of the funds if the query was partially unlocked/recovered.

Recommendation

Kiln should refactor and fix the whole mint/burn/transfer permission logic of the Conduit contract which is incompatible with the required specification.

It's important to note that the code shown above should be seen as a guideline/suggestion and not production-ready code.

Kiln

Fixed by the phase-3 access-control redesign (d88a4887). The entire mint/burn/transfer permission logic was refactored across all three surfaces this finding targets:

• Conduit shares + DEPOSIT create: create() gates the initiator with canDeposit (Conduit.sol:322-328) and screens the receiver via _transferAllowed when receiver != msg.sender (Conduit.sol:330-332); ERC20 _update bypasses system moves (mint/burn, conduit-as-counterparty) and gates user↔user transfers, deployer-only pre-enable (Conduit.sol:363-377). The single policy predicate is _transferAllowed = transferEnabled && (accountList == 0 || canTransfer) (Conduit.sol:738-744).
• Underlying assets + REDEEM create: gated by canRedeem (Conduit.sol:324). canRedeem = !sanctioned (AccountList.sol:256-261), so a blocked/de-listed holder can self-exit but the receiver screen clamps them to themselves — the intended canCreate semantics.
• OwnerRegistry NFT transfer: _update enforces IConduit.isTransferable(from, to) for wrapped tokens (mints bypass) (OwnerRegistry.sol:323-343) and wrap() is gated by isTransferable(self, self) (OwnerRegistry.sol:227-229).

The original TransferMode enum (ACCOUNT_LIST ALLOW/BLOCK, BLOCK_TRANSFER, ALLOW_TRANSFER) was intentionally replaced by a dual-list + AllowlistMode {OPEN, REGULAR, STRICT} model with a one-way transferEnabled latch and sanctions screening (AccountList.sol:247-281), rather than the finding's sample code — which the finding itself notes is a guideline, not production-ready. Acknowledged as resolved by design.

Spearbit

Verified the fixes.

Description

The AccountList can be configured with two different modes:

• Mode.ALLOW that allows only the accounts in isListed to receive or transfer cShares
• Mode.BLOCK that allows only the accounts not in isListed to receive or transfer cShares

The modes are one the opposite of the other and it would create a big problem for the Conduits (given that AccountList can be shared between multiple conduits).

Let's say that AccountList has been configured with the BLOCK mode and it has blocked u1 and u2. This means that everyone is allowed excluding u1 and u2.

If at some point the AccountList is switched to mode ALLOW, everyone is blocked by default and only u1 and u2 (that were in a blacklist) are allowed.

This function should be callable only if the isListed is empty. The problem is that being a isListed is a mapping and it can't be simply reset with delete.

It would need a refactor and maybe use a EnumerableSet of addresses instead of a simple mapping and at that point the best thing would be to have a swapMode function that takes the new mode and the new address[] calldata addrs to add to the list after the mode is swapped instantly.

Recommendation

Kiln should consider refactoring AccountList to allow the swap of the mode only if the isListed mapping is empty. One possible solution is to use EnumerableSet for the isListed list. The setAccountListMode could also accept a new address[] calldata addrs input that would be used to initialize the empty isListed set if needed.

Kiln

Fixed by the phase-3 access-control redesign (d88a4887). The single isListed mapping and the binary Mode.ALLOW/Mode.BLOCK enum described here have been removed entirely. AccountList now keeps two physically independent EnumerableSet.AddressSet instances — allowList and blockList — plus a three-value AllowlistMode {OPEN, REGULAR, STRICT} (AccountList.sol:69-75; IAccountList.sol:29-33). The block-list always denies first and the allow-list is only ever a positive gate (canDeposit:247-253, canTransfer:264-272), so setAllowlistMode (185-190) only changes which list a given operation consults and can never invert an existing list's meaning. The inversion hazard (blocked u1/u2 becoming the only allowed accounts after a mode flip) is therefore structurally impossible. This also adopts the EnumerableSet migration the finding recommended; with the two lists physically separate there is no need to gate mode switching on an empty list or add a swapMode.

Spearbit

Verified the fixes.

Description

When sanctionsEnabled == true it means that the contract wants to validate if the account that interacts with the Conduit is sanctioned.

This validation is provided by the Oracle sanctionsOracle. If sanctionsOracle == address(0) the _isSanctioned falls back to return false (not sanctioned) only to avoid a revert that would break the liveness of the Conduit. The expected "happy path" is that if sanctionsEnabled == true we have

• sanctionsOracle != address(0)
• sanctionsOracle works as expected: doesn't revert, return a well-formed bool when sanctionsOracle.isSanctioned(account) is called

To ensure (as much as possible) that the contract is configured properly and with a working oracle we suggest the following changes:

1. In the constructor revert when sanctionsEnabled_ == true but oracle_ == address(0).
2. In the toggleSanctionsList function revert when enabled == true but sanctionsOracle == address(0)
3. Consider also performing a "liveness" check on the oracle when configured. If the oracle does not work as expected, revert to prevent to configure the contract with a broken one. Below is an example (to be refactored and validated) solution for such a check:

function validateSanctionOracle(address _oracle) private view returns (bool) {        address stubAccount = address(0);        (bool _success, bytes memory _result) =            address(_oracle).staticcall(abi.encodeCall(ISanctionsList.isSanctioned, (stubAccount)));
        if (!_success || _result.length != 32) {            // oracle call has reverted or return value is malformed            return false;        }
        uint256 _word;        assembly {            /// +covered, assembly is not supported            _word := mload(add(_result, 0x20))        }
        // _word > 1: return value malformed, expected boolean type 0 | 1        return _word <= 1;    }

Recommendation

Kiln should consider improving the sanity checks performed on the "sanctioned" logic configuration of the AccountList contract.

Kiln

Fixed by previous commits — the phase-3 access-control redesign (d88a4887). All three suggestions are now enforced in AccountList.sol:

1. initialize() reverts SanctionsOracleRequired when sanctions are enabled without an oracle (line 162).
2. toggleSanctions() reverts SanctionsOracleRequired when enabling without a configured oracle (line 222); additionally setSanctionsOracle() cannot drop the oracle to address(0) while sanctions remain enabled (lines 232-233), closing the symmetric hole.
3. A code-presence check is performed on the oracle at init and on update via CheckLib.checkContract (lines 163 and 235), which rejects EOAs / unset addresses (Check.sol:38-43). Beyond a one-time configuration check, the runtime _isSanctioned() fails closed (lines 358-384): a reverting, EOA, or malformed-return oracle is treated as sanctioned rather than silently passing accounts, which is strictly safer than a one-time liveliness probe.

Spearbit

Verified the fixes.

Description

The STEAM standard supports scenarios where a Query could need multiple unlock or recover calls to finalize a query.

The initial owner of the query is currently allowed to partially unlock/recover the query, wrap it and sell it as a "full" query to another user.

The receiving user has no way at the moment to understand and validate the state of the wrapped query that they are receiving and evaluate its status.

Recommendation

Kiln should:

• consider reverting the wrap and transfer of a query that has been partially unlocked/recovered
• add storage, logics (when needed) and utility functions to both the Conduit and Vehicle to fetch the state of the query. Users should be able to see how much of the query still needs to be unlocked or recovered.

Kiln

Acknowledged — the partial-unlock (non-terminal) case only. Note the finalized/terminal case is now fixed on-chain (finding #9): OwnerRegistry.wrap takes the Query and reverts when it is SETTLED/REJECTED, and the conduit burns the wrapped NFT when a query finalizes, so a finalized query has no transferable NFT. This finding concerns a query that is PARTIALLY unlocked but still in-flight (non-terminal); the finalize-time burn does not apply, and gating it would require on-chain query-completion views that do not exist. For the partial case the wrap/transfer paths intentionally perform no on-chain STEAM completion check: wrap now reverts only on a terminal/empty state (the #9 fix), and OwnerRegistry._update gates wrapped-NFT transfers only on IConduit.isTransferable (transferEnabled && (accountList == 0 || canTransfer)) — neither consults how much of a still-in-flight query remains to be unlocked/recovered. Buyer protection for a partially-consumed but still-live wrapped claim is therefore handled off-chain on the bespoke exchange contract; adding an on-chain remaining-unlock view to gate it is intentionally out of scope.

Spearbit

Acknowledged.

Description

Let's assume that we have a base vehicle that performs the KYC on the owner of the query to allow the query to proceed to the UNLOCKING state.

When the Conduit is configured with that vehicle as the underlying vehicle, the owner of the query that is created on the vehicle will be the Conduit itself and not the receiver passed to the Conduit.create function that will invoke vehicle.create.

This means that the vehicle needs to approve the Conduit itself in the KYC process and won't validate the "real" receiver of the funds that the query unlock/recover.

Recommendation

Kiln should consider refactoring the BaseVehicle logic and the Query structure to track who is the "original" creator of the query that needs to be validated by the underlying vehicle when/where needed.

Note 1: the "root owner" of the query could change during the lifecycle of the query given that a query can be "wrapped" and transferred to another user. In that case the underlying vehicle should re-validate the KYC process given that the user has changed.

Note 2: the same problem exists when the MultiVehicle is configured with vehicles that require a KYC process. In that case the MultiVehicle itself is the owner of the query and not the user that interacts with the MultiVehicle.

Note 3: the KYC was just an example of incompatibility between the base vehicle, MultiVehicle and Conduit. There could be other scenarios where these kinds of problem/incompatibility could arise.

Kiln

Acknowledged — by design. The Conduit (and MultiVehicle's SubQueryEngine) is intentionally always the owner/receiver of the query created on the underlying vehicle; this is the foundation of the wrapping architecture, not a bug. The phase-3 access-control redesign (d88a4887) deliberately enforces compliance at the Conduit/AccountList boundary against the real user — canDeposit/canRedeem on the initiator and the transfer gate on the receiver in create() (Conduit.sol:320-332), isTransferable on wrap and on wrapped-query NFT transfers (OwnerRegistry.sol:227,336), and forceRedeem-based off-boarding (Conduit.sol:219-249) — rather than threading the end-user's identity into the underlying vehicle and the Query struct. We are not adopting the suggested STEAM/Query/BaseVehicle refactor (the Query struct intentionally remains owner/receiver-only, Query.sol:64-72, with no compliance hook in BaseVehicle). Vehicles that require their own on-chain KYC of the query owner should be integrated directly rather than wrapped by a Conduit/MultiVehicle.

Spearbit

Acknowledged.

Description

Let's assume that the Conduit is configured with the transferMode == TransferMode.ACCOUNT_LIST.

When the Conduit._create function is executed, the transaction reverts if the msg.sender or the receiver has not been both allowed in the AccountList.

Let's assume that both bob (the msg.sender) and alice (the receiver and so the owner of the query) have been allowed and that the query created by the underlying vehicle is an "async" query that ends in the PROCESSING state.

Before the query changes the state to UNLOCKING or RECOVERING, bob (the owner of the query) is removed from the allowlist of the AccountList or is sanctioned.

By not being allowed anymore, bob should not be able to receive the "output" of the query (it is not relevant whether it is assets or Conduit's shares). The problem with the current logic is that this behavior is not respected by the Conduit once the query has been created and the assets/cShares are unlocked/recovered.

When Conduit._process is executed and _handleUnlocking or _handleRecovering is executed, we have two scenarios (depending on the state of the query):

• Scenario 1: the conduit distributes the assets (we are unlocking a query created in REDEEM mode or we are recovering a query created in DEPOSIT mode)
• Scenario 2: the Conduit mints and distributes the Conduit's shares (we are unlocking a query created in DEPOSIT mode or we are recovering a query created in REDEEM mode) by executing _mint(address(this), _mintAmount); and _transfer(address(this), recipient, _mintAmount);

In the first scenario (distribution of the underlying assets) no validation is performed at all. The query owner, even if it is not allowed anymore, can receive assets. In the second scenario the Conduit performs "passive" checks on the receiver but all those logics ignore the validation of the receiver of the shares when the execution comes from a "mint" flow.

The _mint function executes _update(from=address(0), to=CONDUIT, value) -> in both transferMode configurations it will not revert:

• with TransferMode.ACCOUNT_LIST: the addr2 == CONDUIT -> return true -> no revert
• with TransferMode.BLOCK_TRANSFER: from == address(0) is already enough to avoid the revert The _transfer function executes _update(from=CONDUIT, to=bob, value) -> in both transferMode configurations it will not revert:
• with TransferMode.ACCOUNT_LIST: the addr1 = CONDUIT -> return true -> no revert
• with TransferMode.BLOCK_TRANSFER: the from == CONDUIT is already enough to avoid the revert

This means that bob will be able to receive the cShare shares from a DEPOSIT operation even if now is not allowed anymore.

If bob was not allowed during the execution of the create operation, the operation would have reverted in the beginning.

Recommendation

If this is the correct behavior, Kiln should carefully document it and let the integrator/manager of the Conduit be aware of it. Otherwise Kiln should refactor the logic to be coherent and revert the transaction, freezing the funds (both assets and Conduit's shares) when the receiver is not allowed anymore to interact with the Conduit. Note that in this case the query won't be able to be finalized and transition to the SETTLED/REJECTED state.

This issue has also been covered in the Finding "Refactor and fix of the mint/burn/transfer permission logic of the Conduit contract".

Kiln

Acknowledged — by design. The phase-3 access-control redesign (d88a4887) intentionally lets in-flight output flow. Gating is enforced at create() (initiator canDeposit/canRedeem + receiver screen) and on user-to-user ERC20 transfers, but process()/unlock/recover deliberately do NOT re-screen the receiver of an already-created query: reverting would permanently brick the underlying assets/cShares in a query that can no longer reach SETTLED/REJECTED. Off-boarding of a now-blocked or sanctioned holder is instead handled actively via the new forceRedeem(user, amount, output) path (Conduit.sol:219-249), gated by AccountList.canForceRedeem (CONDUIT_FORCE_REDEEM role), which returns true only for blocked/sanctioned users and ejects the holder's position back to themselves. This is the documented behavior matching your finding's "if this is the correct behavior, document it" branch.

Spearbit

Acknowledged. Off-boarding of blocked or sanctioned holders is handled through forceRedeem.

Description

The query.output value is used by BaseVehicle as a security measure to protect the user from slippage losses, unexpected changes in the exchange rate or if the performance, management or operational fees (deposit/redeem) result in an output below the user's expectation.

The Conduit is also offering this mechanism but it works differently compared to the one of the Vehicles and could result in confused and useless/misleading behavior.

When the user performs a DEPOSIT action, the user can only specify as q.output the minimum amount of vShares that the conduit should receive when vehicle.create is executed on the underlying vehicle. This behavior does not protect the user relative to the minimum amount of cShares (conduit shares) that the user would receive at the very end.

When the user performs a REDEEM action, the user can only specify as q.output the minimum amount of asset that the Conduit should receive when v.create is executed on the underlying vehicle. Like for the DEPOSIT scenario, this does not protect the user relative to the minimum amount of assets that the user would receive at the very end.

For the cShares case (DEPOSIT) it depends on the exchange rate of the Conduit but also the Conduit's deposit fees that the Conduit's Fee Manager applies.

For the assets case (REDEEM) it depends on the Conduit's redemption fees that the Conduit's Fee Manager applies.

Recommendation

Kiln should consider removing this protection mechanism that does not fully protect a user's like the one offered by the vehicle or consider extensively documenting it and providing clear guidance on how to use it and where the mechanism could end up not protecting the user as it is expected.

Kiln

Doc clarification in fce9beee

Acknowledged — by design. query.output is forwarded verbatim to the underlying vehicle's create (Conduit.sol:438), so it is a vehicle-level slippage floor only (min vehicle shares on deposit, min underlying asset from the vehicle on redeem). The Conduit's own fees and share-exchange rate are applied afterward in _handleUnlocking (feeManager.applyFees at line 574; mint/transfer at 644-653), so the floor intentionally does not bound the final conduit-share or final-asset amount. We have applied the documentation clarification: create and createRedeemFromConduitShares natspec (and their IConduit declarations) now state the query.output/outputAssets floor is enforced at vehicle output only and does not account for Conduit fees or share-exchange rate; users should use estimate() for true expected-output sizing. src/docs/reference/conduit.md carries the same note. No logic change.

Spearbit

Acknowledged. Kiln documented that the query.output floor is enforced at the vehicle output only (commit fce9beee).

Description

The Conduit._create process executed by both the create and createRedeemFromConduitShares functions could end up with a query in the "recovery" phase.

The current implementation does not manage this scenario in a specific way and could end up with a loss on the user side.

Recommendation

Kiln should consider refactoring the _create and _process logic to revert the transaction (that comes from a _create flow) if a "synced" query follows the EMPTY -> PROCESSING -> RECOVERING path.

Kiln

Acknowledged — by design. Conduit._create does not lose user funds on the recover path. _process (Conduit.sol:535-538) routes a RECOVERING query into _handleRecovering (Conduit.sol:604-624), which calls vehicle.recover() and distributes the recovered input back to the receiver: a full/partial refund (deposit assets returned; redeem conduit shares re-minted). Per the STEAM spec, create() can never directly yield RECOVERING, so RECOVERING is an async transition handled defensively. Reverting as recommended would discard the refund and would brick forceRedeem off-boarding for vehicles that fail synchronously, so we keep the current behavior.

Spearbit

Acknowledged.

Description

AccountListFactory

1. Add the FactoryBase public immutable ACCESS_CONTROL_FACTORY state variable to the AccountListFactory storage
2. Add to the constructor the FactoryBase accessControlFactory input to be saved in the ACCESS_CONTROL_FACTORY state variable
3. Add to the _paramsChecks function the validation FactoryLib.validateDeployedBy(ACCESS_CONTROL_FACTORY, address(params.accessControl));
4. Add to the _paramsChecks function the same validations that have been suggested in Finding 8 "Consider improving the validation of the sanctioned configuration logic of the AccountList contract"

ConduitFactory

1. Add the FactoryBase public immutable ACCESS_CONTROL_FACTORY state variable to the ConduitFactory storage
2. Add the FactoryBase public immutable FEE_MANAGER_FACTORY state variable to the ConduitFactory storage
3. Create a function _paramsChecks(SpawnParams calldata params) internal pure function to be executed by the spawn flow at the very beginning.
4. In the new _paramsChecks check add the following validations:
   • FactoryLib.validateDeployedBy(FEE_MANAGER_FACTORY, address(params.feeManager));
   • FactoryLib.validateDeployedBy(ACCESS_CONTROL_FACTORY, address(params.accessControl));
5. In the spawn function: accountList, vehicle and ownerRegistry are not checked to be deployed by actual trusted factories.

Recommendation

Kiln should implement the sanity checks we have suggested above.

Kiln

Fixed in commit 3c099139 (the AccountListFactory item); the ConduitFactory checks landed earlier in the phase-3 access-control redesign (d88a4887).

ConduitFactory was fully reworked in the redesign: it carries FEE_MANAGER_FACTORY / ACCOUNT_LIST_FACTORY / OWNER_REGISTRY_FACTORY / ACCESS_CONTROL_FACTORY immutables and an internal _paramsChecks() (run at spawn start) that calls validateDeployedBy on feeManager, accountList, ownerRegistry and accessControl. The vehicle is intentionally not validated via validateDeployedBy — vehicles are produced by many distinct vehicle-specific factories, so there is no single trusted factory; instead the vehicle's asset() is validated against the AssetRegistry. The previously-outstanding item — the ACCESS_CONTROL_FACTORY immutable + validateDeployedBy(accessControl) on AccountListFactory, which formerly only did CheckLib.checkContract(accessControl) — has now been added, mirroring the ConduitFactory pattern.

Spearbit

Verified the fixes. The ConduitFactory checks landed in commit d88a4887 and the AccountListFactory check in commit 3c099139.

Description

When the Conduit is deployed by the ConduitFactory, the factory will follow these steps:

1. pull _initialDepositAmount of funds from the msg.sender (account with the role Roles.CONDUIT_SPAWN)
2. approve _initialDepositAmount allowance to the Conduit
3. execute conduit.create(_depositQuery, address(this));

The ConduitFactory is not currently managing the scenarios where the query ends up in the final REJECTED state.

If the vehicle used by the conduit is a "synced" vehicle and the _currentState (after the v.create execution) is REJECTED, the factory creates a PendingDeposit and adds it to $pendingInitialDeposits[address(conduit)]. This logic is wrong because the query is not pending, it has been already finalized as rejected and the recovered funds have been already sent to the ConduitFactory itself (receiver == address(this) when conduit.create is executed).

Calling finalizeConduitDeposit will early return in _finalizeConduitDeposit because the query is in the REJECTED state. The original funds (or fewer of them depending on the underlying vehicle recovery process) will remain stuck in the ConduitFactory.

If the vehicle used by the conduit is an "async" vehicle, the _currentState (after the v.create execution) is PROCESSING and the factory creates a PendingDeposit and adds it to $pendingInitialDeposits[address(conduit)]. When finalizeConduitDeposit is executed State _currentState = _pending.vehicle.state(_pending.depositQuery); will be equal to RECOVERING. _finalizeConduitDeposit will call conduit.process that will recover the funds and send them back to the ConduitFactory. The _finalizeConduitDeposit will early return given that currentState != State.SETTLED.

Recommendation

In the first scenario where the conduit's vehicle is a "synced" vehicle and the conduit.create returns a REJECTED query, the spawn function should immediately revert.

The second scenario is more complicated to handle correctly. The conduit's vehicle could be REJECTING the query for multiple reasons. One possible solution would be to "exhaust" the query (the recovery could be a multi-step recovery process) and send those funds back to the original owner or use those funds to try again with another first query deployment from an ad hoc new function.

Kiln

Acknowledged — not actionable for the current vehicle set.

Sub-item 1 (sync vehicle returning REJECTED): cannot occur — the STEAM standard forbids a direct create() transition to REJECTED/RECOVERING (STEAM.md:367); create() reverts the entire transaction on failure. The state right after create() can only be PROCESSING, UNLOCKING, or SETTLED — never REJECTED — so the "revert on REJECTED" recommendation is moot.

Sub-item 2 (async deposit going PROCESSING→RECOVERING and stranding funds): unreachable today — RECOVERING is only reachable from PROCESSING, and every supported vehicle (Aave V3, Morpho Blue, ERC4626, Ethena, Wrapper, MultiVehicle) performs synchronous deposits. The non-settled PendingDeposit record is intentionally retained so an async deposit can be finalized later once it settles (ConduitFactory.sol:299-311). We will add a recovery/sweep path only if an async-deposit vehicle is ever introduced.

Spearbit

Acknowledged.

Description

Attackers can front-run calls to Conduit._create() with the same calldata but just set receiver to be their own controlled address. By doing this they can cause the original user call to create() to revert with the error of ConduitErrors.QueryAlreadyExists(queryId) potentially causing denial of service for targeted users. This will require the attackers to deploy the same amount that the victims are depositing, but they can recover it back by creating a redeem query instantly after.

Recommendation

Consider recalculating queryId by taking the current queryId and hash it together with msg.sender, note that it will solve the issue both for create() and createRedeemFromConduitShares().

Kiln

Fixed in commit cc626030

Fixed on-chain. We bound the query salt to the caller: Conduit.create now takes a bytes32 sourceSalt and requires query.salt == keccak256(abi.encode(msg.sender, sourceSalt)), reverting InvalidQuerySalt otherwise. Because the accepted salt is cryptographically tied to msg.sender, an attacker cannot reproduce a victim's salt, so they cannot replay the victim's query to occupy its queryId (the front-run reverts for the attacker), and two distinct callers can never collide on a queryId. createRedeemFromConduitShares (3rd arg renamed sourceSalt) builds its redeem query with the same bound salt; forceRedeem is unaffected (privileged, uses its own queriesCount-derived salt). This also removes the prior mitigation requirement that integrators pick unguessable/random salts — even a predictable sourceSalt is collision-proof now. queryId stays query.toId(vehicle) over the bound salt, so conduit↔vehicle id equality used by state()/process() and the wrapped-query id from finding #9 are all unchanged.

Spearbit

Fixed by bounding the query salt to the caller: Conduit.create() now takes a bytes32 sourceSalt and requires query.salt == keccak256(abi.encode(msg.sender, sourceSalt)), reverting InvalidQuerySalt otherwise.

Description

The isTransferable function is used to verify that transfers are enabled from from address to to address.

However, this function does not verify that the $.transferMode is not BLOCK_TRANSFER.

This can lead the isTransferable function to indicate that a transfer is allowed while it will revert if executed.

Recommendation

Add a check that the transfer mode is not BLOCK_TRANSFER.

Kiln

Fixed by the phase-3 access-control redesign in commit d88a4887. The TransferMode enum — including BLOCK_TRANSFER — and the $.transferMode storage field were removed entirely; the store now keeps only a transferEnabled boolean. isTransferable (Conduit.sol:275-277) now delegates to a single predicate _transferAllowed = transferEnabled && (accountList == 0 || accountList.canTransfer(from,to)) (Conduit.sol:738-744), which is the identical predicate enforced on the real transfer paths — _update's user↔user gate (Conduit.sol:372) and the create receiver screen (Conduit.sol:330). Because the view and the enforced gate share one code path, isTransferable can no longer report a transfer as allowed that would later revert. The only remaining BLOCK_TRANSFER mention is a stale doc comment in ITransferGate.sol:18 with no behavioral effect.

Spearbit

Fixed. The new $.transferMode is checked in _transferAllowed.

Description

1. AccountList.sol?lines=92,106: In the AccountList contract, the AddressAdded, AddressRemoved and SanctionsOracleSet inputs can be declared indexed. Consider also renaming addr to account.
2. AccountList.sol?lines=182,182: The StateUnchanged error used throughout the AccountList contract is too generic. State may have been changed for multiple accounts while the error is returned. This is counter-intuitive. Consider having dedicated errors.
3. OwnerRegistry.sol?lines=158,159: Code is defining initialize(symbol_, name_) and then calling __ERC721_init(name_, symbol_). The input parameters order is not the same.
4. OwnerRegistry.sol?lines=209,209: make the input order of the OwnerRegistry.wrap function follow the same order of the registerOwner to be consistent and aligned.
5. OwnerRegistry.sol?lines=328,328: the _update function of the OwnerRegistry contract is internally invoked by the ERC721 contract when the NFT is minted/burned or transferred. To mint the NFT, the user must call the wrap function that reverts when conduit == address(0). This means that when the _update function is called, the scenario where the $.queries[_hashId].conduit value is undefined is not possible. Remove such sanity check.
6. Conduit.sol?lines=90,90: Consider adding a VehicleCategory input when the Conduit is deployed and validate that the vehicle used in the __Conduit_init respects the category.
7. Conduit.sol?lines=165,165: The Conduit.createRedeemFromConduitShares function should return the Query internally created to prevent the caller from regenerating it in case it needs to use it in the future. Alternatively, this could be handled by emitting a dedicated event.
8. Conduit.sol?lines=447,447: the Conduit._mintOngoingFees is passing Id.wrap(0) as the Id queryId to the ConduitEvents.Pushed event even if the query id is known when the _mintOngoingFees function is executed.
9. ConduitFactory.sol?lines=239,240: Consider documenting that in case the accountList is set with the Mode.ALLOW, the factory must be allowed before the conduit deployment. It must not be blocklisted in Mode.BLOCK. In both modes, it must not be sanctioned.
10. ConduitFactory.sol?lines=247,252: the ConduitFactory should emit an event to track those Conduit deployments that won't be immediately finalized and need a future execution of the finalizeConduitDeposit function. An external tracker/keeper could need to track those events to later on finalize the deployment and enable the conduit.

Recommendation

Kiln should consider implementing the suggestions listed above.

Kiln

Fixed in commit 92e05a50 (the cosmetic/observability items); the remainder is acknowledged by design.

These are tracked Informational nits; the phase-3 access-control redesign ([d88a4887](https://github.com/railnetorg/hangar/commit/d88a4887)) rewrote AccountList/Conduit and added the OwnerRegistry wrap transfer gate but did not target most of these. We applied the purely cosmetic/observability items: indexed the AccountList event address/oracle params and renamed addr→account; split the generic StateUnchanged into dedicated errors (including an unambiguous one for the per-element no-op in _updateList); aligned OwnerRegistry.initialize and wrap/registerOwner parameter ordering; emitted the real queryId in the _create-path fee Pushed event; and added a dedicated event in ConduitFactory.spawn for an async initial deposit (not finalized immediately). Acknowledged / not actioned (by design): we keep OwnerRegistry._update's conduit != address(0) guard as defensive depth; returning the built Query from createRedeemFromConduitShares and adding VehicleCategory validation on Conduit deployment are enhancements/feature requests rather than bugs (VehicleLib.VehicleCategory exists but is deliberately not wired into the Conduit); and the Mode.ALLOW/Mode.BLOCK documentation note is obsolete since the single-mode model was replaced by the OPEN/REGULAR/STRICT dual-list design — the underlying requirement (the factory must satisfy canDeposit: allow-listed in REGULAR/STRICT and never blocked/sanctioned before deployment) is unchanged and is noted in the factory docs.

Spearbit

Verified the fixes for the cosmetic and observability items (commit 92e05a50); the remaining items are acknowledged by design.

Description

1. Both the OwnerRegistry registerOwner and wrap are permissionless functions that can be called by anyone (even if they are not "real" conduits) and can generate "invalid" and spammy events and modify the state of the contract. This behavior should be annotated and disclosed to let integrators and monitoring tools be aware of it.
2. Conduit.sol?lines=587,587: _distributeAsset returns both the updated total supply and the vehicle holdings.
3. OwnerRegistry.sol?lines=64,67: The @return comment indicates that it returns the AccountList storage reference. This is incorrect as it is the OwnerRegistryStorage storage reference.
4. Conduit.sol?lines=307,307: the comment "Allow all transfers when conduit is not enabled" is wrong or outdated. Please remove it or rewrite it fully.
5. Conduit.sol?lines=297,297: the "BLOCK_TRANSFER prevents all transfers except minting and burning." comment in the Conduit._update function can be seen as misleading. The _update function will also allow transfers calls where the from or to addresses are the Conduit itself.

Recommendation

Review and correct the cited natspec comments and documentation strings to accurately reflect the implemented behavior. Remove unused error declarations.

Kiln

Fixed in commit 12c41d1b

The two misleading transfer comments were already removed by the phase-3 access-control redesign ([d88a4887](https://github.com/railnetorg/hangar/commit/d88a4887)): the old TransferMode/BLOCK_TRANSFER logic and _isAllowed gate are gone, and the rewritten _update/_transferAllowed natspec describes the behavior accurately, including the mint/burn/conduit-as-counterparty system-move bypass. There were also no unused error declarations to remove (all are reverted at least once). The remaining documentation nits have now been fixed: (A) OwnerRegistry.registerOwner and wrap gained a @dev note that they are permissionless and may emit OwnerRegistered/QueryWrapped events for arbitrary, non-real-conduit callers, so integrators and monitoring tools must not treat such events as proof of a genuine conduit; (B) _distributeAsset now documents both return values (the updated total supply and the updated vehicle holdings); (C) _getOwnerRegistryStorage natspec was corrected from "AccountList storage reference" to "OwnerRegistry storage reference".

Spearbit

Verified the fixes. The documentation has been corrected in commit 12c41d1b.

Description

Consider refactoring both the addAddresses and removeAddresses functions of the AccountList contract to use the same logic as much as possible. Create an internal updateAddresses function that can be called by both the mentioned functions.

function updateAddresses(address[] calldata addrs, bool add) internal {        AccountListStorage storage $ = _getAccountListStorage();        uint256 _length = addrs.length;        for (uint256 _i; _i < _length; ++_i) {            address _addr = addrs[_i];            CheckLib.checkAddress(_addr);
            if ($.isListed[_addr] == add ) revert StateUnchanged();            $.isListed[_addr] = add;            if(add) emit AddressAdded(_addr);            else emit AddressRemoved(_addr);        }    }

The same function must also be called in the initialize flow to replace the existing logic.

Recommendation

Kiln should create an internal function updateAddresses and use it to replace the existing code of the addAddresses, removeAddresses and initialize functions.

Kiln

Fixed by the phase-3 access-control redesign in commit d88a4887.

The redesign extracted a single shared internal routine _updateList(set, addrs, add, isAllow) (AccountList.sol:328-351) that performs the add/remove with per-address CheckLib.checkAddress and the StateUnchanged no-op guard. It is reused by all four list mutators (addToAllowList/removeFromAllowList/addToBlockList/removeFromBlockList, lines 195/201/207/213) and by initialize (lines 170-171), exactly as recommended. The old addAddresses/removeAddresses/isListed functions referenced in the finding no longer exist (the contract now uses two independent EnumerableSets). The isAllow flag only selects which list-specific event to emit; the add/remove logic itself is single-sourced.

Spearbit

Verified the fixes. A single internal _updateList routine is now shared by all list mutators and by initialize.

Description

The _create function uses safeIncreaseAllowance from the SafeERC20 library.

However, it is not supposed to increase the allowance but actually set it to the value passed.

function _create(Query memory query, address receiver) internal returns (Id queryId, State state) {        // ...        uint256 _length = query.input.length;        for (uint256 _i; _i < _length; ++_i) {            Asset memory _asset = query.input[_i];            if (_asset.asset == address(_vehicle)) {                uint256 _burnAmount = _convertVehicleSharesToConduitShares(                    _asset.value, _totalSupply, _vehicleHoldings, _assetDecimals, _vehicle, Math.Rounding.Ceil                );                _transfer(msg.sender, address(this), _burnAmount);                emit ConduitEvents.Pulled(address(this), _burnAmount, msg.sender, queryId);                _burn(address(this), _burnAmount);                _totalSupply -= _burnAmount;                _vehicleHoldings -= _asset.value;            } else {                IERC20(_asset.asset).safeTransferFrom(msg.sender, address(this), _asset.value);                emit ConduitEvents.Pulled(_asset.asset, _asset.value, msg.sender, queryId);                IERC20(_asset.asset).safeIncreaseAllowance(address(_vehicle), _asset.value);            }        }

Recommendation

Consider using forceApprove instead of safeIncreaseAllowance.

Kiln

Acknowledged.

After the phase-3 access-control redesign (d88a4887), _create resets every input-asset allowance to zero after vehicle.create() via an unconditional forceApprove(address(vehicle), 0) loop (Conduit.sol:440-446). This guarantees the conduit→vehicle allowance is always 0 before the next safeIncreaseAllowance grant (Conduit.sol:433), so no stale or accumulated allowance can persist — the concern raised by this finding is fully mitigated.

We are intentionally keeping safeIncreaseAllowance rather than switching to forceApprove(value): query.input is not deduplicated, and across duplicate input assets safeIncreaseAllowance correctly accumulates the allowance to the total transferred in, whereas forceApprove would overwrite and under-approve. Combined with the always-zeroed starting allowance, safeIncreaseAllowance is the more robust choice here, so we are not changing this line.

Spearbit

Acknowledged. The allowance is reset to zero after every vehicle.create call, which mitigates the concern.

Description

The Conduit contract's code is not following the same code style best practices that have been applied widely to other contracts in the codebase.

Kiln should consider applying the following suggestions:

Local variable and function input names

1. rename all the local variables that could clash with state variables or function names to have the __ prefix. Example: totalSupply -> __totalSupply or _totalSupply -> __totalSupply
2. rename all the function's input names that could clash with state variables or function names to have the _ suffix. Example: totalSupply -> totalSupply_

Use the same code/approach already adopted by the MultiVehicle and BaseVehicle

Function names: adopt the same function names already used by the BaseVehicle. Here are some examples:

• _mintOngoingFees should be renamed _handleOngoingFees
• _handleUnlocking should be renamed _unlock
• _handleRecovering should be renamed _recover
• and so on

The same functions (conceptually) have different behaviors

To make a clean example: in the BaseVehicle the updated total supply and total assets are directly returned by the _handleOngoingFees function that could modify them (by minting fees). The comparable function in the Conduit (called _mintOngoingFees) is not returning those values but returns only the amount of fees that have been minted (if any) to the FeeManager.

Recommendation

We understand that the Conduit is not a "real" Vehicle but we think that it's fair to say that it acts as one. Given the similarity, it would be beneficial for both the readability and maintainability of the code if the Conduit were refactored to align its code and functions' name to the existing one already adopted by the vehicles.

⚠️ NOTE: all the above were just examples, the codebase has more. In general this is more a suggestion to review the code and see where it can be optimized and refactored to have a common style/approach.

Kiln

Acknowledged. This is a cosmetic style-alignment observation with no functional or security impact. The three noted divergences still exist in current code: (1) Conduit lifecycle helpers take plain totalSupply/vehicleHoldings params whereas BaseVehicle uses underscore-suffixed locals; (2) _mintOngoingFees/_handleUnlocking/_handleRecovering naming differs from BaseVehicle's _handleOngoingFees/_unlock/_recover; (3) _mintOngoingFees returns only sharesToMint. A repo-wide rename/harmonization was out of scope for the phase-3 access-control work and carries regression risk disproportionate to a non-security cleanup; we will fold it into a future dedicated cleanup pass.

Spearbit

Acknowledged.

Description

Kiln should consider adding the following additional sanity checks to the Conduit logics

constructor + __Conduit_init: add a VehicleCategory input to the constructor of the Conduit and enforce that the vehicle with which the Conduit is initialized is indeed respecting the Conduit's expected vehicle category. At least for now the protocol is only supporting single-asset vehicles

process

The function should revert in the following scenarios:

• _owner == address(0): the query does not exist
• vehicle.state(query) == SETTLED / REJECTED: the query has already been finalized and cannot proceed in any other states.

isTransferable

Consider reverting with a Unimplemented or Unsupported custom error when $.ownerRegistry == address(0). In this case the Conduit does not support registering, wrapping and transferring any "wrapped query" as an NFT. This function should never be executable for that kind of Conduit.

Another more "soft" approach is to directly return false.

create

(optional) Consider validating that the query.input, the query.mode and the query.output are compatible with the underlying vehicle's expected value. This suggestion has been marked as "optional" given that we are assuming that the vehicle does respect the STEAM standard and will revert on its own during the vehicle.create(query) execution if the query is malformed or not compatible with the vehicle itself.

createRedeemFromConduitShares

(optional) Consider validating that the query.output is compatible with the underlying vehicle's expected value and the pre-filled query.input and query.mode. This suggestion has been marked as "optional" given that we are assuming that the vehicle does respect the STEAM standard and will revert on its own during the vehicle.create(query) execution if the query is malformed or not compatible with the vehicle itself.

Recommendation

Kiln should implement the sanity checks suggested above.

Kiln

Acknowledged. Of the five suggested sanity checks:

• process intentionally early-returns terminal queries (SETTLED/REJECTED) to anyone and no-ops on EMPTY rather than reverting, per the phase-3 access-control redesign (d88a4887) — Conduit.sol:192-204. Reverting would conflict with that design.
• isTransferable is now the single _transferAllowed predicate reused for ERC20 cShare transfers and the create receiver screen (Conduit.sol:275-277, 330, 738-744). Making it revert/return false when ownerRegistry == 0 would brick ordinary share transfers.
• create/createRedeemFromConduitShares validation is already enforced by the underlying vehicle's create() via STEAM (reverts on invalid input/mode/output, never produces REJECTED).
• vehicle-category check: VehicleLib.isCategory already exists and is enforced for MultiVehicle sub-vehicles (VehicleManager.sol:510). Not wiring it into __Conduit_init is intentional; we are tracking it as optional defense-in-depth hardening.

Spearbit

Acknowledged.

Description

The Conduit._isAllowed() function contains a logic flaw that allows restricted users to interact with the contract by setting the receiver to the Conduit address (address(this)).

// Allow transfers to the conduit itselfif (addr2 == address(this) || addr1 == address(this)) {    return true;}

Impact: Blacklisted users can execute state-changing transactions, interact with underlying vehicles, and emit events (e.g., QueryCreated) containing their address, undermining compliance and security filters.

Recommendation

Refactor _isAllowed() to ensure msg.sender is always validated unless the sender is the Conduit itself.

Kiln

Fixed by the phase-3 access-control redesign (d88a4887). Conduit._isAllowed() and its addr == address(this) short-circuit have been deleted. create() now gates the initiator directly: deposits require AccountList.canDeposit(msg.sender) and redeems require canRedeem(msg.sender) (Conduit.sol:320-328), reverting CreateNotAllowed(msg.sender) independent of the receiver parameter. canDeposit returns false for any blocklisted or sanctioned account (AccountList.sol:247-253), so a blocklisted user can no longer create a query — or emit QueryCreated — by setting receiver = address(this) or any other value. The receiver screen (Conduit.sol:330-332) is additive and only applies when receiver != msg.sender. The one remaining self-create path is a blocked (non-sanctioned) holder self-redeeming (canRedeem = !isSanctioned), which is the intended off-boarding behavior and is clamped to the holder's own address by the receiver screen.

Spearbit

Fixed by implementing the reviewer's recommendation.

Description

The holdings() function returns the vehicle shares balance of the Conduit.

As this function is directly getting the balance of the Conduit, it makes this contract prone to donations which could inflate the conduit shares rate.

Recommendation

Either document that donations are expected to the Conduit, or avoid donations by tracking the amount of shares the Conduit got through queries to the underlying vehicle.

Kiln

Fixed in commit 5b535a78

holdings() previously returned the raw vehicle-share balanceOf(address(this)), so a direct donation of vehicle shares could inflate the conduit-share exchange rate. We added internal vehicle-share accounting — a tracked-holdings field in ConduitStore, updated on every legitimate balance change (deposit vehicle.create, redeem input, and unlock/recover distribution) — and holdings()/totalAssets() now read that tracked amount instead of the live balance. This removes the donation/rate-inflation surface entirely. Shipped with dedicated tests (a vehicle-share donation does not move the conduit-share rate; tracked holdings equal the real balance under normal create/process/unlock/recover flow, including fee accrual) and updated IConduit.holdings() documentation.

Spearbit

Verified the fixes. holdings() and totalAssets() now read internally tracked vehicle shares instead of the live balance, so donations no longer move the exchange rate.

Description

The current implementation of Conduit.process allows anyone to execute the underlying _process call that could resume, unlock or recover a query, even if the msg.sender is not the owner of the query.

This behavior goes against the behavior followed by other contracts in the protocol that follow the STEAM standard. While it's true that the Conduit is not a Vehicle, we also think that it should follow the same behavior.

Recommendation

Kiln should revert the process operation if the caller is not the owner of the query. If this feature needs to be supported (used for example by a Keeper system), Kiln can allow the owner of the query to configure allowed delegators that could process the query on his behalf.

Kiln

Fixed by the phase-3 access-control redesign (d88a4887). Conduit.process is no longer permissionless for live queries: it resolves the query owner and, when msg.sender is not the owner, requires the new CONDUIT_PROCESS role — the keeper path you suggested — see Conduit.sol:198-202 and Roles.sol:163. The role check reverts MissingRole for any unauthorized caller, and supports both global and conduit-scoped grants. The only thing still callable by anyone is the early-return for already-terminal queries (SETTLED/REJECTED) at Conduit.sol:192-196, which is read-only observability and mutates no state. Covered by the dedicated regression suite test/conduit/Conduit.ProcessAuthorization.t.sol (non-owner-without-role reverts; owner, global keeper, and conduit-scoped keeper all succeed).

Spearbit

Verified the fixes. For live queries, process is now restricted to the query owner or holders of the CONDUIT_PROCESS role.

Scope

The contracts below were reviewed at commit e33758593cc46c198c3358a31334a08b9a52a2a5 and the fix review was reviewed at commit ca898a1c07b591ae4943963c099704522dc0f205.

src├── conduit│   ├── abstracts│   │   ├── ConduitErrors.sol│   │   └── ConduitEvents.sol│   ├── AccountList.sol│   ├── Conduit.sol│   ├── interfaces│   │   ├── IAccountList.sol│   │   ├── IConduit.sol│   │   ├── IOwnerRegistry.sol│   │   └── ISanctionsList.sol│   ├── libs│   │   ├── ConduitStore.sol│   │   └── ConduitStructs.sol│   └── OwnerRegistry.sol└── factories    └── conduit        ├── AccountListFactory.sol        ├── ConduitFactory.sol        └── OwnerRegistryFactory.sol

In addition to the contract review above, this phase included an end-to-end testing effort over the full stack (vehicles, MultiVehicles, conduits, and keeper flows), exercising the system through stateful invariant campaigns and live-network fork tests.

Codebase Overview

Kiln's phase 3 audit covers the Conduit layer, the user-facing wrapper that turns a raw STEAM vehicle into an ERC-20 product with fees, compliance controls, and transferable positions.

A Conduit wraps exactly one vehicle and issues its own ERC-20 conduit shares; the conduit holds the vehicle's shares, and value flows through the chain conduit shares ↔ vehicle shares ↔ underlying assets. Users interact through two calls: create validates the request, applies access-control checks, pulls the user's assets (or conduit shares for redemptions), snapshots the active fee configuration for the query, and forwards it to the vehicle; process then drives the query through the STEAM state machine on the owner's behalf — resuming paused queries, unlocking settled ones (minting conduit shares or paying out assets, with fees applied), and recovering failed ones. Synchronous vehicles settle within the create transaction. Management and performance fees accrue through the shared FeeManager, and each conduit enforces a configurable transfer policy on its shares (list-checked, free, or mint/burn-only).

Two optional modules extend a conduit: the AccountList, an allowlist/blocklist with optional sanctions-oracle integration that gates who can create queries, receive shares, and transfer; and the OwnerRegistry, which externalizes query ownership and can wrap an in-flight query into a transferable ERC-721 token. The three conduit factories follow the platform's gated-factory pattern: spawning is role-gated, deploys beacon proxies, and runs an initial burned deposit through the conduit — held as a pending deposit and finalized later when the underlying vehicle settles asynchronously.