Tampering with public value does not make the proving fail

[TheQuantumPhysicist]

There seems to be either a bug or a design problem, or my lack of knowledge in this system is the issue. I was experimenting with SP1 and reading the docs, and I reached quick start. After creating the project with

cargo prove new fibonacci

Then I build the program with

cd program && cargo prove build

Then I prove it with

cd script
RUST_LOG=info cargo run --release -- --prove

All good so far. However, if I change the proof variable to mutable, and add these lines after the proof to tamper with the proof's public values:

        //////// Attempt to tamper with the values
        println!("Proof: {:?}", proof.proof);
        let mut decoded_output =
            PublicValuesStruct::abi_decode(proof.public_values.as_slice(), true).unwrap();
        let PublicValuesStruct { n, a, b } = decoded_output;

        println!("Public values: {n}, {a}, {b}");

        decoded_output.a = 42;
        decoded_output.b = 4242;
        decoded_output.n = 424242;
        let encoded = alloy_sol_types::SolValue::abi_encode(&decoded_output);
        let sp1_encoded: sp1_sdk::SP1PublicValues = sp1_sdk::SP1PublicValues::from(&encoded);
        proof.public_values = sp1_encoded;
        ////////

The code in the next lines still works! The verification succeeds!

I don't know if this is intended behavior... but if it's the intended behavior, the function ProverClient::verify() takes SP1ProofWithPublicValues, not SP1Proof, which doesn't need the public values. Why?

I can see in the function Prover::verify(), that the public values are being used in the plonk arm. This indicates that this is a bug. How could the verification pass, regardless of the values?

I cannot be sure what the intent is... this is either a bug, where the verification should fail after mutating the public values, or an encapsulation issue, where the invariants of the struct are unclear. Since the verification requires the presence of the public values in the code, I should assume that they should be part of the verification. If they're not required, then verification should accept only what's needed, like SP1Proof, not SP1ProofWithPublicValues. I hope my ignorance on the details isn't tricking me.

[jtguibas]

If you run the code with the following modifications, the verification will fail.

 //////// Attempt to tamper with the values
println!("Proof: {:?}", proof.proof);
        let mut decoded_output =
            PublicValuesStruct::abi_decode(proof.[public_values.as](http://public_values.as/)_slice(), true).unwrap();
        let PublicValuesStruct { n, a, b } = decoded_output;
        println!("Public values: {n}, {a}, {b}");
        decoded_output.a = 42;
        decoded_output.b = 4242;
        decoded_output.n = 424242;
        let encoded = alloy_sol_types::SolValue::abi_encode(&decoded_output);
        let sp1_encoded: sp1_sdk::SP1PublicValues = sp1_sdk::SP1PublicValues::from(&encoded);
        proof.public_values = sp1_encoded;
        match &mut proof.proof {
            SP1Proof::Core(proofs) => {
                proofs[0].public_values[0] = Default::default();
            }
            _ => panic!("something wrong"),
        }
        ////////

The reason why this happens is because you are modifying the public_values which is the "human friendly decoding in bytes" instead of the public_values inside the cryptographic proof itself. This seems to only affects non succinct proofs (i.e., core/compress) and not proofs for PLONK or Groth16.

Thanks for finding this, it has been fixed in #1501.

[TheQuantumPhysicist]

Thank you for fixing the bug. Though I'm a little confused with this conclusion. Is it possible to verify the proof without the public values? What is it that one has to serialize (and send over wire, if necessary)? From what you're saying, it sounds like the public values are somehow not required, because they're embedded in the cryptographic proof. Can one prove only using SP1Proof? If yes, why does the verification function require SP1ProofWithPublicValues?