Skip to content

Commit

Permalink
fix: add field size check for public inputs (#12)
Browse files Browse the repository at this point in the history
Issue:
- there is no explicit check that public inputs are less than field size

Changes:
- add optional check to `prepare_inputs` which checks that inputs are less than field size
- add `verify_unchecked` to bypass the checks for performance
  • Loading branch information
ananas-block authored Sep 2, 2024
1 parent aaebdc7 commit 8892dae
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 13 deletions.
15 changes: 7 additions & 8 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ ark-serialize = "0.4.2"
ark-ec = "0.4.2"
ark-ff = "0.4.2"
ark-bn254 = "0.4.0"
num-bigint = "0.4.6"


[dev-dependencies]
Expand Down
2 changes: 2 additions & 0 deletions src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,6 @@ pub enum Groth16Error {
DecompressingG1Failed,
#[error("DecompressingG2Failed")]
DecompressingG2Failed,
#[error("PublicInputGreaterThenFieldSize")]
PublicInputGreaterThenFieldSize,
}
75 changes: 70 additions & 5 deletions src/groth16.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,13 @@
//!
//! See functional test for a running example how to use this library.
//!
use crate::errors::Groth16Error;
use ark_ff::PrimeField;
use num_bigint::BigUint;
use solana_program::alt_bn128::prelude::{
alt_bn128_addition, alt_bn128_multiplication, alt_bn128_pairing,
};

use crate::errors::Groth16Error;

#[derive(PartialEq, Eq, Debug)]
pub struct Groth16Verifyingkey<'a> {
pub nr_pubinputs: usize,
Expand Down Expand Up @@ -85,10 +86,13 @@ impl<const NR_INPUTS: usize> Groth16Verifier<'_, NR_INPUTS> {
})
}

pub fn prepare_inputs(&mut self) -> Result<(), Groth16Error> {
pub fn prepare_inputs<const CHECK: bool>(&mut self) -> Result<(), Groth16Error> {
let mut prepared_public_inputs = self.verifyingkey.vk_ic[0];

for (i, input) in self.public_inputs.iter().enumerate() {
if CHECK && !is_less_than_bn254_field_size_be(input) {
return Err(Groth16Error::PublicInputGreaterThenFieldSize);
}
let mul_res = alt_bn128_multiplication(
&[&self.verifyingkey.vk_ic[i + 1][..], &input[..]].concat(),
)
Expand All @@ -105,8 +109,20 @@ impl<const NR_INPUTS: usize> Groth16Verifier<'_, NR_INPUTS> {
Ok(())
}

/// Verifies the proof, and checks that public inputs are smaller than
/// field size.
pub fn verify(&mut self) -> Result<bool, Groth16Error> {
self.prepare_inputs()?;
self.verify_common::<true>()
}

/// Verifies the proof, and does not check that public inputs are smaller
/// than field size.
pub fn verify_unchecked(&mut self) -> Result<bool, Groth16Error> {
self.verify_common::<false>()
}

fn verify_common<const CHECK: bool>(&mut self) -> Result<bool, Groth16Error> {
self.prepare_inputs::<CHECK>()?;

let pairing_input = [
self.proof_a.as_slice(),
Expand All @@ -130,14 +146,19 @@ impl<const NR_INPUTS: usize> Groth16Verifier<'_, NR_INPUTS> {
}
}

pub fn is_less_than_bn254_field_size_be(bytes: &[u8; 32]) -> bool {
let bigint = BigUint::from_bytes_be(bytes);
bigint < ark_bn254::Fr::MODULUS.into()
}

#[cfg(test)]
mod tests {
use crate::decompression::{decompress_g1, decompress_g2};

use super::*;
use ark_bn254;
use ark_ff::BigInteger;
use ark_serialize::{CanonicalDeserialize, CanonicalSerialize, Compress, Validate};

use std::ops::Neg;
type G1 = ark_bn254::g1::G1Affine;
type G2 = ark_bn254::g2::G2Affine;
Expand Down Expand Up @@ -311,6 +332,19 @@ mod tests {
139, 253, 65, 152, 92, 209, 53, 37, 25, 83, 61, 252, 42, 181, 243, 16, 21, 2, 199, 123, 96,
218, 151, 253, 86, 69, 181, 202, 109, 64, 129, 124, 254, 192, 25, 177, 199, 26, 50,
];

#[test]
fn test_is_less_than_bn254_field_size_be() {
let bytes = [0u8; 32];
assert!(is_less_than_bn254_field_size_be(&bytes));

let bytes: [u8; 32] = BigUint::from(ark_bn254::Fr::MODULUS)
.to_bytes_be()
.try_into()
.unwrap();
assert!(!is_less_than_bn254_field_size_be(&bytes));
}

#[test]
fn proof_verification_should_succeed() {
let proof_a: G1 = G1::deserialize_with_mode(
Expand Down Expand Up @@ -339,6 +373,7 @@ mod tests {
Groth16Verifier::new(&proof_a, &proof_b, &proof_c, &PUBLIC_INPUTS, &VERIFYING_KEY)
.unwrap();
verifier.verify().unwrap();
verifier.verify_unchecked().unwrap();
}

fn compress_g1_be(g1: &[u8; 64]) -> [u8; 32] {
Expand Down Expand Up @@ -399,6 +434,7 @@ mod tests {
Groth16Verifier::new(&proof_a, &proof_b, &proof_c, &PUBLIC_INPUTS, &VERIFYING_KEY)
.unwrap();
verifier.verify().unwrap();
verifier.verify_unchecked().unwrap();
}

#[test]
Expand All @@ -418,5 +454,34 @@ mod tests {
verifier.verify(),
Err(Groth16Error::ProofVerificationFailed)
);
assert_eq!(
verifier.verify_unchecked(),
Err(Groth16Error::ProofVerificationFailed)
);
}

#[test]
fn public_input_greater_than_field_size_should_not_suceed() {
let proof_a = PROOF[0..64].try_into().unwrap();
let proof_b = PROOF[64..192].try_into().unwrap();
let proof_c = PROOF[192..256].try_into().unwrap();
let mut public_inputs = PUBLIC_INPUTS;
public_inputs[0] = ark_bn254::Fr::MODULUS.to_bytes_be().try_into().unwrap();
let mut verifier = Groth16Verifier::new(
&proof_a, // using non negated proof a as test for wrong proof
&proof_b,
&proof_c,
&public_inputs,
&VERIFYING_KEY,
)
.unwrap();
assert_eq!(
verifier.verify_unchecked(),
Err(Groth16Error::ProofVerificationFailed)
);
assert_eq!(
verifier.verify(),
Err(Groth16Error::PublicInputGreaterThenFieldSize)
);
}
}

0 comments on commit 8892dae

Please sign in to comment.