diff --git a/src/nova/mod.rs b/src/nova/mod.rs index 4d03d958e..1c70ec9c6 100644 --- a/src/nova/mod.rs +++ b/src/nova/mod.rs @@ -1176,6 +1176,66 @@ mod tests { test_ivc_trivial_with::(); } + /// A `RecursiveSNARK` whose witness vectors do not match the shape must be + /// rejected, not abort the process. + /// + /// `RecursiveSNARK` derives `Deserialize` with no validation of its internal + /// vector lengths, so a verifier that accepts one from an untrusted source — + /// which is what a succinct proof is for — can be handed any shape at all. + /// `verify`'s pre-checks cover `i`, `z0` and the `X` lengths, and the + /// output-hash check does not cover `W` or `E`, so a proof with one witness + /// element added or removed reaches the satisfiability check with a length + /// that used to be asserted rather than returned. + fn test_ivc_malformed_witness_is_rejected_with() + where + E1: Engine::Scalar>, + E2: Engine::Scalar>, + { + let test_circuit = TrivialCircuit::<::Scalar>::default(); + let pp = PublicParams::::Scalar>>::setup( + &test_circuit, + &*default_ck_hint(), + &*default_ck_hint(), + ) + .unwrap(); + let z0 = [::Scalar::ZERO]; + + let mut recursive_snark = RecursiveSNARK::new(&pp, &test_circuit, &z0).unwrap(); + recursive_snark.prove_step(&pp, &test_circuit).unwrap(); + assert!(recursive_snark.verify(&pp, 1, &z0).is_ok()); + + // One element too many in the primary running witness. + let mut grown = recursive_snark.clone(); + grown.r_W_primary.W.push(::Scalar::ZERO); + assert!(matches!( + grown.verify(&pp, 1, &z0), + Err(NovaError::InvalidWitnessLength) + )); + + // One element too few in the primary error vector. + let mut shrunk = recursive_snark.clone(); + shrunk.r_W_primary.E.pop(); + assert!(matches!( + shrunk.verify(&pp, 1, &z0), + Err(NovaError::InvalidWitnessLength) + )); + + // And on the secondary curve, whose incoming witness goes through + // `is_sat` rather than `is_sat_relaxed`. + let mut secondary = recursive_snark.clone(); + secondary.l_w_secondary.W.push(::Scalar::ZERO); + assert!(matches!( + secondary.verify(&pp, 1, &z0), + Err(NovaError::InvalidWitnessLength) + )); + } + + #[test] + fn test_ivc_malformed_witness_is_rejected() { + test_ivc_malformed_witness_is_rejected_with::(); + test_ivc_malformed_witness_is_rejected_with::(); + } + fn test_ivc_nontrivial_with() where E1: Engine::Scalar>, diff --git a/src/r1cs/mod.rs b/src/r1cs/mod.rs index 0e8a90f54..752246eda 100644 --- a/src/r1cs/mod.rs +++ b/src/r1cs/mod.rs @@ -450,9 +450,21 @@ impl R1CSShape { U: &RelaxedR1CSInstance, W: &RelaxedR1CSWitness, ) -> Result<(), NovaError> { - assert_eq!(W.W.len(), self.num_vars); - assert_eq!(W.E.len(), self.num_cons); - assert_eq!(U.X.len(), self.num_io); + // A `RelaxedR1CSWitness` can arrive from `Deserialize` with any lengths at + // all, so these are checks on untrusted input rather than internal + // invariants: assert here and a verifier that accepts a proof from a peer + // aborts instead of rejecting it. + // + // `W` and `X` are also covered one call deeper, since `multiply_vec` + // returns `InvalidWitnessLength` when `z` does not match the shape. + // `E` is not: it is not part of `z`, and a short one is an out-of-bounds + // index in the satisfiability loop below. + if W.W.len() != self.num_vars || W.E.len() != self.num_cons { + return Err(NovaError::InvalidWitnessLength); + } + if U.X.len() != self.num_io { + return Err(NovaError::InvalidInputLength); + } // verify if Az * Bz = u*Cz + E let res_eq = { @@ -496,8 +508,14 @@ impl R1CSShape { U: &R1CSInstance, W: &R1CSWitness, ) -> Result<(), NovaError> { - assert_eq!(W.W.len(), self.num_vars); - assert_eq!(U.X.len(), self.num_io); + // Same reasoning as `is_sat_relaxed`: attacker-supplied lengths, not + // internal invariants. + if W.W.len() != self.num_vars { + return Err(NovaError::InvalidWitnessLength); + } + if U.X.len() != self.num_io { + return Err(NovaError::InvalidInputLength); + } // verify if Az * Bz = u*Cz let res_eq = {