Skip to content

feat(policy): enforce TRANSFER_EXECUTOR_POLICY on every transfer path #224

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rayyan224 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(policy): enforce TRANSFER_EXECUTOR_POLICY on every transfer path #224

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/interfaces/IB20.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,8 @@ interface IB20 {
/// @return Policy scope constant.
function TRANSFER_RECEIVER_POLICY() external view returns (bytes32);

/// @notice Policy slot consulted against `msg.sender` on `transferFrom` when distinct from `from`.
/// Not consulted on `transfer`.
/// @notice Policy slot consulted against `msg.sender` (the initiator) on every transfer,
/// including when `msg.sender == from`.
/// @dev Bypassed for factory-originated calls during the creation (bootstrap) window; see
/// `IB20Factory.createB20`.
/// @return Policy scope constant.
Expand Down Expand Up @@ -317,6 +317,7 @@ interface IB20 {
/// @dev Reverts with `ContractPaused(TRANSFER)` when `TRANSFER` is paused.
/// @dev Reverts with `InvalidReceiver` when `to == address(0)`.
/// @dev Reverts with `InvalidSender` when `msg.sender == address(0)`.
/// @dev Reverts with `PolicyForbids(TRANSFER_EXECUTOR_POLICY, ...)` when `msg.sender` is not authorized.
/// @dev Reverts with `PolicyForbids(TRANSFER_SENDER_POLICY, ...)` when `msg.sender` is not authorized.
/// @dev Reverts with `PolicyForbids(TRANSFER_RECEIVER_POLICY, ...)` when `to` is not authorized.
/// @dev Reverts with `InsufficientBalance` when `msg.sender`'s balance is below `amount`.
Expand All @@ -333,7 +334,7 @@ interface IB20 {
/// @dev Reverts with `InvalidReceiver` when `to == address(0)`.
/// @dev Reverts with `InvalidSender` when `from == address(0)`.
/// @dev Reverts with `InsufficientAllowance` when the caller's allowance from `from` is below `amount`.
/// @dev Reverts with `PolicyForbids(TRANSFER_EXECUTOR_POLICY, ...)` when `msg.sender != from` and `msg.sender` is not authorized.
/// @dev Reverts with `PolicyForbids(TRANSFER_EXECUTOR_POLICY, ...)` when `msg.sender` is not authorized.
/// @dev Reverts with `PolicyForbids(TRANSFER_SENDER_POLICY, ...)` when `from` is not authorized.
/// @dev Reverts with `PolicyForbids(TRANSFER_RECEIVER_POLICY, ...)` when `to` is not authorized.
/// @dev Reverts with `InsufficientBalance` when `from`'s balance is below `amount`.
Expand Down
52 changes: 20 additions & 32 deletions test/lib/mocks/MockB20.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,21 +198,11 @@ abstract contract MockB20 is IB20 {
_requireNonZeroActors(from, to);
// Allowance is consumed unconditionally — including during the factory
// bootstrap window (`_isPrivileged()`). Matches the Rust precompile,
// which carves no `privileged` exception for allowance accounting;
// only the executor-policy check below is bypassed
// for a privileged caller. An infinite allowance is still not
// decremented (handled inside `_consumeAllowance`).
// which carves no `privileged` exception for allowance accounting. An
// infinite allowance is still not decremented (handled inside
// `_consumeAllowance`). The executor policy is enforced centrally in
// `_transfer` (on `msg.sender`), which honors the bootstrap bypass.
_consumeAllowance(from, msg.sender, amount);
if (!_isPrivileged() && msg.sender != from) {
// Read the executor policy ID out of the transfer-side packed
// slot. Cold here; warm by the time _transfer reads the same
// slot for sender + receiver. Skipped when the caller is the
// owner — sender-policy already covers `from` inside _transfer.
uint64 executorPolicyId = MockB20Storage.layout().transferPolicyIds.executor;
if (!IPolicyRegistry(POLICY_REGISTRY).isAuthorized(executorPolicyId, msg.sender)) {
revert PolicyForbids(TRANSFER_EXECUTOR_POLICY, executorPolicyId);
}
}
_transfer(from, to, amount);
return true;
}
Expand Down Expand Up @@ -247,16 +237,10 @@ abstract contract MockB20 is IB20 {
{
_requireNonZeroActors(from, to);
// Allowance is consumed unconditionally — including during the factory
// bootstrap window — matching the Rust precompile.
// Only the executor-policy check below is bypassed for a privileged
// caller; infinite allowance is still not decremented.
// bootstrap window — matching the Rust precompile. Infinite allowance
// is still not decremented. The executor policy is enforced centrally
// in `_transfer` (on `msg.sender`), which honors the bootstrap bypass.
_consumeAllowance(from, msg.sender, amount);
if (!_isPrivileged() && msg.sender != from) {

@rayyan224 rayyan224 Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed here, becaues repulled in the helper no need for an extra SLOAD

uint64 executorPolicyId = MockB20Storage.layout().transferPolicyIds.executor;
if (!IPolicyRegistry(POLICY_REGISTRY).isAuthorized(executorPolicyId, msg.sender)) {
revert PolicyForbids(TRANSFER_EXECUTOR_POLICY, executorPolicyId);
}
}
_transfer(from, to, amount);
emit Memo(msg.sender, memo);
return true;
Expand Down Expand Up @@ -754,18 +738,22 @@ abstract contract MockB20 is IB20 {
/// this helper. `transferFrom` / `transferFromWithMemo`
/// additionally consume the allowance (unconditionally —
/// including in the bootstrap window, matching the Rust
/// precompile) and check the executor
/// policy in their bodies before calling here; only the
/// executor-policy check honors the bootstrap bypass,
/// consistent with the sender/receiver policy bypass below.
/// precompile) before calling here.
///
/// Enforces the executor (`msg.sender`), sender (`from`), and receiver
/// (`to`) policies. Gating the executor here — not just on delegated
/// `transferFrom` — lets an executor allowlist restrict who may
/// initiate any transfer, including a holder moving their own tokens.
/// All honor the bootstrap bypass; an unset lane is always-allow.
function _transfer(address from, address to, uint256 amount) internal {
if (!_isPrivileged()) {
// One SLOAD pulls both policy IDs we need for the transfer
// check (and was already warmed if we came in via transferFrom,
// which reads the executor lane of the same slot first).
// Solidity emits a single SLOAD for the struct read + masked
// extracts for the named fields.
// One SLOAD pulls all three policy IDs we need for the transfer
// check. Solidity emits a single SLOAD for the struct read +
// masked extracts for the named fields.
MockB20Storage.TransferPolicyIds memory packed = MockB20Storage.layout().transferPolicyIds;
if (!IPolicyRegistry(POLICY_REGISTRY).isAuthorized(packed.executor, msg.sender)) {
revert PolicyForbids(TRANSFER_EXECUTOR_POLICY, packed.executor);
}
if (!IPolicyRegistry(POLICY_REGISTRY).isAuthorized(packed.sender, from)) {
revert PolicyForbids(TRANSFER_SENDER_POLICY, packed.sender);
}
Expand Down
6 changes: 3 additions & 3 deletions test/lib/mocks/MockB20Storage.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ library MockB20Storage {
// not declared as a field is simply uninitialized (zero) and the
// struct cannot accidentally write to it.

/// @notice Transfer-side policy IDs (read by `_transfer` and `transferFrom*`).
/// @notice Transfer-side policy IDs (all three lanes read by `_transfer`).
/// @dev Bit layout (Solidity LSB-first):
/// bits 0.. 63 : sender
/// bits 64..127 : receiver
Expand Down Expand Up @@ -119,8 +119,8 @@ library MockB20Storage {
// access (`$.transferPolicyIds.sender = id;`) instead of inline
// shifts and mask operations.
//
// Transfer-side policies (read by `_transfer`, `transferFrom*`,
// and the blocked check in the deprecated `burnBlocked`).
// Transfer-side policies (read by `_transfer`, and the sender lane
// by the blocked check in the deprecated `burnBlocked`).
TransferPolicyIds transferPolicyIds;
// Mint-side policies (read by `_mint`). Only `MINT_RECEIVER_POLICY`
// is defined today; future granular mint-side policy types (e.g.
Expand Down
90 changes: 90 additions & 0 deletions test/unit/B20/erc20/transfer.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,28 @@ contract B20TransferTest is B20Test {
token.transfer(to, amount);
}

/// @notice Verifies transfer reverts when the executor (msg.sender) is not authorized under
/// TRANSFER_EXECUTOR_POLICY
/// @dev On the direct `transfer` path the executor is `msg.sender` (== `from`). The executor
/// gate is enforced in `_transfer` before the sender/receiver gates, so a blocked executor
/// reverts PolicyForbids(TRANSFER_EXECUTOR_POLICY, ...) even for a holder moving their own
/// tokens. No balance needed — the policy check fires first.
function test_transfer_revert_executorPolicyForbids(address from, address to, uint256 amount) public {
_assumeValidActor(from);
_assumeValidActor(to);
_setPolicy(B20Constants.TRANSFER_EXECUTOR_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID);

vm.prank(from);
vm.expectRevert(
abi.encodeWithSelector(
IB20.PolicyForbids.selector,
B20Constants.TRANSFER_EXECUTOR_POLICY,
PolicyRegistryConstants.ALWAYS_BLOCK_ID
)
);
token.transfer(to, amount);
}

/// @notice Verifies transfer reverts when sender balance is insufficient
/// @dev Balance precondition; checks InsufficientBalance(sender, balance, amount) error
function test_transfer_revert_insufficientBalance(address from, address to, uint256 amount) public {
Expand Down Expand Up @@ -299,6 +321,74 @@ contract B20TransferTest is B20Test {
token.transfer(to, amount);
}

/// @notice Verifies transfer succeeds when the executor (msg.sender) is a member of a custom
/// ALLOWLIST policy
/// @dev Exercises the external-registry authorization path for the executor scope: only an
/// allowlisted initiator can move tokens. Here the holder `from` is on the allowlist, so
/// their own `transfer` clears the executor gate.
function test_transfer_success_externalExecutorPolicyAllows(address from, address to, uint256 amount) public {
_assumeValidActor(from);
_assumeValidActor(to);
vm.assume(from != to);
amount = bound(amount, 0, B20Constants.MAX_SUPPLY_CAP);

uint64 id = _createAllowlist(from, true);
_setPolicy(B20Constants.TRANSFER_EXECUTOR_POLICY, id);
_mint(from, amount);

vm.prank(from);
token.transfer(to, amount);

assertEq(token.balanceOf(to), amount, "transfer must succeed when executor is allowlisted");
}

/// @notice Verifies transfer reverts when the executor (msg.sender) is NOT a member of a custom
/// ALLOWLIST policy
/// @dev Negative external-registry path for the executor scope: an allowlist without membership
/// for `from` resolves isAuthorized to false, so the executor gate reverts PolicyForbids
/// with the custom id. This is the case an issuer uses to restrict transfers to specific
/// initiators (e.g. a settlement contract). No balance needed — the policy check fires first.
function test_transfer_revert_externalExecutorPolicyDenies(address from, address to, uint256 amount) public {
_assumeValidActor(from);
_assumeValidActor(to);
vm.assume(from != to);

uint64 id = _createAllowlist(from, false); // create the allowlist but do NOT add `from`
_setPolicy(B20Constants.TRANSFER_EXECUTOR_POLICY, id);

vm.prank(from);
vm.expectRevert(abi.encodeWithSelector(IB20.PolicyForbids.selector, B20Constants.TRANSFER_EXECUTOR_POLICY, id));
token.transfer(to, amount);
}

/// @notice Verifies a privileged (factory bootstrap) transfer bypasses the TRANSFER_EXECUTOR_POLICY
/// @dev Executor mirror of the sender/receiver bootstrap bypasses: the initCalls set the executor
/// policy to ALWAYS_BLOCK and transfer from the factory. A non-privileged transfer would
/// revert PolicyForbids(EXECUTOR, ...); the privileged init-call transfer must succeed,
/// proving the executor gate honors the bootstrap bypass on the direct transfer path. Runs
/// the real factory bootstrap path with no vm.store cheat, so it holds under LIVE_PRECOMPILES.
function test_transfer_success_privilegedBypassesExecutorPolicy(address to, uint256 amount) public {
_assumeValidActor(to);
amount = bound(amount, 0, B20Constants.MAX_SUPPLY_CAP);

bytes32 salt = keccak256("privileged-executor-bypass");
// The fuzzed recipient must not collide with the to-be-created token's own address.
vm.assume(to != factory.getB20Address(IB20Factory.B20Variant.ASSET, alice, salt));

bytes[] memory initCalls = new bytes[](3);
initCalls[0] = abi.encodeWithSelector(IB20.mint.selector, address(factory), amount);
initCalls[1] = abi.encodeWithSelector(
IB20.updatePolicy.selector, B20Constants.TRANSFER_EXECUTOR_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID
);
initCalls[2] = abi.encodeWithSelector(IB20.transfer.selector, to, amount);

address newToken = _createAsset(alice, salt, _assetParams(), initCalls);

assertEq(
IB20(newToken).balanceOf(to), amount, "privileged transfer must succeed despite blocked executor policy"
);
}

/// @notice Creates a custom ALLOWLIST policy administered by `admin`, optionally seeding
/// `member`, and returns its id. Drives the external-registry authorization path
/// (custom policy id) beyond the ALWAYS_ALLOW / ALWAYS_BLOCK sentinels.
Expand Down
23 changes: 16 additions & 7 deletions test/unit/B20/erc20/transferFrom.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,11 +347,15 @@ contract B20TransferFromTest is B20Test {
assertEq(token.balanceOf(to), spendAmount, "to must receive the spent amount");
}

/// @notice Verifies transferFrom with self-caller skips the executor policy check
/// @dev Self-caller is not an executor distinct from `from`; sender-policy already
/// covers `from` inside _transfer. Executor policy MUST NOT fire — pins the
/// one carve-out we intentionally keep around `msg.sender == from`.
function test_transferFrom_success_selfCaller_skipsExecutorPolicy(address from, address to, uint256 amount) public {
/// @notice Verifies transferFrom with a self-caller is still gated by the executor policy
/// @dev Executor enforcement is centralized in `_transfer` on `msg.sender`, so the old
/// `msg.sender == from` carve-out is gone: a holder moving their own tokens via
/// transferFrom must also clear TRANSFER_EXECUTOR_POLICY. This closes the bypass where
/// an executor allowlist could be sidestepped by routing a self-transferFrom. Allowance
/// is self-approved so the executor check — not the allowance gate — is what fires.
function test_transferFrom_revert_selfCaller_executorPolicyForbids(address from, address to, uint256 amount)
public
{
_assumeValidActor(from);
_assumeValidActor(to);
vm.assume(from != to);
Expand All @@ -363,9 +367,14 @@ contract B20TransferFromTest is B20Test {
_setPolicy(B20Constants.TRANSFER_EXECUTOR_POLICY, PolicyRegistryConstants.ALWAYS_BLOCK_ID);

vm.prank(from);
vm.expectRevert(
abi.encodeWithSelector(
IB20.PolicyForbids.selector,
B20Constants.TRANSFER_EXECUTOR_POLICY,
PolicyRegistryConstants.ALWAYS_BLOCK_ID
)
);
token.transferFrom(from, to, amount);

assertEq(token.balanceOf(to), amount, "transfer must succeed despite blocked executor policy");
}

// ============================================================
Expand Down
35 changes: 17 additions & 18 deletions test/unit/B20/erc20/transferFrom_revertOrder.t.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,28 +9,28 @@ import {PolicyRegistryConstants} from "base-std-test/lib/mocks/MockPolicyRegistr

/// @title Differential check-order tests for `transferFrom`.
///
/// @notice `transferFrom` layers two body-level preconditions
/// (ALLOWANCE and EXECUTOR-POLICY) on top of `_transfer`'s
/// policy / balance checks. The PAUSE / ZERO-RECEIVER /
/// ZERO-SENDER guards run before the allowance / executor-policy
/// work in the entrypoint body.
/// @notice `transferFrom` consumes the allowance in the entrypoint body, then
/// defers to `_transfer` for the policy / balance checks. The PAUSE /
/// ZERO-RECEIVER / ZERO-SENDER guards run before the allowance work in
/// the entrypoint body; the EXECUTOR / SENDER / RECEIVER / BALANCE
/// checks all run inside `_transfer`, with EXECUTOR first.
///
/// **Canonical order (Solidity reference, when
/// `msg.sender != from`):**
/// **Canonical order (Solidity reference):**
/// 1. PAUSE (`whenNotPaused(TRANSFER)` modifier) → `ContractPaused`
/// 2. ZERO-RECEIVER (`to == address(0)`) → `InvalidReceiver`
/// 3. ZERO-SENDER (`from == address(0)`) → `InvalidSender`
/// 4. ALLOWANCE (`_consumeAllowance`) → `InsufficientAllowance`
/// 5. EXECUTOR-POLICY (`isAuthorized(executorPolicyId, msg.sender)`)
/// 5. EXECUTOR-POLICY (`_transfer` body: `isAuthorized(executor, msg.sender)`)
/// → `PolicyForbids(EXECUTOR, ...)`
/// 6..N. All `_transfer` body checks — see `transfer_revertOrder.t.sol`
/// 6..N. Remaining `_transfer` body checks — see `transfer_revertOrder.t.sol`
/// (SENDER-POLICY → RECEIVER-POLICY → BALANCE).
///
/// The full pair matrix between body-level ALLOWANCE/EXECUTOR-POLICY
/// and the PAUSE/ZERO-RECEIVER/ZERO-SENDER guards is pinned below;
/// one test against a representative `_transfer` body check
/// (SENDER-POLICY) proves ALLOWANCE and EXECUTOR-POLICY both
/// fire before `_transfer` is entered.
/// The executor gate is enforced on every transfer path, not only when
/// `msg.sender != from`; this suite exercises the delegated path with a
/// distinct caller (the self-caller case is pinned in `transferFrom.t.sol`).
/// The pair matrix between the body-level ALLOWANCE guard, the
/// PAUSE/ZERO-RECEIVER/ZERO-SENDER guards, and the leading `_transfer`
/// EXECUTOR-POLICY check is pinned below.
contract B20TransferFromRevertOrderTest is B20Test {
// --- Pairs where PAUSE wins (PAUSE is canonical first) ---

Expand Down Expand Up @@ -186,10 +186,9 @@ contract B20TransferFromRevertOrderTest is B20Test {

// --- Pair where EXECUTOR-POLICY wins (everything earlier satisfied) ---

/// @notice EXECUTOR-POLICY beats anything in `_transfer` (representative: SENDER-POLICY).
/// @dev Allowance is set high enough to pass the allowance check, so the
/// executor-policy check runs next and fires before `_transfer` is
/// entered.
/// @notice EXECUTOR-POLICY beats the other `_transfer` checks (representative: SENDER-POLICY).
/// @dev Allowance is set high enough to pass the allowance check, so `_transfer` is entered;
/// the executor gate is checked first inside `_transfer` and fires before SENDER-POLICY.
function test_transferFrom_revertOrder_executorPolicy_beats_transferBody(
address caller,
address from,
Expand Down
Loading
Loading