fix: boomerang variable in sha256 hash function (#8581)

Static analyzer found boomerang variable in the function extend_witness.

The problem is that variable w_out wasn't connected with variable
w_out_raw in the function extend_witness in sha256 hash function. As a
result, you can put random variable in the extend_witness.

Test was created to prove this issue. You can modify a result of
function extend_witness, and the circuit will be correct.

Also function extend_witness was patched to remove this issue.

---------

Co-authored-by: Rumata888 <isennovskiy@gmail.com>

Author: DanielKotov

barretenberg/cpp/src/barretenberg/goblin/mock_circuits.hpp

@@ -56,7 +56,7 @@ class GoblinMockCircuits {
     {
         if (large) { // Results in circuit size 2^19
             stdlib::generate_sha256_test_circuit(builder, 12);
-            stdlib::generate_ecdsa_verification_test_circuit(builder, 11);
+            stdlib::generate_ecdsa_verification_test_circuit(builder, 10);
             stdlib::generate_merkle_membership_test_circuit(builder, 12);
         } else { // Results in circuit size 2^17
             stdlib::generate_sha256_test_circuit(builder, 9);

barretenberg/cpp/src/barretenberg/stdlib/hash/sha256/sha256.test.cpp

@@ -21,6 +21,7 @@ using Builder = UltraCircuitBuilder;
 using byte_array_ct = byte_array<Builder>;
 using packed_byte_array_ct = packed_byte_array<Builder>;
 using field_ct = field_t<Builder>;
+using witness_ct = witness_t<Builder>;
 
 constexpr uint64_t ror(uint64_t val, uint64_t shift)
 {
@@ -425,3 +426,38 @@ TEST(stdlib_sha256, test_input_str_len_multiple)
         EXPECT_EQ(circuit_output, expected);
     }
 }
+
+TEST(stdlib_sha256, test_boomerang_value_regression)
+{
+    auto builder = Builder();
+    std::array<field_t<Builder>, 16> input;
+
+    // Create random input witnesses and ensure that the witnesses are constrained to constants
+    for (size_t i = 0; i < 16; i++) {
+        auto random32bits = engine.get_random_uint32();
+        field_ct elt(witness_ct(&builder, fr(random32bits)));
+        elt.fix_witness();
+        input[i] = elt;
+    }
+    // Check correctness
+    std::array<field_t<Builder>, 64> w_ext = sha256_plookup::extend_witness(input);
+    bool result1 = CircuitChecker::check(builder);
+    EXPECT_EQ(result1, true);
+    bool result2 = false;
+    for (auto& single_extended_witness : w_ext) {
+
+        auto random32bits = engine.get_random_uint32();
+        uint32_t variable_index = single_extended_witness.witness_index;
+        // Ensure our random value is different
+        while (builder.variables[builder.real_variable_index[variable_index]] == fr(random32bits)) {
+            random32bits = engine.get_random_uint32();
+        }
+        auto backup = builder.variables[builder.real_variable_index[variable_index]];
+        builder.variables[builder.real_variable_index[variable_index]] = fr(random32bits);
+        // Check that the circuit fails
+        result2 = result2 || CircuitChecker::check(builder);
+        builder.variables[builder.real_variable_index[variable_index]] = backup;
+    }
+    // If at least one of the updated witnesses hasn't caused the circuit to fail, we're in trouble
+    EXPECT_EQ(result2, false);
+}

barretenberg/cpp/src/barretenberg/stdlib/hash/sha256/sha256_plookup.cpp

@@ -139,7 +139,15 @@ std::array<field_t<Builder>, 64> extend_witness(const std::array<field_t<Builder
         } else {
             w_out = witness_t<Builder>(
                 ctx, fr(w_out_raw.get_value().from_montgomery_form().data[0] & (uint64_t)0xffffffffULL));
+            static constexpr fr inv_pow_two = fr(2).pow(32).invert();
+            // If we multiply the field elements by constants separately and then subtract, then the divisor is going to
+            // be in a normalized state right after subtraction and the call to .normalize() won't add gates
+            field_pt w_out_raw_inv_pow_two = w_out_raw * inv_pow_two;
+            field_pt w_out_inv_pow_two = w_out * inv_pow_two;
+            field_pt divisor = (w_out_raw_inv_pow_two - w_out_inv_pow_two).normalize();
+            ctx->create_new_range_constraint(divisor.witness_index, 3);
         }
+
         w_sparse[i] = sparse_witness_limbs(w_out);
     }