Skip to content

OpenZeppelin-Remediation.md

Latest commit

 

History

History
74 lines (50 loc) · 15.2 KB

OpenZeppelin-Remediation.md

File metadata and controls

74 lines (50 loc) · 15.2 KB
Raw

OpenZeppelin Audit (v0.3.0) — Remediation Response

Audit: CMTAT Confidential Audit, OpenZeppelin Security, June 24, 2026 Audited commit: 463087c — repo CMTA/CMTAT-Confidential Remediation branch: audit-fix

PR: all fixes below are bundled in a single pull request from audit-fix. Replace PR #NN / the PR URL placeholder with the actual pull request once opened: https://github.com/taurushq-io/CMTATFHE/pull/NN

Summary

Issue ID Issue Title Commit Comment
M-01 Rule Engine Not Applied to Mint and Burn Leaving Issuance and Redemption Unscreened 4e559b1, 0b10f17 Fixed. RuleEngine screening is now applied to mint and burn in the RuleEngine variant via _validateMint/_validateBurn, matching standard CMTAT's ruleEngine.transferred chokepoint. Following CMTAT v3.3.0, the operator (_msgSender()) is forwarded as the spender and address(0) as the mint/burn leg, exactly as CMTAT's _mintOverride/_burnOverride do (4-arg transferred); rules must exempt the spender on those legs, as production RuleWhitelist does. Forced operations (forcedTransfer/forcedBurn) intentionally continue to bypass the engine. Regression tests cover blocked/allowed mint and burn, the operator-as-spender forwarding, the forced-ops bypass, and engine-disabled behaviour.
L-01 Sequential Total Supply Disclosures Leak Individual Mint and Burn Amounts de3f439 Accepted as residual risk; mitigated by documentation. The cross-publication delta-inference channel (|V2 − V1|) cannot be fully closed in code — a SUPPLY_PUBLISHER_ROLE holder can always disclose the aggregate. A counter ≥ k gate on publishTotalSupply() would give k-anonymity against external observers, but it only hides the exact split (not the per-window sum/magnitude) and — decisively — trades away the transparent, on-demand total supply, since the exact figure cannot be published right after a single operation. It was therefore deliberately not implemented (see L-01 — In-code mitigation analysis). (It does not need to defend against the trusted MINTER_ROLE/BURNER_ROLE operators, who already know their own operations.) We also prefer to leave the behaviour as-is because CMTAT-Confidential is a library used by many issuers with different goals: some may deliberately want to publish the total supply after every mint/burn for maximal transparency, accepting the delta-inference leak as a conscious trade-off, and a hard-coded gate would deny them that. It is now documented in the publishTotalSupply NatSpec, the module docstring, the README operator guidance, together with the recommended operational mitigations: aggregate many supply-changing operations per disclosure, and restrict SUPPLY_PUBLISHER_ROLE to a multisig/timelock rather than a high-frequency automated caller.
L-02 ERC7984TokenAttributeModule Seeds Attributes via a Skippable Internal Initializer 8eb95f2 Fixed. The skippable _initTokenAttributes internal initializer was replaced with a constructor on ERC7984TokenAttributeModule, and the module was added to CMTATConfidentialBase's constructor inheritance list. Solidity constructor-chaining now forces every inheriting variant to seed name/symbol at compile time; a variant that omits it no longer compiles instead of deploying with empty attributes.
N-01 Missing Docstrings 8d283f6 Fixed. NatSpec was added to the eight role constants (MINTER_ROLE, BURNER_ROLE, FORCED_OPS_ROLE, OBSERVER_ROLE, SUPPLY_PUBLISHER_ROLE, RULE_ENGINE_ROLE, TOKEN_ATTRIBUTE_ROLE, SUPPLY_OBSERVER_ROLE), to version() (documented on CMTATConfidentialVersionModule, referencing ERC-8303), to supportsInterface, and to the CMTATConfidential confidentialTransfer*/decimals/name/symbol overrides (via @inheritdoc).
N-02 Incomplete Docstrings 20a87db Fixed. Added the missing @param/@return tags to canTransfer (base, RuleEngine and Whitelist variants), canTransferFrom (RuleEngine variant), and setRuleEngine (newRuleEngine param). The intentionally-ignored amount argument remains unnamed and therefore carries no @param, as documented in the @dev note.
N-03 Floating Pragma — Won't fix (by design). CMTAT-Confidential is intended to be consumed as a library, not only as a fixed set of pre-compiled deployables. Pinning pragma solidity 0.8.27; would force every downstream integrator to compile with exactly that version; the floating ^0.8.27 is deliberate so library users can choose the 0.8.x compiler appropriate for their own project. Deployers who require a reproducible build should pin the compiler version in their own build configuration (this repository currently compiles and tests against 0.8.34).
N-04 Prefix Increment Operator ++i Can Save Gas in Loops 28090a8 Fixed. Changed the post-increment i++ to pre-increment ++i in ERC7984TotalSupplyViewModule._updateTotalSupplyObserversAcl (the only post-increment loop in the codebase).
N-05 Misleading Documentation 104218a Fixed, all four items: (1) added the silent-refund-failure warning to both confidentialTransferFromAndCall overloads so they are not treated as atomic pay-and-call primitives; (2) corrected the _afterBurn comment to state it calls ERC7984BurnModule._afterBurn directly (both base hooks are empty, so no chain is preserved); (3) corrected the CMTATConfidential header and section comments to reflect the explicit CMTATConfidentialBase.<fn>(...) call style rather than super; (4) reworded the ERC7984TokenAttributeModule docstring (base-constructor invocation, not a super call — done as part of L-02).

Notes

  • Commit links point to taurushq-io/CMTATFHE. Each commit is self-contained for its finding, except where noted (the N-05 item 4 wording was corrected in the L-02 commit 8eb95f2).
  • N-03 has no commit: it is a deliberate no-change decision. The rationale in its Comment cell is the statement to quote in the issue update.
  • L-01 is a documentation/operational remediation; its Comment is the statement to quote in the issue update. The claim that it "cannot be closed in code" is an over-simplification — a partial in-code mitigation exists but was deliberately not implemented. See L-01 — In-code mitigation analysis below.
  • Detailed per-finding analysis and code excerpts are in feedback.md.

L-01 — In-code mitigation analysis

An earlier framing stated the cross-publication delta channel "cannot be prevented in code." That is too strong. A concrete in-code mitigation was considered:

Maintain an operation counter incremented on every mint/burn (in _afterMint/_afterBurn). Require counter ≥ k in publishTotalSupply() and reset it to 0 on each disclosure, so every published window aggregates at least k supply-changing operations.

Why it is a real mitigation (against external observers)

The channel is |V2 − V1| = the net of mint/burn amounts between two disclosures. The counter gate helps precisely because of two properties of this contract:

  1. Supply-changing operations are privileged. mint is onlyMinter, burn is onlyBurner, and there is no public self-burn path. Total supply changes only via role-gated calls. A non-privileged observer therefore cannot inject a known operation into a disclosure window to collapse the anonymity set.

  2. Disclosure windows are disjoint. Because publishTotalSupply() resets the counter, each mint/burn amount contributes to exactly one delta equation:

    delta_1 = a1 + a2         (window 1, ≥ k ops)
delta_2 = a3 + a4 + a5    (window 2, ≥ k ops)

    Every unknown appears in a single equation and each equation has ≥ k unknowns, so the system is underdetermined — no cross-window pivot recovers an individual aᵢ. Disclosure is itself role-gated (SUPPLY_PUBLISHER_ROLE), so an outsider cannot even choose the window boundaries.

Against a non-privileged external observer, therefore, a counter ≥ k gate provides genuine k-anonymity on individual mint/burn amounts. The original "one operation in between fully reveals the amount" case is blocked outright by k = 2.

Why it is not a complete fix (and was not implemented)

  1. The operator boundary is not the one being defended. MINTER_ROLE/BURNER_ROLE are trusted roles, and an operator already knows the amount of an operation it performed — so "the gate does not hide the amount from the operator" is not a real confidentiality loss. The confidential-amount guarantee is against the public / external observers, not against the trusted issuer, and the gate does defend that boundary. The operator-knowledge angle only bites in the unusual model of multiple mutually-distinct issuers who must stay blind to each other (a minter subtracting their own amount to recover another's); in the standard single-issuer trust model it is a non-issue and should not be counted against the gate.
  2. k-anonymity, not zero leakage. A window still discloses the sum of its operations, hence an upper bound on each and the order of magnitude of a dominant one (e.g. {large mint, dust burn}). It hides the exact split, not the aggregate.
  3. Side knowledge degrades it. An observer with ACL view access to a participant's balance, or off-chain knowledge of one operation's amount, effectively "knows" one term and reduces the window's anonymity by one — the same effect as injection, but requiring no role.
  4. Cost. One additional SSTORE on every mint/burn (a hot path) plus a liveness constraint: no disclosure is possible until k operations have accrued.

The liveness / freshness trade-off

The gate restricts disclosure, not mint/burn. The counter is incremented inside mint/burn, but the counter >= k check lives in publishTotalSupply(). Mint and burn always execute — they are never blocked or reverted by the gate; they merely bump the counter and pay one extra SSTORE. What is withheld is the disclosure: publishTotalSupply() reverts until enough supply-changing operations have accrued. This directly conflicts with the purpose of publishTotalSupply() — an exact, auditable, on-demand supply figure. The two goals ("publish the exact current supply whenever asked" and "never let a delta reveal a single operation") are opposed: the counter buys privacy by spending freshness. Concretely, the token can no longer offer a transparent, always-available total supply: right after a single mint or burn the counter is 1, so publishTotalSupply() reverts and the up-to-date supply cannot be disclosed until a further supply-changing operation occurs.

  • On-demand / scheduled disclosure breaks. With a fixed cadence (e.g. a daily dashboard or regulator feed), any period with fewer than k supply-changing operations cannot be published — the call reverts until the k-th operation occurs, which may be never on a quiet day. The current supply is simply unavailable in that window.
  • Accuracy is preserved, only timeliness is lost. The counter does not round or perturb the value. When a disclosure clears the gate, V2 is the exact real supply — the figure is always true as of the moment it publishes. What is lost is freshness, not correctness.
  • Permanent-block failure mode. A token that mints once and then stays quiet can never publish its supply under k = 2. Avoiding this needs an escape hatch (admin override or a time-based fallback), which re-opens the very leak the gate was meant to close.

This trade-off is the strongest argument against implementing the gate here: the feature exists specifically to provide an exact, on-demand figure for a regulated security token, and "you may only publish when the protocol decides the timing is safe" defeats that purpose. The operational mitigation — a multisig/timelock SUPPLY_PUBLISHER_ROLE that chooses to batch operations before disclosing — achieves the same privacy goal while keeping disclosure exact and available on demand when the operator judges it safe, with no SSTORE-per-operation cost and no permanent-block failure mode.

Decision

The counter gate does defend the boundary that matters (external observers), but it is only a partial hardening — it still leaks the per-window sum (k-anonymity, not zero leakage) and is degraded by side knowledge — and it carries a permanent SSTORE on every mint/burn plus, decisively, the loss of a transparent on-demand total supply: the exact figure cannot be published right after a single mint or burn. Since the entire purpose of publishTotalSupply() is an exact, auditable, on-demand figure, these trade-offs outweigh the marginal benefit. Moreover, CMTAT-Confidential is intended to be consumed as a library by many different issuers, each with their own disclosure needs and cadence; baking a fixed counter policy (and its liveness/gas trade-offs) into the module would impose one disclosure discipline on every downstream deployment. Some issuers may deliberately want to publish the total supply after every mint/burn for maximal, per-operation transparency, accepting the delta-inference leak as a conscious trade-off; a hard-coded counter ≥ k gate would make that impossible. We prefer to leave the behaviour as-is for now and let each integrator choose the disclosure governance that fits their use case, rather than impose a single solution. The finding therefore remains an accepted residual risk mitigated operationally, with no code change at this time. The precise framing to use in the issue update is:

The cross-publication delta channel cannot be fully closed in code. A counter ≥ k gate on publishTotalSupply() would give k-anonymity for mint/burn amounts against external observers (supply-changing ops are role-gated and windows are disjoint), but it only hides the exact split, not the per-window sum/magnitude, is degraded by an observer with side knowledge, and — decisively — costs the token its transparent, on-demand supply, since the exact figure cannot be published right after a single operation, at an SSTORE per mint/burn. (It does not need to hide amounts from the MINTER_ROLE/BURNER_ROLE operators, who are trusted and already know their own operations.) The residual risk is accepted and mitigated operationally: aggregate many operations per disclosure and restrict SUPPLY_PUBLISHER_ROLE to a multisig/timelock.