Midnight Circuits and Proof System Follow-up Audit

Description. The as_public_input() helper in the ECC chip keeps only the lowest limb of the $y$-coordinate to resolve the ambiguity between the two curve points $(x,y)$ and $(x,-y)$.

// From y we only keep one limb, since it is enough to resolve the +- ambiguity.
let mut pis = [
    self.base_field_chip.as_public_input(layouter, &p.x)?.as_slice(),
    &self.base_field_chip.as_public_input(layouter, &p.y)?[..1],
]
.concat();

However, the chip accepts non-canonical base field encodings. The limbs may represent $y+m$ where $m$ is the foreign field modulus. base_field_chip.as_public_input() calls normalize() but that only affects field elements where the individual limbs are not within range.

If $y=k·\text{BASE}/2$, the lowest limb of $m+y$ equals that of $m-y$, since

The lowest limb therefore cannot distinguish between $y$ and $-y$.

Impact. The public input does not disambiguate the curve point: the prover can use $-P$ for witnesses while the output appears as $P$. This is a soundness issue and could have severe consequences for applications.

Recommendation. There are two possible routes to resolve this:

• Emit the full y coordinate, or at least enough public information to uniquely identify the point. For example, as mentioned by Midnight developers, coordinate limbs might be packed into fewer native field elements.
• Alternatively, constrain the $y$ coordinate to be canonical. When that is done, even the lowest bit is enough to distinguish between $y$ and $m-y$, since adding the odd number $m$ flips the parity.

Client Response. The issue was fixed in commit 1e0e061 by returning the full $y$ coordinate as public input.

Description. Line 920 in circuits/src/field/foreign/field_chip.rs contains a typo that causes the inverse of zero to return zero instead of failing as expected. The line incorrectly assigns the fixed value zero when it should assign one:

// Line 920 (typo)
let one: AssignedField<F, K, P> = self.assign_fixed(layouter, K::ZERO)?;

When it should use K::ONE to match the correct pattern established in line 928:

// Line 928 (correct)
let one = self.assign_fixed(layouter, K::ONE)?;

The consequence is that line 922’s check if x == &one compares against zero instead of one, causing inv(0) to return zero rather than attempting the inversion and failing at line 929.

Impact. This typo causes inv(0) = 0 behavior, which violates the expected trait contract for field inversion operations. According to standard field arithmetic and the native version implementation, attempting to invert zero should fail rather than return zero. This could lead to unexpected behavior in circuits that rely on proper field inversion semantics, potentially masking bugs where zero values are incorrectly passed to inversion operations.

Recommendation. Change line 920 from K::ZERO to K::ONE to fix the typo and ensure proper field inversion behavior that matches the trait contract and native implementation.

Client Response. Client has acknowledged this issue and fixed it in https://github.com/midnightntwrk/midnight-zk/pull/125.

Description. The core is_equal() and is_equal_to_fixed() gadgets in the native chip use add_and_double_mul() as if it enforced its result to be zero:

// (ii) enforced as res * x - res * y = 0.
self.add_and_double_mul(
    layouter,
    (F::ZERO, &res),
    (F::ZERO, x),
    (F::ZERO, y),
    F::ZERO,
    (F::ONE, -F::ONE),
)?;

However, add_and_double_mul() only returns of the arithmetic combination, and never constrains it to be zero. Therefore, the intended constraint that enforces the is_equal result to be zero when $x\ne y$ is ineffective. A malicious prover can output true or even non-Boolean outputs even though the inputs are unequal.

Impact. Equality checks throughout the code base can be forged, rendering many typical circuits severely underconstrained.

Recommendation. Actually constrain (x - y) * res to be zero.

Client Response. The issue was fixed in PR #117 by passing the output of add_and_double_mul() to assert_zero() to enforce the zero constraint.

Description. The poseidon_varlen gadget returns incorrect hash outputs when called with input slices of length 1 or 2.

For background, Poseidon is instantiated with a rate of 2 and therefore processes inputs in chunks of size 2.

Variable length arrays are represented by an array padded with unconstrained full chunks at the beginning. Therefore, when computing the hash, we have to keep track whether the current chunk already belongs to the actual array content. The Poseidon code tracks this with a boolean variable updating:

// Flag that will signal when the hash input starts and chunks need to be
// effectively processed and update the state.
let mut updating: AssignedBit<F> = self.native_gadget.assign_fixed(layouter, false)?;

// [ZKSECURITY] ...

// Conditional update loop. Stops 1 chunk before the end.
for i in 0..(MAX_LEN / RATE) - 1 {
    // Determines when we have arrived at the first chunk of input.
    let b = ng.is_equal_to_fixed(
        layouter,
        &rounded_len,
        F::from((MAX_LEN - (i * RATE)) as u64),
    )?;

    updating = ng.xor(layouter, &[b, updating])?;
    register = self.cond_update(layouter, &register, chunk, &updating)?;

    chunk = chunk_iter.next().expect("One more chunk.");
}

// Modify last chunk with the appropriate padding if necessary.
let chunk = self.pad_last_chunk(layouter, chunk, len, &offset)?;
// [ZKSECURITY] `updating` is not recomputed for the final chunk!
// [ZKSECURITY] => if rounded_len = 2, we are incorrectly missing this update
register = self.cond_update(layouter, &register, &chunk, &updating)?;

The problem is that for the last chunk, we are not correctly recomputing updating. If it was false throughout the loop, the final conditional update is not performed, even if rounded_len is 2 and the final chunk has actual content. Instead of being hashed, the single message chunk is simply ignored.

Existing tests for variable-length Poseidon were not exercising the case of input lengths 1 and 2.

Impact. Any consumer that hashes a single- or double-element input through poseidon_varlen will derive an incorrect digest. Even worse, the digest does not depend on the hashed content, which breaks all properties expected of a hash function and makes essentially every use of this gadget which does not control the input length insecure.

Recommendation. The following patch, proposed by Midnight developers, is enough to fix the issue:

let chunk = self.pad_last_chunk(layouter, chunk, len, &offset)?;
+ let updating = {
+   let b = ng.is_equal_to_fixed(layouter, &rounded_len, F::from(RATE as u64))?;
+   ng.xor(layouter, &[b, updating])?
+ };
register = self.cond_update(layouter, &register, &chunk, &updating)?;

Client Response. The issue was fixed in commit bb8f118 with the recommended patch.

Description. The blinding_factors() function in line 2355 of proofs/src/plonk/circuit.rs performs addition that can overflow when later cast to i32:

let factors = factors + self.trashcans.len();

This value is subsequently used in multiple locations where it gets cast to i32, such as:

• proofs/src/plonk/permutation/verifier.rs:221
• proofs/src/plonk/evaluation.rs:358
• proofs/src/plonk/prover.rs:228,248

The cast occurs in expressions like:

let x_last = vk.domain.rotate_omega(x, Rotation(-((blinding_factors + 1) as i32)));

When blinding_factors exceeds i32::MAX ($2^{31}-1$), the cast wraps around, turning a large positive number into a negative value. This negative value is then multiplied by -1, producing an incorrect positive rotation amount and causing polynomial evaluation at wrong points in the verifier.

Consider this scenario:

let i32_max = i32::MAX as usize;
let base_factors = 10;
let massive_trash_count = i32_max + 100; // Force overflow
let total_blinding_factors = base_factors + massive_trash_count + 1;

// This wraps: 2147483757 as i32 = -2147483539
let rotated = -((total_blinding_factors) as i32);
println!("Expected negative, got positive: {}", rotated); // Shows: 2147483539

Impact. This could potentially break soundness by allowing malicious provers to exploit the incorrect rotation calculations. While this was theoretically possible in previously audited code via num_advice_queries, it was impractical due to $O(n^2)$ complexity. The introduction of trash arguments makes this more feasible with $O(n)$ complexity, as an attacker now needs $2^{31}$ trash arguments to trigger the overflow.

Although this would require several GB of RAM and produce multi-GB proofs that might exhaust the verifier before soundness exploitation, it may succeed on some systems. The verification logic does not prevent large proofs or excessive trash arguments.

Recommendation. Validate that factors <= i32::MAX in the blinding_factors() function and fail gracefully before returning to avoid the casting issue:

fn blinding_factors(&self) -> Result<usize, Error> {
    let factors = factors + self.trashcans.len();
    if factors > i32::MAX as usize {
        return Err(Error::BlindingFactorsOverflow);
    }
    Ok(factors)
}

Client Response. Client has acknowledged this issue and fixed it in https://github.com/midnightntwrk/midnight-zk/pull/125.

Description. The prover and verifier use different logic to determine which PCS queries belong to committed instance columns. The prover uses positional slicing (.take(nb_committed_instances) in prover.rs:774) to select the first N queries regardless of their column association:

pk.vk.cs.instance_queries.iter().take(nb_committed_instances).map(...)

While the verifier uses column-based filtering (column.index() < nb_committed_instances in verifier.rs:251) to expect all queries from the first $N$ instance columns.

The committed instances feature operates under the architectural constraint that the first $N$ instance columns by index are always the committed ones. However, when a committed instance column is queried multiple times (e.g., at different rotations like Rotation::cur() and Rotation::next()), or when query ordering doesn’t align with column indices due to interleaved advice/fixed queries, the prover omits some committed instance openings from the multi-opening batch while the verifier still expects them.

Impact. This causes a completeness issue where verification fails with an “Opening” error, making the committed instances feature fail for circuits that query committed columns at multiple rotations or mix advice, fixed, and instance queries in gates. While this doesn’t happen in the aggregator, it could affect other circuits with more complex query patterns.

Recommendation. Align the prover logic with the verifier’s column-based approach. Instead of using positional slicing, filter queries by column index:

pk.vk.cs.instance_queries.iter()
    .filter(|&(column, _)| column.index() < nb_committed_instances)
    .map(...)

This ensures that all queries belonging to committed instance columns are properly included in the multi-opening batch, regardless of their position in the query list or rotation patterns.

Client Response. The issue was fixed in commit e97f1e1.

Description. The trusted setup ceremony lacks validation of the SRS file size when reading, verifying structure, or updating the ceremony. This allows an attacker to truncate $G_1$ points from the SRS and still pass all validations. The truncated SRS will then be propagated to subsequent participants, who will update the compromised SRS and continue passing validations.

A proof-of-concept demonstrates the vulnerability:

1. Downloaded the latest SRS file (3,221,225,856 bytes)
2. Truncated the file by removing the last 1000 $G_1$ points (3,221,129,856 bytes)
3. File size reduction was only 96KB, making the attack difficult to detect
4. Both verify-structure and verify-chain operations passed on the truncated file

# Original SRS
SHA256 (powers_of_tau.1) = a8a9c33e7a974399b38770f4ee61a9403ce341005f6190b6a25af0bfa3b55d01

# Truncated SRS (1000 G_1 points removed)
SHA256 (powers_truncated) = c9035797846168011fc0cfcce1fd9047149664b5dbd753a3f59ef463daef8f01

# Verification still passes
./target/release/srs_utils powers_truncated verify-chain
Verifying the chain of update proofs...
The chain of update proofs is correct!

The size difference is minimal:

• Original: 3,221,225,856 bytes
• Truncated: 3,221,129,856 bytes (96KB smaller)

Impact. This vulnerability enables stealthy attacks where small truncations are unlikely to be noticed due to minimal file size changes. Once a truncated SRS enters the ceremony chain, it permanently corrupts the entire process as all subsequent participants will unknowingly work with the compromised SRS. The truncation will only be discovered when circuits requiring the missing high-order powers are used, potentially long after the ceremony concludes, requiring a complete restart of the trusted setup ceremony.

Recommendation. Implement comprehensive SRS size validation in the ceremony tools by verifying that the SRS file has the expected exact size before processing and checking that the SRS contains the expected number of $G_1$ and $G_2$ points. Add size checks in critical operations including before reading SRS files, during structure verification, before ceremony updates, and in chain validation processes. Additionally, implement file server-side validation if the SRS is distributed through a centralized mechanism to prevent corrupted files from entering the ceremony chain.

Client Response. The client has acknowledged the issue and has fixed it in https://github.com/midnightntwrk/midnight-trusted-setup/pull/69/.

Description. The PLONK verifier assumes that the committed_instances vector is non-empty, causing a panic when called with empty instance vectors. In lines 40 and 199 of proofs/src/plonk/verifier.rs, the code performs:

let nb_committed_instances = committed_instances[0].len();

This directly accesses the first element of committed_instances without checking if the vector is empty, leading to an index out of bounds panic.

The following test demonstrates the vulnerability:

let empty_instances: &[&[&[Fr]]] = &[];

let _result = prepare_plonk_proof::<Fr, Scheme, _>(
    &vk,
    #[cfg(feature = "committed-instances")]
    &[], // empty committed instances
    empty_instances,
    &mut transcript,
);

This results in:

thread 'test_empty_instances_vulnerability' panicked at midnight-zk/proofs/src/plonk/verifier.rs:40:34:
index out of bounds: the len is 0 but the index is 0

Impact. This creates a potential denial-of-service (DoS) vulnerability where malicious actors can crash the verifier by providing empty committed instances. This could disrupt proof verification processes and affect system availability.

Recommendation. Check if committed_instances is empty before accessing its first element. The verifier should fail gracefully if empty instances are not expected, or default to 0 if that behavior is appropriate for the protocol. Consider adding bounds checking and graceful error handling for this scenario.

Client Response. Client has acknowledged this issue and fixed it in https://github.com/midnightntwrk/midnight-zk/pull/125.

Description. The proving system does not enforce which instance columns should be committed versus public at the protocol level. The prover specifies the split between committed and “plain” instance columns at proof generation time, and the core verification API accepts whatever split the prover supplies. This allows the prover to arbitrarily “promote” instance columns (marking them as committed and supplying only their commitment) or “demote” them (streaming them in the clear).

Both proof types verify successfully at the protocol level because the commitment openings still match and the algebraic checks remain intact. A proof-of-concept demonstrates:

• Promotion: Taking a normally public instance column, committing to it, and providing only the commitment to the verifier
• Demotion: Taking a normally committed instance column and providing its values directly as public instances

The proving system API cannot reject malformed splits and any validation of the expected committed/public boundary must be implemented in external verifier logic outside the core proving system.

Impact. While this doesn’t create soundness or completeness failures at the cryptographic level, it places the burden of enforcing the correct instance split on application-level verifier logic. If the external verifier logic doesn’t validate the expected split, provers can manipulate which values are committed versus public. This could lead to unexpected behavior in applications that assume certain values will be available for off-circuit validation or that rely on specific privacy guarantees.

Recommendation. Consider whether the current design is intentional. If applications require enforcement of specific committed/public splits, this must be implemented in external verifier logic rather than relying on the proving system. Alternatively, consider encoding the expected number of committed instances in the verification key or circuit definition to provide protocol-level enforcement of the commitment scheme interface.