Audit of Zcash: Orchard

Description. hash_to_point accepts a Message with zero pieces. The Message constructor only bounds the number of words from above, so an empty message passes:

fn from(pieces: Vec<MessagePiece<F, K>>) -> Self {
    // A message cannot contain more than `MAX_WORDS` words.
    assert!(pieces.iter().map(|piece| piece.num_words()).sum::<usize>() < MAX_WORDS);
    Message(pieces)
}

When there are no pieces, the hashing loop in hash_all_pieces never runs, so hash_piece is never called and no q_sinsemilla1 row (nor the generator-table lookup or transition gate) is enabled. The returned y-coordinate is then assigned as plain advice into lambda_1, constrained only by the initialization gate:

// Hash each piece in the message.
for (idx, piece) in message.iter().enumerate() {
    let final_piece = idx == message.len() - 1;
    let (x, y, zs) = self.hash_piece(region, offset, piece, x_a, y_a, final_piece)?;
    offset += piece.num_words();
    x_a = x;
    y_a = y;
    zs_sum.push(zs);
}

// Assign the final y_a.
let y_a = {
    let y_a_cell =
        region.assign_advice(|| "y_a", config.double_and_add.lambda_1, offset, || y_a.0)?;
    // dummy lambda2 / x_p assigned as zero, since they are multiplied by zero
    y_a_cell
};

The initialization gate pins the output x-coordinate to Q.x and relates the y-coordinate only through a single equation:

init_y_q_check:  2·y_Q − Y_A = 0,   Y_A = (λ₁ + λ₂)·(x_a − x_R),   x_R = λ₁² − x_a − x_p

Here x_a is fixed to the constant x_Q and y_Q to a fixed column, but λ₁ (the returned y-coordinate), λ₂, and x_p are free advice.

A prover can pick any target y for the output and set the auxiliary cells x_p = y² − 2·x_Q + 1 and λ₂ = 2·y_Q − y, which give x_a − x_R = 1 and Y_A = 2·y_Q, satisfying the gate. We developed a proof-of-concept that drives a real SinsemillaChip on an empty message and returns (x_Q, −y_Q) = −Q, even though the specification defines SinsemillaHashToPoint(D, []) = Q.

Impact. This is an exploitable soundness gap for consumers of the Sinsemilla gadget that permit empty messages and use the returned point as a full curve point. It has no impact on Orchard, which does not hash empty Sinsemilla messages. Callers that only use the x-coordinate via extract are unaffected.

Recommendation. If empty messages are meant to be supported, constrain the empty case to return the domain point Q = (x_Q, y_Q). The initialization already pins the returned x-coordinate to the constant x_Q (via assign_advice_from_constant); the empty branch should bind the returned y-coordinate to the constant y_Q in the same way, instead of assigning it as free advice. This fix lives entirely in the empty-message code path: it reuses the existing fixed_y_q and permutation machinery and adds no gate or column at configure time, so the constraint system that Orchard synthesizes is unchanged.

Alternatively, if empty messages need not be supported, reject them in Message::from / Message::from_pieces / hash_all_pieces.

Description. Full-width fixed-base scalar multiplication decomposes a ScalarFixed into windows and computes [windows]·B, but it never constrains those windows to equal the witnessed scalar value. A ScalarFixed is created through the public constructor:

pub fn new(
    chip: EccChip,
    mut layouter: impl Layouter<pallas::Base>,
    value: Value<pallas::Scalar>,
) -> Result<Self, Error> {
    // ...
}

The value here is a host-side Value, used only during witness generation to derive the window assignments. Under the hood new calls witness_scalar_fixed, which ignores its layouter and assigns nothing, returning EccScalarFixed { value, windows: None }; the only cells are the window digits created later inside .mul, and they are range- and coordinate-checked only, never tied back to value. No equality constraint links the two. The relation the gadget actually enforces is therefore existential: result = [windows]·B for prover-chosen windows, i.e. “the output is B multiplied by some scalar the prover knows.” It does not enforce that two multiplications sharing a single ScalarFixed object use the same scalar.

A caller who passes one ScalarFixed to two multiplications intending them to share an exponent gets no such guarantee. We developed a proof-of-concept that reuses a single ScalarFixed across P = [α]·G and Q = [α]·H, a discrete-log-equality (DLEQ) relationship. Because neither multiplication binds its windows to the scalar value, a prover can desynchronize them: the circuit accepts P = [5]·G and Q = [11]·H — two points that do not share a discrete log — even though a single ScalarFixed was passed to both.

This is an instance of a “value, not cell” gap: the API takes a Value and uses it to compute witnesses, while a caller might assume that passing the same value binds the two results to the same scalar. An honest prover always makes the windows agree with the value, so happy-path tests and witness-time self-checks pass; only a misuse combined with a malicious prover diverges.

Impact. The gadget is used correctly in the audited code. Orchard’s spend-authority block reads as if it binds the scalar to a cell, but the same-scalar guarantee it visually suggests does not exist:

let alpha = ScalarFixed::new(ecc_chip, layouter.namespace(|| "alpha"), self.alpha)?;
let (alpha_commitment, _) = spend_auth_g.mul(layouter, alpha)?;   // [alpha]·SpendAuthG

Here alpha is a private prover hint, so the existential contract (“the output is the base multiplied by some scalar the prover knows”) is exactly what Orchard needs, and no cross-multiplication scalar equality is relied upon. The issue is a latent footgun for other callers — such as a DLEQ construction — that would need two fixed-base multiplications to be pinned to the same scalar.

Recommendation. Document that full-width fixed-base multiplication enforces only an existential relation on the scalar and does not bind a reused ScalarFixed to a single value. Callers requiring a shared scalar across multiplications must bind it themselves — e.g. by copying the scalar cell into the running-sum decomposition (copy_decompose) rather than witnessing the windows from a Value.

Description. Output::verify_note_commitment reconstructs a note from the PCZT metadata (recipient, value, rho, rseed) and checks that its commitment equals the output’s cmx:

let note = Note::from_parts(
    self.recipient.ok_or(VerifyError::MissingRecipient)?,
    self.value.ok_or(VerifyError::MissingValue)?,
    Rho::from_nf_old(spend.nullifier),
    self.rseed.ok_or(VerifyError::MissingRandomSeed)?,
)
.into_option()
.ok_or(VerifyError::InvalidOutputNote)?;

if ExtractedNoteCommitment::from(note.commitment()) == self.cmx {
    Ok(())
} else {
    Err(VerifyError::InvalidExtractedNoteCommitment)
}

This authenticates the note fields against the commitment, but it never inspects self.encrypted_note — the enc_ciphertext and out_ciphertext. Those ciphertext bytes are supplied by the PCZT constructor, and tx_extractor.rs copies them verbatim into the final action. Nothing in the verification helpers proves that the ciphertext actually encrypts the note whose fields the signer approves.

The Output documentation explicitly delegates this check to the signer (“The Signer can use recipient and rseed (if present) to verify that enc_ciphertext is correctly encrypted”), but the crate does not provide a helper that performs it. A signer relying on the available PCZT verification helpers therefore has no tooling to catch a malformed or mismatched ciphertext.

Impact. This is not a consensus issue — Orchard consensus does not require output ciphertexts to be decryptable. The impact is on signer/wallet flows: a signer can approve a semantically correct note (correct recipient, value, and memo per the metadata) while signing a transaction whose output ciphertext is undecryptable or encrypts different data. The recipient would be unable to detect or spend the note, effectively burning the funds, with no indication at approval time.

Recommendation. Provide a verification helper that checks enc_ciphertext (and out_ciphertext, when ock is available) against the approved note fields, and wire it into the documented signer checklist so that semantic approval covers the ciphertext the signer is committing to.

Description. Proof::verify and Proof::add_to_batch feed the proof bytes into the Halo2 transcript without checking that the transcript consumes the entire slice:

let mut transcript = Blake2bRead::init(&self.0[..]);
plonk::verify_proof(&vk.params, &vk.vk, strategy, &instances, &mut transcript)

Halo2 reads exactly the expected proof elements and stops, so a proof carrying arbitrary trailing bytes is still accepted by these low-level APIs. Canonical size is only enforced elsewhere: Bundle::try_from_parts calls validate_proof_size when passed ProofSizeEnforcement::Strict, and PCZT extraction validates size before producing an authorized bundle. Consumers that call Proof::verify / add_to_batch directly, or construct bundles with ProofSizeEnforcement::Unenforced, get no such check.

Recommendation. Reject non-canonical proof length in the low-level verifier, or document verify / add_to_batch as transcript-only APIs that require a separate Proof::expected_proof_size check by the caller.