Skip to content

fix(token)!: reject contract mint/burn recipients - #833

Open
0xisk wants to merge 1 commit into
release/v0.3.0-alpha.1-rc.0from
fix/audit-nst-h3-rc0
Open

fix(token)!: reject contract mint/burn recipients#833
0xisk wants to merge 1 commit into
release/v0.3.0-alpha.1-rc.0from
fix/audit-nst-h3-rc0

Conversation

@0xisk

@0xisk 0xisk commented Aug 27, 2026

Copy link
Copy Markdown
Member

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update (if none of the other choices apply)

Fixes #794

Based on release/v0.3.0-alpha.1-rc.0, not main. That branch predates #694, so its test files carry no live-backend plumbing; the main-based version of the same change sits on fix/audit-nst-h3 (463f794d) for when the release line is re-cut.

This closes the contract arm rather than implementing the two-contract claim the finding sketches (transientCommit plus the recipient's own claimContractCall). That pattern needs a call into the recipient contract, which a library module cannot make on its caller's behalf. Nothing about the arm is persisted, so it can be widened again once contract-to-contract calls land, with no state migration.

Breaking: pass a bare ZswapCoinPublicKey where an Either used to go.

Not visible in the diff:

  • The mint-to-contract and zero-address tests are deleted, not replaced. The narrowed parameter type is the guarantee, and the retired shape is reachable only by casting through it, so a test would exercise compactc's argument marshaller rather than this module.
  • Live mint→burn round-trips with coin-public-key recipients pass against a local node.
  • The finding's full derivation against the ledger and compiler sources is written up locally, not included here. Say if it should land under docs/.

PR Checklist

A coin addressed to a contract must be claimed as a shielded receive by
that contract's own call in the same transaction (the ledger's
unconditional effects check). The module cannot arrange that claim
without cross-contract calls, so a contract recipient made every
_mint/_burn transaction malformed (audit H-03).

* Narrow `recipient`/`refundTo` from
  `Either<ZswapCoinPublicKey, ContractAddress>` to `ZswapCoinPublicKey`
  in NativeShieldedTokenCore and both flavors; wrap with `left<...>` at
  the protocol boundary and guard zero keys with `Utils_isKeyZero`.
* Update the mocks, simulators, and supply-extension doc examples to the
  narrowed signatures; refresh the stale `@circuitInfo` rows.
* Drop the mint-to-contract and zero-address tests: the narrowed
  parameter type is the guarantee, and the retired shape is reachable
  only by casting through it.
@0xisk
0xisk requested review from a team as code owners August 27, 2026 08:18
@coderabbitai

coderabbitai Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2f1b808e-6916-4f2f-84de-c142fb3eed1a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant