Pre-auditThree days of senior audit time for $1,500, credited in full toward your Zealynx audit.3 audit days, credited toward your auditSee how →
L-09·audit-finding

The holder's refusal reports success for Special badges it does not bind

Fixednfterc-1155marketplace
TL;DR

The holder's refusal reports success for Special badges it does not bind. Final disposition: Fixed.

Severity
LOW
Method
MManual review
CAT.

Tracker: issue #38 · Re-review ID: RR-03 · Status: Fixed · Verified at: generation-d

Severity: Low Location: src/PlakxioBadge.sol — setSettlementOptOut / setSettlementOptOutFor / isApprovedForAll Introduced by: the M-01 fix (#17) together with the relayed opt-out (#25), verified at audit-v3-rereview-handoff

02Section · What's wrong

What's wrong

A holder refusing an operator that holds CLASS_MASK_ALL gets every signal of success:

  • the transaction succeeds — _setSettlementOptOut only refuses a mask of exactly CLASS_MASK_SPECIAL, so an all-class operator is a permitted target
  • SettlementOptOutSet is emitted
  • isApprovedForAll(holder, operator) returns false

and the operator keeps moving that holder's Special badges, because _checkHolderPermits exempts the class.

The refusal does bind the operator's ordinary moves, which is the intended behaviour and is why the mask check is == and not & — that part is right and we verified it. The issue is purely that nothing tells the holder the refusal is partial.

03Section · Why it matters

Why it matters

This is the state a holder is most likely to be in: operators are granted CLASS_MASK_ALL in practice, so "I refused them" and "they can still take my Special" are simultaneously true, and every observable says the refusal took. An integrator reading isApprovedForAll is told the operator is not approved while a Special transfer from that operator will succeed — the view and the gate disagree in the direction that misleads.

No funds move that would not otherwise move; the harm is that a holder and an integrator both act on a protection they do not have.

04Section · Recommendation

Recommendation

Make the partiality legible. Any of these is enough:

  • emit the scope: SettlementOptOutSet(holder, operator, optedOut, effectiveMask) where the mask excludes CLASS_MASK_SPECIAL
  • add a view that answers per id — operatorMayMove(holder, operator, id) — and point integrators at it instead of isApprovedForAll
  • at minimum, state it in the natspec of setSettlementOptOut in the words a holder would read, and in the client surface that offers the refusal

Related: #37 (the underlying exemption) and #3 (the activation-delay floor that stands in for the missing control).

05Section · Remediation

Remediation

Fixed.

The remediation takes the two options this finding offered together: a per-badge view and the scope stated in the contract's own documentation. operatorMayMove(holder, operator, id) answers whether a given operator may move a given holder's badge now, and it is built by an external self-call into requireOperatorMayMove, so the transfer gate's own reverts become the answer rather than a separately maintained restatement of the policy that could drift from it. The gate itself is unchanged. isApprovedForAll now directs integrators to the per-badge view, and the refusal setter's documentation states, in a holder's terms, that the refusal does not reach Special badges and that the transaction, the event and isApprovedForAll will not say so.

We independently re-verified the failure mode the client's own adversarial round found in the first cut of this view: a gas scan across every value from 2,000 to 60,000 produced no misleading false answer — every read returned either the authority answer or a named refusal. A gas-starved read can therefore no longer report "cannot move" for a badge the operator can move, which would have reproduced this finding's harm inside the view that fixes it.

Retained and documented: an empty batch falls outside a single-token view's domain, and the settlement and market still restate the gate in their own readiness views rather than asking this one — consistent in the reviewed code, but a drift risk later. Client-facing refusal copy and its rendered behaviour were not verified from the contract repository; the client reports that copy as awaiting its own approval, and this result does not certify frontend adoption.

06Section · Verification provenance

Verification provenance

Final disposition: Fixed. Verified at generation-d, commit bcf18bb2926589bb8538fa8c322eabc871ff2801. This finding was assessed in the generation-d mitigation review. The report-wide scope and evidence limitations remain applicable; see the final report overview.

Status
Fixed
L-09