fix!: bitwise ghost row vulnerablities - start_sha and start_keccak (#19875)

## Summary

- Fix a vulnerability in `bitwise.pil` where `start_keccak` and
`start_sha256` destination selectors can be activated on inactive rows
(`sel=0`), allowing a malicious prover to forge arbitrary XOR/AND
results for keccak and SHA256 hash computations.
- Add exploit tests demonstrating the vulnerability and verifying the
fix.

## Vulnerability

The `start_keccak` and `start_sha256` columns serve as lookup
destination selectors for keccak permutation and SHA256 compression XOR
operations respectively. No constraint prevented these selectors from
being set to 1 on rows where `sel=0`. Since all bitwise polynomial
constraints (including `BYTE_OPERATIONS`) are gated by `sel`, a ghost
row with `sel=0` bypasses all correctness checks while still appearing
as a valid lookup destination.

## Attack Path (Keccak)

1. **Ghost row creation**: The prover fills an unused row (`sel=0`) in
the bitwise subtrace with `start=1`, `start_keccak=1`, and a fake
accumulator output (`acc_ic = FORGED_XOR_RESULT`). A tag mismatch
(`tag_a != tag_b`) forces `err=1`, which sets `sel_get_ctr=0` (skipping
the `INTEGRAL_TAG_LENGTH` lookup), sets `last=1`, and leaves `ctr`
unconstrained (so `ctr=0` implies `sel=0`). All polynomial constraints
are genuinely satisfied on this row.
2. **No conflict with `DISPATCH_TO_BITWISE`**: The execution-to-bitwise
dispatch lookup uses `bitwise.start` as its destination selector. The
ghost row is in this destination table (`start=1`), but its counts
column is 0 (no execution source matches it). In log-derivative lookups,
destination entries with multiplicity 0 are valid — they don't
contribute to the sum.
3. **Keccak lookup satisfied**: The `THETA_XOR_01` lookup uses
`bitwise.start_keccak` as destination. The ghost row has
`start_keccak=1` and matching tuple values (with the forged output). The
counts column is 1, satisfying the lookup for the mutated source row.
4. **Forging the committed column**: `theta_xor_01` in the keccak trace
is a committed polynomial constrained only by the `THETA_XOR_01` lookup
— no polynomial relation independently enforces `theta_xor_01 =
state_in_00 XOR state_in_01`. The ghost row provides the matching
destination entry for an arbitrary value, completely breaking the keccak
permutation.

## Attack Path (SHA256)

1. **Ghost row creation**: Same mechanism as keccak, but with
`start_sha256=1` instead. The prover sets `tag_a=U32, tag_b=U8` (tag
mismatch) to force the error path with `sel=0`.
2. **No conflict with `DISPATCH_TO_BITWISE`**: Same as keccak — the
ghost row has `start=1` with multiplicity 0 for the execution dispatch
lookup.
3. **SHA256 lookup satisfied**: The `W_S_0_XOR_0` lookup (and similar
SHA256 XOR lookups) use `bitwise.start_sha256` as destination. The ghost
row has `start_sha256=1` and matching tuple values with a forged XOR
output.
4. **Forging the committed column**: `w_15_rotr_7_xor_w_15_rotr_18` in
the SHA256 trace is a committed polynomial constrained only by the
`W_S_0_XOR_0` lookup. By providing a forged destination entry, the
attacker corrupts the SHA256 message schedule computation (`sigma_0`),
producing arbitrary compression outputs.

## Fix

Added two constraints to `bitwise.pil`:
```
+#[BITW_START_ONLY_WHEN_SEL]
+(start_keccak + start_sha256) * (1 - sel) = 0;
```

This ensures keccak/sha256 destination selectors can only be active on
rows where `sel=1`, meaning the `BYTE_OPERATIONS` lookup enforces
correct byte-level XOR/AND computation. Note: the more general `start *
(1 - sel) = 0` would be too aggressive since error rows legitimately
need `start=1` with `sel=0` for the execution dispatch.

## Test plan

- [x] Exploit tests (`VulnerabilityStartKeccakWithoutSel`,
`VulnerabilityStartSha256WithoutSel`) that are turned into expected
constraint violation tests after the fix
- [x] Full exploit tests (`VulnerabilityFakeKeccakXorOutput`,
`VulnerabilityFakeSha256XorOutput`) that are turned into expected
constraint violation tests after the fix — ghost rows are rejected
- [x] Existing positive bitwise tests (AND/OR/XOR with tracegen, mixed
operations, error handling) continue to pass
- [x] Keccak and SHA256 integration tests pass (legitimate operations
unaffected by the new constraints)

---------

Co-authored-by: jeanmon <[email protected]>

Author: dbanks12

barretenberg/cpp/pil/vm2/bitwise.pil

@@ -2,6 +2,44 @@ include "precomputed.pil";
 include "constants_gen.pil";
 
 /**
+ * ==============================
+ * INVOCATION WITH ERROR HANDLING
+ * ==============================
+ * Used by execution dispatch #[DISPATCH_TO_BITWISE] in execution.pil for bitwise opcodes
+ * and reads back the tag error flag.
+ *
+ * sel_exec_dispatch_bitwise {
+ *    subtrace_operation_id, sel_opcode_error,
+ *    register[0], mem_tag_reg[0],
+ *    register[1], mem_tag_reg[1],
+ *    register[2], mem_tag_reg[2]
+ * } in bitwise.start {
+ *    bitwise.op_id, bitwise.err,
+ *    bitwise.acc_ia, bitwise.tag_a,
+ *    bitwise.acc_ib, bitwise.tag_b,
+ *    bitwise.acc_ic, bitwise.tag_c
+ * };
+ *
+ * =================================
+ * INVOCATION WITHOUT ERROR HANDLING
+ * =================================
+ *
+ * Used by both `keccakf1600.pil` and `sha256.pil` sub-traces using their respective selectors:
+ * `start_keccak` and `start_sha256`. It assumes that the bitwise operation is always performed
+ * as tags of both inputs are known to be valid. Therefore, there is no error handling. However,
+ * tracegen still needs to populate the trace to satisfy the constraints related to error handling.
+ *
+ * Usage is of the form:
+ *
+ * sel_XXX { a, b, c, xor_sel, tag_a }
+ * in
+ * bitwise.start_XXX { bitwise.acc_ia, bitwise.acc_ib, bitwise.acc_ic, bitwise.op_id, bitwise.tag_a };
+ *
+ * where start_XXX is either start_keccak or start_sha256.
+ * Note that the order of the tuple columns might differ though. As long as the correct columns
+ * are passed, the usage is sound.
+ *
+ *
  * The bitwise trace performs AND/OR/XOR operations on non-FF types. It does
  * this by decomposing the inputs into 8-bit chunks and looks up the resulting
  * chunk of the operation in the precomputed bitwise table. The final output is
@@ -61,6 +99,14 @@ start_sha256 * (1 - start_sha256) = 0;
 // If any of the above selectors is 1, then start must be 1.
 (start_keccak + start_sha256) * (1 - start) = 0;
 
+// Crucial constraint to prevent the vulnerability described in PR #19875 consisting
+// of maliciously setting start=1 on inactive rows (sel=0) by toggling an error
+// and forging in any bitwise operation result.
+#[BITW_START_ONLY_WHEN_SEL]
+(start_keccak + start_sha256) * (1 - sel) = 0;
+// Note: the more general `start * (1 - sel) = 0` would be too aggressive since error rows
+// legitimately need start=1 with sel=0 for the execution dispatch (#[DISPATCH_TO_BITWISE]).
+
 // To support dynamically sized memory operands we use a counter against a lookup
 // This decrementing counter goes from [TAG_LEN, 0] where TAG_LEN is the number of bytes in the
 // corresponding integer. i.e. TAG_LEN is between 1 (U1/U8) and 16 (U128).

barretenberg/cpp/pil/vm2/sha256.pil

@@ -107,7 +107,7 @@ namespace sha256;
 
     // Propagate initial state values across all rounds.
     // Initial states must be propagated correctly becuse they are used in the `last` row
-    // when adding to the compression output.  `last` is mutually exclusive to `perform_round` but 
+    // when adding to the compression output. `last` is mutually exclusive to `perform_round` but
     // these constraints don't need to use `last`  because on the 64th row of `perform_round` we
     // will constrain the next row (i.e. the `last` row) to be correct.
     #[PROPAGATE_INIT_A]
@@ -289,9 +289,9 @@ namespace sha256;
 
     pol commit w_s_1;
     #[W_S_1_XOR_1]
-    sel_compute_w { w_2_rotr_17_xor_w_2_rotr_19, w_2_rshift_10, w_s_1, xor_sel, u32_tag, u32_tag }
+    sel_compute_w { w_2_rotr_17_xor_w_2_rotr_19, w_2_rshift_10, w_s_1, xor_sel, u32_tag }
     in
-    bitwise.start_sha256 { bitwise.acc_ia, bitwise.acc_ib, bitwise.acc_ic, bitwise.op_id, bitwise.tag_a, bitwise.tag_a };
+    bitwise.start_sha256 { bitwise.acc_ia, bitwise.acc_ib, bitwise.acc_ic, bitwise.op_id, bitwise.tag_a };
 
     // ========== START OF COMPRESSION BLOCK ==================