The holder's refusal reports success for Special badges it does not bind
The holder's refusal reports success for Special badges it does not bind. Final disposition: Fixed.
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
What's wrong
A holder refusing an operator that holds CLASS_MASK_ALL gets every signal of success:
- the transaction succeeds —
_setSettlementOptOutonly refuses a mask of exactlyCLASS_MASK_SPECIAL, so an all-class operator is a permitted target SettlementOptOutSetis emittedisApprovedForAll(holder, operator)returnsfalse
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.
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.
Recommendation
Make the partiality legible. Any of these is enough:
- emit the scope:
SettlementOptOutSet(holder, operator, optedOut, effectiveMask)where the mask excludesCLASS_MASK_SPECIAL - add a view that answers per id —
operatorMayMove(holder, operator, id)— and point integrators at it instead ofisApprovedForAll - at minimum, state it in the natspec of
setSettlementOptOutin 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).
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.
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.