Coinbase: Tokenization Rewrite

Description

IB20 declares pausedFeatures() external view returns (PausableFeature[] memory), but SecurityToken implements only pause, unpause, and isPaused and there is no pausedFeatures(). A caller casting the token to IB20/IB20 asset and invoking pausedFeatures() hits no matching function and reverts.

Unlike the policy-surface omissions (policyId/updatePolicy), which the contract explicitly documents as intentional divergences, this omission is undocumented and reads as an oversight.

Recommendation

Implement pausedFeatures() by expanding the stored pausedMask bits into a PausableFeature[]. If the omission is instead deliberate, document it alongside the existing policy-surface divergence note rather than leaving it silent.

Description

PolicyRegistry contract rejects address(0) in _batchSetMembers(), so zero can never be added as a policy member. SecurityToken contract runs the configured transferPolicyId policy inside SecurityToken._update() against from, to, and msg.sender. This includes minting, where from == address(0), and burning, where to == address(0).

transferPolicyId is only documented as conventionally a BLOCKLIST; it is not required to be one. setTransferPolicyId() only checks that the new policy exists and does not enforce that it is a BLOCKLIST. As a result, an admin can configure an ALLOWLIST as transferPolicyId after deployment.

With an ALLOWLIST transfer policy, address(0) is not authorized because it is not a member and cannot be added. This makes minting and burning revert whenever SecurityToken._update() checks the zero address. The highest impact is that a valid policy update can unintentionally disable token issuance and redemption flows. Note that the policy can be changed post-deploy via setTransferPolicyId().

Recommendation

Choose one invariant and enforce it consistently:

1. Require transferPolicyId to be a BLOCKLIST in both documentation and on-chain validation in initialize() and setTransferPolicyId().
2. Permit address(0) in policy membership, so allowlisting zero unblocks mint and burn, while blocklisting zero can intentionally pause mint and burn.
3. Skip policy checks for address(0) in SecurityToken._update(), so mint and burn do not depend on zero-address policy membership.

Coinbase: We understand the issue, we are going to put off-chain checks to ensure this isn't possible

Description

The @dev natspec on burnBlocked says the path preserves the "to/sender/origin checks." But _burnSkippingFromSanctions hardcodes to = address(0), and the contract never checks tx.origin. Only the sender check is enforced, alongside the BURN pause vector.

Recommendation

Reword to name only the checks that run - the msg.sender sanctions check and the BURN pause vector:

-  /// design) while preserving the to/sender/origin checks and the+ /// design) while preserving the `msg.sender` sanctions check and the

Description

Function initialize() rejects ALWAYS_ALLOW_ID (0) as the factory default to force sanctions gating onto every token's transfer slots.

But _buildInitCalls() seeds those four slots first and then appends the caller's extraData init-calls, so a TOKEN_DEPLOYER_ROLE caller can pass updatePolicy(slot, 0) to overwrite all four with the open-allow sentinel and the token still auto-registers with the TokenSupplyManager. The guard therefore constrains only the stored default, not the deployed outcome it implies.

Recommendation

Document on initialize/deployToken that the InvalidPolicyId check is init-only and does not prevent a trusted TOKEN_DEPLOYER_ROLE deployer from re-pointing the slots to 0 via extraData i.e. the open-allow protection rests on the trusted-role assumption plus off-chain review, not on an on-chain end-to-end guarantee.

Optionally, for defense-in-depth, scan the decoded extraData for updatePolicy(<transfer slot>, ALWAYS_ALLOW_ID) and reject it, mirroring the init-time guard.

Coinbase: Understood, but we won't be allowing this via offchain checks, the documentation is updated in b3b6af9

Description

Function initialize() rejects transferPolicyId == ALWAYS_ALLOW_ID (0), but setTransferPolicyId() checks only if the policyExists. So a DEFAULT_ADMIN_ROLE admin can set it post-deploy, making _isSanctioned() return false for everyone and silently disabling _checkSanctions() on every transfer/mint/burn. By design: a trusted admin can reach the same fail-open state (or brick the token via an all-blocking policy / ALWAYS_BLOCK_ID) through any user-created policy, so a literal-0 check at the update path adds no real protection. The asymmetry is simply undocumented.

Recommendation

Add an inline comment on setTransferPolicyId() so readers understand why 0 is rejected at init but not here, and extend the existing admin-trust natspec to name the fail-open / brick (centralization) risk.

Description

The redeemWithMemo() natspec claims the memo is emitted "associating it with the redeemer not the manager." But transferFromWithMemo() emits Memo(msg.sender, memo) (SecurityToken.sol:637), whose first field is caller and when the TSM calls it, that caller is the manager (TSM), not the redeemer.

The redeemer is actually captured by Transfer.from and Redeemed.from, which do the real tracking; the memo path exists only so a redeemer can tag their redemption.

Recommendation

Simplify the comment to drop the redeemer-attribution claim and state that Memo.caller is the manager, that redeemer attribution comes from Redeemed (and Transfer.from), and that the memo just lets a redeemer tag their redemption.

Description

SecurityTokenFactory defines one upgrade credential, UPGRADER_ROLE (keccak256("UPGRADER_ROLE")), and uses it to gate two distinct and independently-scoped upgrade surfaces:

• upgradeBeacon(address newImplementation) (SecurityTokenFactory.sol:317) moves the shared UpgradeableBeacon pointer, atomically replacing the logic of every deployed SecurityToken in a single call.
• _authorizeUpgrade(address newImplementation) (SecurityTokenFactory.sol:368) authorizes the factory's own UUPS implementation upgrade.

Concentrating both upgrade surfaces into a single UPGRADER_ROLE reduces granularity of control and increases the trust placed in that one credential, so that a single compromised key is sufficient to subvert both the factory and every deployed token at once.

Recommendation

Separate the two upgrade surfaces into distinct roles: for example BEACON_UPGRADER_ROLE (token implementation) and FACTORY_UPGRADER_ROLE (factory UUPS), so they can be assigned to different holders and governed independently (if required).

Description

The IPolicyRegistry interface documents an explicit overflow contract for both creation entry points:

@dev Panics with arithmetic overflow (Panic 0x11) when the policy counter has reached its maximum value.

However, the implementation does the opposite. _create reads the uint56 nextCounter and increments it inside an unchecked block:

uint56 counter = $.nextCounter;// No overflow guard: at one policy per 2-second block, exhausting// the 56-bit counter space takes ~4.6 billion years.unchecked {  $.nextCounter = counter + 1;}

unchecked suppresses the over/underflow check for all integer widths, uint56 included, so at the ceiling type(uint56).max + 1 wraps to 0 with no Panic(0x11), directly contradicting the interface.

Beyond the documentation mismatch, the wrap is actively corrupting rather than merely non-conformant. The condition is unreachable in practice as exhausting the 56-bit space at one policy per ~2s block takes ~4.6 billion years. So there is no exploitable risk.

Recommendation

Make the implementation comply to the standard interface definition by removing the unchecked wrapper so the increment reverts with Panic(0x11) at the ceiling.

Or consider documenting this inconsistency.

Description

In SecurityToken contract, burn() and burnWithMemo() call _requireUnpaused() for IB20.PausableFeature.BURN before calling _burn(). However, _burn() routes through _update(), which already checks the same burn pause state when to == address(0). This leads to successful burns paying unnecessary gas, and the duplicated check adds maintenance noise without changing behavior.

Recommendation

Remove the redundant _requireUnpaused() calls and rely on _update().

- _requireUnpaused(IB20.PausableFeature.BURN);  _burn(msg.sender, amount);

Description

In SecurityToken and TokenSupplyManager contracts, several state-changing operations lack variants that accept a memo. While mintWithMemo() and burnWithMemo() exist, there are no equivalent functions for batchMint(), burnBlocked(), or create() in TokenSupplyManager.

Recommendation

Consider adding batchMintWithMemo(), burnBlockedWithMemo(), and createWithMemo() to provide a consistent API surface for memoized operations across all token flows.

Coinbase: This is a B20 Parity point, there is no withMemo in those ABIs, and create is an admin OP, so we don't expect to need withMemo.

Cantina: Acknowledged.

Description

In the TokenSupplyManager contract, several administrative functions omit a check for whether the target token is registered (_storage().state[token].registered). These functions include setMinimumRedeemableShares(), grantRedeemPauser(), revokeRedeemPauser(), pauseRedeem(), unpauseRedeem(), removeCaller(), and replenishAllowance().

This can lead to an admin accidentally configuring state, such as pausers or redeem settings, for a token that has been deregistered or does not exist. While this does not pose a direct security risk, it can leave confusing stale configuration for an inactive token. Moreover, if the same token address is later re-registered, those previously applied settings will become active and affect the new registration unexpectedly.

Recommendation

Consider adding the TokenNotRegistered check to these administrative functions to ensure state is only modified for active tokens.

Description

SecurityToken claims IB20Asset conformance (which is IB20), but implements none of the IB20 supply-cap surface: there is no supplyCap storage field, no supplyCap() view, no updateSupplyCap() setter, and no SupplyCapExceeded check on any mint path.

IB20 mandates that all the mint functions revert SupplyCapExceeded when totalSupply + amount > supplyCap.

Recommendation

Implement the full IB20 supply-cap surface. Alternatively, if the omission is intentional, document it explicitly as a deliberate divergence and remove updateSupplyCap references from the B20AssetFactory contract.

Description

IB20Asset specifies that when an inner call dispatched by announce reverts, the contract reverts with InternalCallFailed(call) and "the inner revert reason is not bubbled".

SecurityToken.announce inverts this: on a failed inner delegatecall it bubbles the inner revert data verbatim when returndata is present, and only falls back to InternalCallFailed(internalCalls[i]) when the inner call reverts with empty returndata.

Recommendation

Decide which behavior is canonical and align the two.

Coinbase: Understood, we want to allow the revert to bubble up for off-chain access. We can't fix B20 ourselves.

Description

On HyperLiquid, bridging a linked token from HyperCore into HyperEVM is not atomic and has no retry or idempotency.

The sequence is:

1. HyperCore debits (burns) the Core side first, then
2. HyperCore calls transfer(recipient, amount) on the HyperEVM ERC-20 as the token's system address to credit the recipient.

If step 2 reverts, HyperCore does not retry or roll back step 1. Since the Core-side debit has already happened, so the bridged tokens are irrecoverably destroyed.

The HyperEVM SecurityToken is pausable per-operation (TRANSFER / MINT / BURN) via the base _update() gate, and it starts fully paused at initialization (operators must explicitly unpause each feature).

The inbound bridge credit is an ordinary transfer, so it routes through _update to _requireUnpaused(TRANSFER). Whenever TRANSFER is paused - at launch before unpausing, or any time an operator pauses transfers (e.g. during an incident) every Core to EVM bridge-in reverts on the credit leg and the user's bridged tokens are permanently lost, even though the user did nothing wrong.

There is no on-chain recourse: the revert does not block the move, it burns the funds. This makes a routine, intended administrative action (pausing transfers) silently destructive to honest users who bridge in during the pause window.

The issue was identified by the Coinbase team during the course of the review.

Recommendation

Exempt the inbound system-address credit leg from the pause gate so the credit settles instead of reverting, while keeping all compliance (sanctions/policy) checks intact.