Early rate$2,400 of senior audit time for $500. Early members keep the rate as it climbs.$2,400 of senior audit time for $500See how →
F-2026-0001·access-control

Any whitelisted operator can take any holder's badges, and the holder's revocation is unreadable

Fixednfterc-1155marketplace
TL;DR

The badge reported every whitelisted operator as approved for every account, and nothing required that operator to be a contract, so a plain whitelisted address could move any holder's badges without a signature or payment. A holder's explicit refusal was written to storage but never read.

Severity
HIGH
Impact
HIGH
Likelihood
MEDIUM
Method
MManual review
CAT.
Complexity
MEDIUM
Exploitability
MEDIUM
02Section · Description

Description

isApprovedForAll returns true for any whitelisted operator against every account, unconditionally:

solidity
function isApprovedForAll(address account, address operator) public view override returns (bool) {
return isSettlement[operator] || super.isApprovedForAll(account, operator);
}

OpenZeppelin's ERC1155.safeTransferFrom gates on _checkAuthorized(_msgSender(), from), which consults exactly this override. The walled-garden gate that runs first checks only the operator's whitelist membership and the token's class bit:

solidity
function _checkOperatorMayMove(address operator, uint256 id) internal view {
if (!isSettlement[operator]) revert NotSettlementContract(operator);
_checkClassPermitted(operator, settlementClassMask[operator], id);
}

Neither path checks that from consented. settlementClassMask restricts which class an operator may move; nothing restricts whose badge. The security model therefore rests on an unstated property — that the whitelist only ever contains contracts whose own code enforces consent. PlakxioSettlement does enforce it, through EIP-712 order signatures. Nothing in PlakxioBadge requires a whitelisted address to.

setSettlementContract accepts a bare address with no code.length check and takes effect in the same transaction, so the whitelisted address need not be a contract at all.

Because the left operand of the || is already true, a holder calling setApprovalForAll(operator, false) gets a successful transaction and an ApprovalForAll event while the contract's answer stays true. The revocation is written to storage and never read.

Vulnerable Scenario:

  1. A holder owns badges across classes and pre-emptively calls setApprovalForAll(grantee, false).
  2. The admin calls setSettlementContract(grantee, true, CLASS_MASK_ALL) where grantee is a plain EOA.
  3. isApprovedForAll(holder, grantee) now returns true, overriding step 1.
  4. grantee calls safeBatchTransferFrom(holder, grantee, [normalId, premiumId, specialId], [1,1,1], ""). Whitelist membership passes, every class bit is set, and _checkAuthorized passes on the override.
  5. All three badges move. No order signature is presented and no payment is made.
  6. The holder cannot move a badge to safety: safeTransferFrom from their own address reverts NotSettlementContract, and no burn function exists.
03Section · Impact

Impact

Every badge holder loses every badge of every class named in the mask, without payment and without any signature of theirs. Special one-of-ones are included, so the ascending-floor mechanic is bypassed entirely for those tokens rather than ratcheted. Holders have no exit and no revocation the contract will honour.

Severity is adjusted one tier down from the matrix lookup because the path requires DEFAULT_ADMIN_ROLE — a fully-trusted role — to act outside its stated purpose. The trust assumption being violated is that the whitelist admits only consent-enforcing settlement contracts, which setSettlementContract does not check and cannot express.

04Section · Recommendation

Recommendation

Two options; they are independent and Option B preserves the no-approval seller UX.

Option A — require holder approval as well as the whitelist. The gate becomes a restriction rather than a grant, and a holder can revoke:

diff
function isApprovedForAll(address account, address operator) public view override returns (bool) {
- return isSettlement[operator] || super.isApprovedForAll(account, operator);
+ return isSettlement[operator] && super.isApprovedForAll(account, operator);
}

This is a design change rather than a patch: the current flow relies on sellers never needing an approval, so the change fails 82 tests in the existing suite until the seller-side approval is introduced end to end. Weigh it against Option B accordingly.

Option B — delay the grant and require code. Keeps the UX and turns an unobservable one-transaction confiscation into an announced, cancellable one:

diff
+uint256 public constant ACTIVATION_DELAY = 2 days;
+mapping(address account => uint256) public settlementActiveFrom;
if (allowed && classMask == 0) revert EmptyClassMask(settlement);
+if (allowed && settlement.code.length == 0) revert SettlementNotAContract(settlement);
+if (allowed) settlementActiveFrom[settlement] = block.timestamp + ACTIVATION_DELAY;
diff
function _checkOperatorMayMove(address operator, uint256 id) internal view {
if (!isSettlement[operator]) revert NotSettlementContract(operator);
+ if (block.timestamp < settlementActiveFrom[operator]) revert SettlementNotActiveYet(operator);
_checkClassPermitted(operator, settlementClassMask[operator], id);
}

Revocation must stay instant — it writes isSettlement = false and zeroes the mask, and no delay should apply to it.

Independently of either option, isApprovedForAll returning true for an operator whose class mask excludes the token is misleading to any integrator reading it as the ERC-1155 authority signal. Returning false when settlementClassMask[operator] == 0 costs nothing and removes the worst of that.

05Section · Resolution

Resolution

Three controls landed: setSettlementContract refuses an address with no code, a settlement grant waits an activation delay before it may move badges, and a holder's opt-out is now honoured on the view and on both the single and batch transfer paths. A zero class mask no longer reads as approved. Each step of the proof-of-concept was re-run against the fix and is closed.

One note for deployment: the activation delay is an immutable constructor argument with no lower bound, and a value of zero reinstates immediate activation on a contract that cannot be replaced without stranding every badge. A floor in the constructor would make the control code-enforced rather than deployment-dependent. Consent also remains opt-out rather than opt-in, so a holder who never refuses is still exposed to a bad whitelist entry — the residual is admin-gated, and the default admin role is itself now two-step and delayed.

06Section · Affected files

Affected files

  • src/PlakxioBadge.sol#L258-L259 and src/PlakxioBadge.sol#L300-L303 at commit 5c38893
Status
Fixed
F-2026-0001