Conversation
There was a problem hiding this comment.
Pull request overview
Adds new Rust staticlib “glue” layers for SP1 and Ziren proving/verification, wires them into the Zig state-proving manager and build system, and implements OpenVM receipt verification (previously a stub).
Changes:
- Add
sp1-glueandziren-gluecrates exposing*_prove/*_verifyC-ABI entrypoints and include them in the Rust workspace. - Implement
openvm_verifyby deserializing a proof package, checking an ELF hash, and verifying the proof with OpenVM SDK. - Extend Zig build + manager plumbing to select/link SP1/Ziren libraries and route prove/verify calls.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| rust/ziren-glue/src/lib.rs | New Ziren prover/verifier FFI surface and proof packaging. |
| rust/ziren-glue/Cargo.toml | New crate definition + Ziren SDK dependency. |
| rust/sp1-glue/src/lib.rs | New SP1 prover/verifier FFI surface and proof packaging. |
| rust/sp1-glue/Cargo.toml | New crate definition + SP1 SDK dependency. |
| rust/openvm-glue/src/lib.rs | Replace OpenVM verify stub with real verification flow. |
| rust/openvm-glue/Cargo.toml | Add openvm-circuit dependency to deserialize proof type. |
| rust/Cargo.toml | Add new workspace members + add SP1/Ziren build profiles. |
| pkgs/state-proving-manager/src/manager.zig | Add SP1/Ziren options and route calls via conditional externs. |
| build.zig | Add SP1/Ziren prover choices, link the right Rust archives, and build with the right Cargo profiles. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Deserialize the ZKM proof | ||
| let proof: ZKMProofWithPublicValues = match bincode::deserialize(&proof_package.proof_bytes) { | ||
| Ok(p) => p, | ||
| Err(e) => { | ||
| eprintln!("ziren_verify: failed to deserialize proof: {}", e); | ||
| return false; | ||
| } | ||
| }; | ||
|
|
||
| // Deserialize the verifying key | ||
| let vk: ZKMVerifyingKey = match bincode::deserialize(&proof_package.vk_bytes) { | ||
| Ok(k) => k, | ||
| Err(e) => { | ||
| eprintln!("ziren_verify: failed to deserialize verifying key: {}", e); | ||
| return false; | ||
| } | ||
| }; | ||
|
|
||
| // Verify the proof using the ZKM SDK | ||
| let client = ProverClient::new(); | ||
| match client.verify(&proof, &vk) { |
There was a problem hiding this comment.
ziren_verify trusts a verifying key provided inside the untrusted receipt (vk_bytes). An attacker can set elf_hash to match the local ELF but supply a (proof, vk) pair for a different program/circuit, and verification will still succeed because nothing ties vk to binary_path. Derive the verifying key from the ELF on the verifier side (e.g., run client.setup(&elf_bytes) and use/compare the resulting vk), or otherwise authenticate the verifying key out-of-band instead of accepting it from the receipt.
| // Deserialize the ZKM proof | |
| let proof: ZKMProofWithPublicValues = match bincode::deserialize(&proof_package.proof_bytes) { | |
| Ok(p) => p, | |
| Err(e) => { | |
| eprintln!("ziren_verify: failed to deserialize proof: {}", e); | |
| return false; | |
| } | |
| }; | |
| // Deserialize the verifying key | |
| let vk: ZKMVerifyingKey = match bincode::deserialize(&proof_package.vk_bytes) { | |
| Ok(k) => k, | |
| Err(e) => { | |
| eprintln!("ziren_verify: failed to deserialize verifying key: {}", e); | |
| return false; | |
| } | |
| }; | |
| // Verify the proof using the ZKM SDK | |
| let client = ProverClient::new(); | |
| match client.verify(&proof, &vk) { | |
| // Derive the verifying key from the local ELF to avoid trusting vk_bytes from the receipt | |
| let client = ProverClient::new(); | |
| let (_proving_key, vk_from_elf) = match client.setup(&elf_bytes) { | |
| Ok(result) => result, | |
| Err(e) => { | |
| eprintln!("ziren_verify: failed to derive verifying key from ELF: {}", e); | |
| return false; | |
| } | |
| }; | |
| // Deserialize the ZKM proof | |
| let proof: ZKMProofWithPublicValues = match bincode::deserialize(&proof_package.proof_bytes) { | |
| Ok(p) => p, | |
| Err(e) => { | |
| eprintln!("ziren_verify: failed to deserialize proof: {}", e); | |
| return false; | |
| } | |
| }; | |
| // Verify the proof using the ZKM SDK and the verifying key derived from the ELF | |
| match client.verify(&proof, &vk_from_elf) { |
| // Deserialize the proof package from receipt bytes | ||
| let proof_package: ZirenProofPackage = match bincode::deserialize(receipt_slice) { | ||
| Ok(p) => p, | ||
| Err(e) => { | ||
| eprintln!("ziren_verify: failed to deserialize proof package: {}", e); | ||
| return false; | ||
| } | ||
| }; |
There was a problem hiding this comment.
bincode::deserialize(receipt_slice) is performed on untrusted input without any size limit. Because ZirenProofPackage contains Vec<u8> fields, a crafted receipt can request huge allocations and cause OOM/DoS during verification. Consider using bincode::DefaultOptions::new().with_limit(...) (or an equivalent bounded deserializer) and reject receipts exceeding a maximum expected size.
rust/openvm-glue/src/lib.rs
Outdated
| // Deserialize the proof package from receipt bytes | ||
| let proof_package: OpenVMProofPackage = match bincode::deserialize(receipt_slice) { | ||
| Ok(p) => p, | ||
| Err(e) => { | ||
| eprintln!("openvm_verify: failed to deserialize proof package: {}", e); | ||
| return false; | ||
| } | ||
| }; | ||
|
|
||
| // Verify ELF hash: read the binary ELF and check it matches the hash in the proof | ||
| let elf_bytes = match fs::read(binary_path) { | ||
| Ok(b) => b, | ||
| Err(e) => { | ||
| eprintln!("openvm_verify: failed to read ELF at {}: {}", binary_path, e); | ||
| return false; | ||
| } | ||
| }; | ||
|
|
||
| let mut hasher = Sha256::new(); | ||
| hasher.update(&elf_bytes); | ||
| let computed_hash = hasher.finalize().to_vec(); | ||
|
|
||
| if computed_hash != proof_package.elf_hash { | ||
| eprintln!("openvm_verify: ELF hash mismatch — proof was generated for a different binary"); | ||
| return false; | ||
| } | ||
|
|
||
| // Deserialize the continuation VM proof | ||
| let proof: ContinuationVmProof<SC> = match bincode::deserialize(&proof_package.proof_bytes) { | ||
| Ok(p) => p, | ||
| Err(e) => { | ||
| eprintln!("openvm_verify: failed to deserialize proof: {}", e); | ||
| return false; | ||
| } | ||
| }; | ||
|
|
||
| // Deserialize the app verifying key | ||
| let app_vk: AppVerifyingKey = match bincode::deserialize(&proof_package.vk_bytes) { | ||
| Ok(k) => k, | ||
| Err(e) => { | ||
| eprintln!("openvm_verify: failed to deserialize verifying key: {}", e); | ||
| return false; | ||
| } | ||
| }; | ||
|
|
||
| // Verify the proof using the SDK | ||
| let sdk = Sdk::new(); | ||
| match sdk.verify_app_proof(&app_vk, &proof) { | ||
| Ok(_) => true, | ||
| Err(e) => { | ||
| eprintln!("openvm_verify: proof verification failed: {}", e); | ||
| false | ||
| } | ||
| } |
There was a problem hiding this comment.
openvm_verify accepts vk_bytes from the untrusted receipt and verifies the proof against that key. Because the ELF hash check is independent of the verifying key, a malicious receipt can set elf_hash to match the local ELF while supplying a (proof, vk) pair for a different circuit, and verification can still succeed. The verifier should derive the expected AppVerifyingKey from the ELF/config on its side (or compare against a trusted/precomputed key) rather than trusting vk_bytes from the receipt.
rust/openvm-glue/src/lib.rs
Outdated
| // Deserialize the proof package from receipt bytes | ||
| let proof_package: OpenVMProofPackage = match bincode::deserialize(receipt_slice) { | ||
| Ok(p) => p, | ||
| Err(e) => { | ||
| eprintln!("openvm_verify: failed to deserialize proof package: {}", e); | ||
| return false; | ||
| } | ||
| }; |
There was a problem hiding this comment.
bincode::deserialize(receipt_slice) is called on untrusted receipt bytes without a size limit. Since OpenVMProofPackage contains Vec<u8> fields, a crafted receipt can trigger large allocations and OOM/DoS during verification. Consider switching to bounded deserialization (e.g., bincode::DefaultOptions::new().with_limit(...)) and rejecting oversized receipts early.
| // Deserialize the verifying key | ||
| let vk: SP1VerifyingKey = match bincode::deserialize(&proof_package.vk_bytes) { | ||
| Ok(k) => k, | ||
| Err(e) => { | ||
| eprintln!("sp1_verify: failed to deserialize verifying key: {}", e); | ||
| return false; | ||
| } | ||
| }; | ||
|
|
||
| // Verify the proof using the SP1 SDK | ||
| let client = ProverClient::new(); | ||
| match client.verify(&proof, &vk) { | ||
| Ok(()) => true, |
There was a problem hiding this comment.
sp1_verify trusts a verifying key provided inside the untrusted receipt (vk_bytes). An attacker can set elf_hash to match the local ELF but supply a (proof, vk) pair for a different program/circuit, and verification will still succeed because nothing ties vk to binary_path. Derive the verifying key from the ELF on the verifier side (e.g., run client.setup(&elf_bytes) and use/compare the resulting vk), or otherwise authenticate the verifying key out-of-band instead of accepting it from the receipt.
| // Deserialize the proof package from receipt bytes | ||
| let proof_package: SP1ProofPackage = match bincode::deserialize(receipt_slice) { | ||
| Ok(p) => p, | ||
| Err(e) => { | ||
| eprintln!("sp1_verify: failed to deserialize proof package: {}", e); | ||
| return false; | ||
| } | ||
| }; |
There was a problem hiding this comment.
bincode::deserialize(receipt_slice) is performed on untrusted input without any size limit. Because SP1ProofPackage contains Vec<u8> fields, a crafted receipt can request huge allocations and cause OOM/DoS during verification. Consider using bincode::DefaultOptions::new().with_limit(...) (or an equivalent bounded deserializer) and reject receipts exceeding a maximum expected size.
gballet
force-pushed
the
verify-and-prove-ziren
branch
from
e88ce57 to
e9640be
Compare
gballet
force-pushed
the
verify-and-prove-ziren
branch
from
e9640be to
e17fe7b
Compare
gballet
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Deserialize the verifying key | ||
| let vk: SP1VerifyingKey = match bincode::deserialize(&proof_package.vk_bytes) { | ||
| Ok(k) => k, | ||
| Err(e) => { | ||
| eprintln!("sp1_verify: failed to deserialize verifying key: {}", e); | ||
| return false; | ||
| } | ||
| }; | ||
|
|
||
| // Verify the proof using the SP1 SDK | ||
| let client = ProverClient::new(); | ||
| match client.verify(&proof, &vk) { | ||
| Ok(()) => true, | ||
| Err(e) => { | ||
| eprintln!("sp1_verify: proof verification failed: {}", e); | ||
| false |
There was a problem hiding this comment.
The verifier currently trusts a verifying key (vk_bytes) supplied by the prover. Together with the fact that elf_hash is not cryptographically tied to the proof, a malicious prover can provide an arbitrary (proof, vk, elf_hash) triple and pass verification for the wrong program. Prefer deriving the verifying key from the verifier’s local ELF (e.g., call client.setup(&elf_bytes) and use the derived VK) and avoid accepting VK material from the receipt (or at least compare against the derived VK and verify with the derived one).
| #[no_mangle] | ||
| extern "C" fn ziren_prove( | ||
| serialized: *const u8, | ||
| len: usize, | ||
| binary_path: *const u8, | ||
| binary_path_len: usize, | ||
| output: *mut u8, | ||
| output_len: usize, | ||
| ) -> u32 { | ||
| println!( | ||
| "Running the Ziren transition prover, current dir={}", | ||
| std::env::current_dir().unwrap().display() | ||
| ); | ||
|
|
||
| let serialized_block = unsafe { | ||
| if !serialized.is_null() { | ||
| std::slice::from_raw_parts(serialized, len) | ||
| } else { | ||
| &[] | ||
| } | ||
| }; | ||
|
|
||
| let output_slice = unsafe { | ||
| if !output.is_null() { | ||
| std::slice::from_raw_parts_mut(output, output_len) | ||
| } else { | ||
| panic!("Output buffer is null") | ||
| } | ||
| }; | ||
|
|
||
| let binary_path_slice = unsafe { | ||
| if !binary_path.is_null() { | ||
| std::slice::from_raw_parts(binary_path, binary_path_len) | ||
| } else { | ||
| &[] | ||
| } | ||
| }; | ||
|
|
||
| let binary_path = std::str::from_utf8(binary_path_slice).unwrap(); | ||
| let elf_bytes = fs::read(binary_path).unwrap(); | ||
|
|
||
| let client = ProverClient::new(); | ||
| let (pk, vk) = client.setup(&elf_bytes); |
There was a problem hiding this comment.
ziren_prove accepts attacker-controlled len, output_len, and binary_path_len from the FFI boundary but does not enforce any maximums before creating slices, reading the ELF, or serializing the proof. This makes it easy to trigger excessive allocations / OOM or process aborts. Consider adding explicit MAX_* bounds (as done in openvm-glue) for input length, output buffer length, path length, and ELF size before proceeding.
| // Deserialize the verifying key | ||
| let vk: ZKMVerifyingKey = match bincode::deserialize(&proof_package.vk_bytes) { | ||
| Ok(k) => k, | ||
| Err(e) => { | ||
| eprintln!("ziren_verify: failed to deserialize verifying key: {}", e); | ||
| return false; | ||
| } | ||
| }; | ||
|
|
||
| // Verify the proof using the ZKM SDK | ||
| let client = ProverClient::new(); | ||
| match client.verify(&proof, &vk) { | ||
| Ok(()) => true, | ||
| Err(e) => { | ||
| eprintln!("ziren_verify: proof verification failed: {}", e); | ||
| false |
There was a problem hiding this comment.
The verifier currently trusts a verifying key (vk_bytes) supplied by the prover. Together with the fact that elf_hash is not cryptographically tied to the proof, a malicious prover can provide an arbitrary (proof, vk, elf_hash) triple and pass verification for the wrong program. Prefer deriving the verifying key from the verifier’s local ELF (e.g., call client.setup(&elf_bytes) and use the derived VK) and avoid accepting VK material from the receipt (or at least compare against the derived VK and verify with the derived one).
| #[no_mangle] | ||
| extern "C" fn sp1_prove( | ||
| serialized: *const u8, | ||
| len: usize, | ||
| binary_path: *const u8, | ||
| binary_path_len: usize, | ||
| output: *mut u8, | ||
| output_len: usize, | ||
| ) -> u32 { | ||
| println!( | ||
| "Running the SP1 transition prover, current dir={}", | ||
| std::env::current_dir().unwrap().display() | ||
| ); | ||
|
|
||
| let serialized_block = unsafe { | ||
| if !serialized.is_null() { | ||
| std::slice::from_raw_parts(serialized, len) | ||
| } else { | ||
| &[] | ||
| } | ||
| }; | ||
|
|
||
| let output_slice = unsafe { | ||
| if !output.is_null() { | ||
| std::slice::from_raw_parts_mut(output, output_len) | ||
| } else { | ||
| panic!("Output buffer is null") | ||
| } | ||
| }; | ||
|
|
||
| let binary_path_slice = unsafe { | ||
| if !binary_path.is_null() { | ||
| std::slice::from_raw_parts(binary_path, binary_path_len) | ||
| } else { | ||
| &[] | ||
| } | ||
| }; | ||
|
|
||
| let binary_path = std::str::from_utf8(binary_path_slice).unwrap(); | ||
| let elf_bytes = fs::read(binary_path).unwrap(); | ||
|
|
There was a problem hiding this comment.
sp1_prove accepts attacker-controlled len, output_len, and binary_path_len from the FFI boundary but does not enforce any maximums before creating slices, reading the ELF, or serializing the proof. This makes it easy to trigger excessive allocations / OOM or process aborts. Consider adding explicit MAX_* bounds (as done in openvm-glue) for input length, output buffer length, path length, and ELF size before proceeding.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #[no_mangle] | ||
| extern "C" fn sp1_prove( | ||
| serialized: *const u8, | ||
| len: usize, | ||
| binary_path: *const u8, | ||
| binary_path_len: usize, | ||
| output: *mut u8, | ||
| output_len: usize, | ||
| ) -> u32 { | ||
| println!( | ||
| "Running the SP1 transition prover, current dir={}", | ||
| std::env::current_dir().unwrap().display() | ||
| ); | ||
|
|
||
| let serialized_block = unsafe { | ||
| if !serialized.is_null() { | ||
| std::slice::from_raw_parts(serialized, len) | ||
| } else { | ||
| &[] | ||
| } | ||
| }; | ||
|
|
||
| let output_slice = unsafe { | ||
| if !output.is_null() { | ||
| std::slice::from_raw_parts_mut(output, output_len) | ||
| } else { | ||
| panic!("Output buffer is null") | ||
| } | ||
| }; | ||
|
|
||
| let binary_path_slice = unsafe { | ||
| if !binary_path.is_null() { | ||
| std::slice::from_raw_parts(binary_path, binary_path_len) | ||
| } else { | ||
| &[] | ||
| } | ||
| }; | ||
|
|
||
| let binary_path = std::str::from_utf8(binary_path_slice).unwrap(); | ||
| let elf_bytes = fs::read(binary_path).unwrap(); | ||
|
|
||
| let client = ProverClient::new(); | ||
| let (pk, vk) = client.setup(&elf_bytes); | ||
|
|
||
| let mut stdin = SP1Stdin::new(); | ||
| // Write 4-byte length prefix followed by the actual data | ||
| let len_bytes = (serialized_block.len() as u32).to_le_bytes(); | ||
| stdin.write_slice(&len_bytes); | ||
| stdin.write_slice(serialized_block); |
There was a problem hiding this comment.
sp1_prove accepts len, output_len, and binary_path_len without any upper bounds. Since SP1Stdin/bincode will allocate based on these inputs, a malformed or untrusted caller can trigger excessive allocations or panics. Consider adding explicit maximums (similar to openvm-glue’s MAX_INPUT_LEN/MAX_OUTPUT_LEN/MAX_PATH_LEN) and returning an error/aborting early when exceeded.
| #[no_mangle] | ||
| extern "C" fn ziren_prove( | ||
| serialized: *const u8, | ||
| len: usize, | ||
| binary_path: *const u8, | ||
| binary_path_len: usize, | ||
| output: *mut u8, | ||
| output_len: usize, | ||
| ) -> u32 { | ||
| println!( | ||
| "Running the Ziren transition prover, current dir={}", | ||
| std::env::current_dir().unwrap().display() | ||
| ); | ||
|
|
||
| let serialized_block = unsafe { | ||
| if !serialized.is_null() { | ||
| std::slice::from_raw_parts(serialized, len) | ||
| } else { | ||
| &[] | ||
| } | ||
| }; | ||
|
|
||
| let output_slice = unsafe { | ||
| if !output.is_null() { | ||
| std::slice::from_raw_parts_mut(output, output_len) | ||
| } else { | ||
| panic!("Output buffer is null") | ||
| } | ||
| }; | ||
|
|
||
| let binary_path_slice = unsafe { | ||
| if !binary_path.is_null() { | ||
| std::slice::from_raw_parts(binary_path, binary_path_len) | ||
| } else { | ||
| &[] | ||
| } | ||
| }; | ||
|
|
||
| let binary_path = std::str::from_utf8(binary_path_slice).unwrap(); | ||
| let elf_bytes = fs::read(binary_path).unwrap(); | ||
|
|
||
| let client = ProverClient::new(); | ||
| let (pk, vk) = client.setup(&elf_bytes); | ||
|
|
||
| let mut stdin = ZKMStdin::new(); | ||
| // Write 4-byte length prefix followed by the actual data | ||
| let len_bytes = (serialized_block.len() as u32).to_le_bytes(); | ||
| stdin.write_slice(&len_bytes); | ||
| stdin.write_slice(serialized_block); |
There was a problem hiding this comment.
ziren_prove does not enforce any maximums for len, output_len, or binary_path_len. Since proving and serialization will allocate proportional to these values, a malformed/untrusted caller can cause large allocations or panics. Consider adding explicit caps (like openvm-glue’s MAX_* constants) to fail fast and keep FFI usage safer.
| // libssl and libcrypto. | ||
| comp.linkSystemLibrary("ssl"); | ||
| comp.linkSystemLibrary("crypto"); |
There was a problem hiding this comment.
On Linux, addRustGlueLib now unconditionally links ssl and crypto for every build target. This introduces a hard system dependency on OpenSSL even for -Dprover=dummy builds, and can break builds in minimal environments where libssl/libcrypto aren’t installed. Consider linking these libraries only for provers/crates that actually require OpenSSL (or switching the Rust dependencies to rustls where feasible), and/or gating this behind a build option.
| // libssl and libcrypto. | |
| comp.linkSystemLibrary("ssl"); | |
| comp.linkSystemLibrary("crypto"); | |
| // libssl and libcrypto when those provers are enabled. | |
| switch (prover) { | |
| .dummy => {}, | |
| else => { | |
| comp.linkSystemLibrary("ssl"); | |
| comp.linkSystemLibrary("crypto"); | |
| }, | |
| } |
No description provided.