zcash / halo2

The Halo2 zero-knowledge proving system
https://zcash.github.io/halo2/
Other
712 stars 487 forks source link

Modify constraint in `decompose_running_sum` #789

Open enricobottazzi opened 1 year ago

enricobottazzi commented 1 year ago

This issue originated from a discussion with @therealyingtong.

https://github.com/zcash/halo2/blob/f9838c127ec9c14f6f323e0cfdc0c1392594d37f/halo2_gadgets/src/utilities/decompose_running_sum.rs#L197-L202

This constraint assumes that the length of zs is equal to num_windows + 1.

If the length of zs is not constrained to be equal to num_windows + 1 an attacker could add an extra zero-value to zs and this would cause the constrain_constant(zs.last().unwrap().cell(), F::ZERO)? to trivially pass, even in the case of a word decomposition that doesn't fit in WINDOW_NUM_BITS * num_windows bits.

What we concluded is this attack is not possible since the loop that builds zs goes over words which is a vector of dimension num_windows so the length of zs is known at keygen time (and it is equal to num_windows + 1).

Despite that, we believe that removing the expression assert_eq!(zs.len(), num_windows + 1); and adding the constraint region.constrain_constant(zs[num_windows].cell(), F::ZERO)?; would make it clearer.