c4-submissions commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-10-zksync/blob/1fb4649b612fac7b4ee613df6f6b7d921ddd6b0d/code/era-zkevm_circuits/src/code_unpacker_sha256/mod.rs#L208-L210

Vulnerability details

Impact

In code unpacker, version hash is not correctly enforced.

Proof of Concept

In code unpacker, we enforce that version hash is correct.

    let version_hash_matches = UInt16::equals(cs, &chunks[1], &versioned_hash_top_16_bits);
    state.state_get_from_queue.conditionally_enforce_true(cs, version_hash_matches);

The second line means, if version hash matches, state_get_from_queue must be true. But this is not what we want. The logic should be, if we are getting from queue, version hash must match. Otherwise, even if version hash is incorrect, this line passes as no constraint is actually enforced.

Let's take a closer look at fn conditionally_enforce_true, bool should_enforce is the third parameter.

pub fn conditionally_enforce_true<CS: ConstraintSystem<F>>(
    &self,
    cs: &mut CS,
    should_enforce: Self,
) {
    // this is equal to having !self && should_enforce == false;
    // so (1 - self) * should_enforce == 0

    if cs.gate_is_allowed::<FmaGateInBaseFieldWithoutConstant<F>>() {
        let zero_var = cs.allocate_constant(F::ZERO);

        let gate = FmaGateInBaseFieldWithoutConstant {
            params: FmaGateInBaseWithoutConstantParams {
                coeff_for_quadtaric_part: F::MINUS_ONE,
                linear_term_coeff: F::ONE,
            },
            quadratic_part: (self.variable, should_enforce.variable),
            linear_part: should_enforce.variable,
            rhs_part: zero_var,
        };

        gate.add_to_cs(cs);
    } else {
        unimplemented!()
    }
}