Skip to content

Harden untrusted decode paths - #2

Open
mellowcroc wants to merge 2 commits into
mainfrom
harden/untrusted-decode-dos
Open

Harden untrusted decode paths#2
mellowcroc wants to merge 2 commits into
mainfrom
harden/untrusted-decode-dos

Conversation

@mellowcroc

@mellowcroc mellowcroc commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

Four consumer paths decoded ceremony-derived artifacts before checking the
length/count fields that drive allocation, letting a hostile or corrupt input
exhaust memory or panic — an unrecoverable throw on the native verifier, a
module abort on wasm.

  • prover.UnmarshalProof — preflight the BSB22 commitment-count prefix and
    the exact encoded length before gnark-crypto runs make([]G1Affine, count).
    Closes a remote, unauthenticated OOM on the verifier HTTP API (a ~200-byte
    body with a 0xFFFFFFFF count requested ~412 GB).

  • wasm-prover fetchCCS — bound the decoded CCS to its signed size
    (io.LimitReader), cap the zstd decoder window (WithDecoderMaxMemory, was
    64 GiB default), and recover() the decode so a hostile length prefix errors
    instead of aborting the module.

  • proofassets.ValidatePKIndexAllocations — bound NbWires,
    NbInfinityA/B, and NbCommitmentKeys against the signed FileSize and
    section geometry. Applied on the full-index paths (ReadPKIndex,
    streampk.ValidateIndex); the manifest digest covers only geometry, so the
    counters were otherwise free.

  • streampk domain decode — validate the FFT cardinality is canonical
    (power-of-two with a real generator, bounding it to the field's 2-adicity)
    before precomputing twiddles, so a hostile 2^32 cardinality is rejected
    before the ~274 GB allocation.

  • Reject untrimmed whitespace in attested string fields
    ContributionEnvironment.OS/.Architecture and audit findings used a plain
    == "" check, so a single space satisfied "must not be empty". Requires the
    trimmed, non-empty form, matching Identity.DisplayName and artifact names.
    (Moved here from PR Audit hardening: Ed25519 key validation and input-validation fixes #1 during a history rewrite; it was never part of the
    audit series.)

  • Keep on-curve validation when skipping subgroup checksKeySource.open
    decodes the G1 and G2 singletons with NoSubgroupChecks. Skipping the subgroup
    check is a deliberate throughput trade on a proving key callers are expected to
    digest-authenticate first, and internal/msmengine makes the same trade — but
    msmengine still runs IsOnCurve on every decoded point and streampk ran no
    validation at all. OpenKeyURL reaches this code over HTTP range requests, and
    the URL caller in cmd/wasm-prover does not digest the proving key before
    opening it, so a point that parsed but was not on the curve entered a
    multi-scalar multiplication unchallenged. This does not close that missing
    digest verification, which needs a separate change.

Adds regression tests for the proof and PK-index paths.

Independent of #1: the two PRs now touch no files in common. #1 is ceremony code
(internal/mpcceremony, cmd/mpc-ceremony, docs); this PR is the proving and
artifact-decode surface. They can land in either order.

@mellowcroc
mellowcroc changed the base branch from main to audit/ed25519-key-validation August 13, 2026 09:47
@mellowcroc
mellowcroc force-pushed the audit/ed25519-key-validation branch from 4164355 to dbbfb84 Compare August 13, 2026 09:48
@mellowcroc
mellowcroc force-pushed the harden/untrusted-decode-dos branch from 96daa63 to 56f410e Compare August 13, 2026 09:48
@mellowcroc
mellowcroc force-pushed the audit/ed25519-key-validation branch from dbbfb84 to 4932d2b Compare August 14, 2026 06:01
@mellowcroc
mellowcroc force-pushed the harden/untrusted-decode-dos branch from 56f410e to d29a4bc Compare August 14, 2026 06:01
@mellowcroc
mellowcroc force-pushed the harden/untrusted-decode-dos branch from d29a4bc to c92f9d2 Compare August 14, 2026 06:23
@mellowcroc
mellowcroc changed the base branch from audit/ed25519-key-validation to main August 14, 2026 06:23
KeySource.open decodes the G1 singletons (alpha, beta, delta) and the G2
singletons (beta, delta) with NoSubgroupChecks. Skipping the subgroup
check is a deliberate throughput trade on a proving key that callers are
expected to digest-authenticate first, and internal/msmengine makes the
same trade. The difference is that msmengine still runs IsOnCurve on
every decoded point, and streampk ran no validation at all.

That gap matters because OpenKeyURL reaches this code over HTTP range
requests, and the URL caller in cmd/wasm-prover does not digest the
proving key before opening it. A point that parses but is not on the
curve therefore entered a multi-scalar multiplication unchallenged.

IsOnCurve is cheap relative to the decode and is now applied to all five
singletons. This does not close the missing digest verification on the
URL path, which needs a separate change.
Four consumer paths decoded ceremony-derived artifacts before checking the
length/count fields that drive allocation, letting a hostile or corrupt input
exhaust memory or panic (unrecoverable throw on the native verifier, module
abort on wasm):

- prover.UnmarshalProof: preflight the BSB22 commitment-count prefix and the
  exact encoded length before gnark-crypto runs make([]G1Affine, count). Closes
  a remote unauthenticated OOM on the verifier HTTP API.
- wasm-prover fetchCCS: bound the decoded CCS to its signed size, cap the zstd
  decoder window, and recover() the decode so a hostile length prefix errors
  instead of aborting the module.
- proofassets.ValidatePKIndexAllocations: bound NbWires, NbInfinityA/B, and
  NbCommitmentKeys against the signed FileSize and section geometry. Applied on
  the full-index paths (ReadPKIndex, streampk.ValidateIndex); the manifest
  digest covers only geometry, so the counters were otherwise free.
- streampk domain decode: validate the FFT cardinality is canonical before
  precomputing twiddles, so a hostile 2^32 cardinality is rejected before the
  ~274 GB allocation.

Adds regression tests for the proof and PK-index paths.
@mellowcroc
mellowcroc force-pushed the harden/untrusted-decode-dos branch from c92f9d2 to e9743f5 Compare August 14, 2026 06:26
@mellowcroc mellowcroc changed the title Bound untrusted decode paths against allocation DoS Harden untrusted decode paths Aug 14, 2026
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