-
Notifications
You must be signed in to change notification settings - Fork 574
Description
This issue originated from a discussion with @therealyingtong.
halo2/halo2_gadgets/src/utilities/decompose_running_sum.rs
Lines 197 to 202 in f9838c1
| assert_eq!(zs.len(), num_windows + 1); | |
| if strict { | |
| // Constrain the final running sum output to be zero. | |
| region.constrain_constant(zs.last().unwrap().cell(), F::ZERO)?; | |
| } |
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.