fix!: multiple traces had ghost row injection vulnerabilities #19480

dbanks12 · 2026-01-09T23:37:09Z

Summary

Fixes ghost row injection vulnerabilities in three PIL traces where sub-selectors firing permutations were not constrained to be 0 when the main selector was inactive.

The vulnerability pattern: When sel=0 (gadget inactive), sub-selectors like is_write_memory_value could still be set to 1 by a malicious prover. This would fire permutations to memory, allowing arbitrary memory writes without going through legitimate execution paths.

The fix pattern: Add constraints of the form sub_selector * (1 - sel) = 0 which forces sub-selectors to 0 when sel=0.

Changes

PIL Fixes

File	Constraint Added
`emit_unencrypted_log.pil`	`#[IS_WRITE_MEMORY_VALUE_REQUIRES_SEL]`
`get_contract_instance.pil`	`#[IS_VALID_WRITES_IN_BOUNDS_REQUIRES_SEL]`
`to_radix_mem.pil`	`#[SEL_SHOULD_WRITE_MEM_REQUIRES_SEL]`

Test Updates

Updated ghost row injection tests to verify fixes block the attacks
Tests use EXPECT_THROW_WITH_MESSAGE to confirm constraints fail as expected
Removed debug output (cout statements)
Renamed tests from NegativeFullAttackWithAllTraces to NegativeGhostRowInjectionBlocked

dbanks12 · 2026-01-09T23:37:27Z

fix!: multiple traces had ghost row injection vulnerabilities #19480 👈 (View in Graphite)
merge-train/avm

This stack of pull requests is managed by Graphite. Learn more about stacking.

sirasistant · 2026-01-12T09:13:25Z

barretenberg/cpp/pil/vm2/opcodes/emit_unencrypted_log.pil

    // if we are not ending, the next is_write_memory_value is equal to current is_write_memory_value or is_write_contract_address
    // since both can't be on at the same time, we can use add.
    NOT_END * (is_write_memory_value + is_write_contract_address - is_write_memory_value') = 0;
+    // Force is_write_memory_value to 0 when sel is 0 (prevents ghost row injection attacks)


We could instead do sel_should_read_memory = sel * is_write_memory_value * (1 - error_out_of_bounds);. However it might not be necessary, since it does control a permutation, but a read only one.

I agree it is not a problem because superfluous illegitimate memory reads do not harm.
However, it is cleaner and cost is very minimal.
I would also favor the proposal of @sirasistant to add the gate sel.

sirasistant · 2026-01-12T09:18:32Z

barretenberg/cpp/pil/vm2/opcodes/get_contract_instance.pil

    WRITES_OUT_OF_BOUNDS * is_valid_member_enum = 0;
+    // Force is_valid_member_enum to 0 when sel is 0 (prevents ghost row injection attacks)
+    #[IS_VALID_MEMBER_ENUM_REQUIRES_SEL]
+    is_valid_member_enum * (1 - sel) = 0;


Should we instead do is_valid_writes_in_bounds*(1-sel) = 0? is upper in the chain of selectors, and we also disable is_valid_member_enum if is_valid_writes_in_bounds is zero

#[IS_VALID_MEMBER_ENUM_ONLY_SET_BY_PRECOMPUTED_LOOKUP] WRITES_OUT_OF_BOUNDS * is_valid_member_enum = 0;

I think it is a good idea @sirasistant. I would suggest then that @dbanks12 adds a comment that is_valid_member_enum == 0 outside of the active trace portion as a consequence of
#[IS_VALID_MEMBER_ENUM_ONLY_SET_BY_PRECOMPUTED_LOOKUP] and the new relation.

sirasistant · 2026-01-12T09:20:27Z

barretenberg/cpp/pil/vm2/to_radix_mem.pil

    NOT_LAST * (sel_should_write_mem' - sel_should_write_mem) = 0;
+    // Force sel_should_write_mem to 0 when sel is 0 (prevents ghost row injection attacks)
+    #[SEL_SHOULD_WRITE_MEM_REQUIRES_SEL]
+    sel_should_write_mem * (1 - sel) = 0;


jeanmon · 2026-01-12T09:34:58Z

barretenberg/cpp/pil/vm2/opcodes/emit_unencrypted_log.pil

    // if we are not ending, the next is_write_memory_value is equal to current is_write_memory_value or is_write_contract_address
    // since both can't be on at the same time, we can use add.
    NOT_END * (is_write_memory_value + is_write_contract_address - is_write_memory_value') = 0;
+    // Force is_write_memory_value to 0 when sel is 0 (prevents ghost row injection attacks)


I agree it is not a problem because superfluous illegitimate memory reads do not harm.
However, it is cleaner and cost is very minimal.
I would also favor the proposal of @sirasistant to add the gate sel.

jeanmon · 2026-01-12T09:45:00Z

barretenberg/cpp/pil/vm2/opcodes/get_contract_instance.pil

    WRITES_OUT_OF_BOUNDS * is_valid_member_enum = 0;
+    // Force is_valid_member_enum to 0 when sel is 0 (prevents ghost row injection attacks)
+    #[IS_VALID_MEMBER_ENUM_REQUIRES_SEL]
+    is_valid_member_enum * (1 - sel) = 0;


I think it is a good idea @sirasistant. I would suggest then that @dbanks12 adds a comment that is_valid_member_enum == 0 outside of the active trace portion as a consequence of
#[IS_VALID_MEMBER_ENUM_ONLY_SET_BY_PRECOMPUTED_LOOKUP] and the new relation.

jeanmon · 2026-01-12T09:49:32Z

barretenberg/cpp/src/barretenberg/vm2/constraining/relations/emit_unencrypted_log.test.cpp

+// =====================================================================
+// Ghost Row Injection Vulnerability Tests
+// =====================================================================
+// These tests verify that ghost rows (sel=0) cannot fire permutations.


If we keep this one, I would welcome a little comment that it is more of a "defensive/sanity" measure because this is a "memory read" and therefore there is no known exploitability.

jeanmon

Approved but please address my comment.

jeanmon · 2026-01-12T16:13:58Z

barretenberg/cpp/pil/vm2/opcodes/emit_unencrypted_log.pil

    // since both can't be on at the same time, we can use add.
    NOT_END * (is_write_memory_value + is_write_contract_address - is_write_memory_value') = 0;
+    // Force is_write_memory_value to 0 when sel is 0 (prevents ghost row injection attacks)
+    #[IS_WRITE_MEMORY_VALUE_REQUIRES_SEL]


But now @dbanks12 you can remove this one. Because, by definition, we have sel ==0 => sel_should_read_memory == 0.

AztecBot · 2026-01-12T17:57:36Z

Flakey Tests

🤖 says: This CI run detected 2 tests that failed, but were tolerated due to a .test_patterns.yml entry.

\033FLAKED\033 (8;;http://ci.aztec-labs.com/12aaab41e1425053�12aaab41e14250538;;�):  yarn-project/end-to-end/scripts/run_test.sh simple src/e2e_p2p/multiple_validators_sentinel.parallel.test.ts "collects attestations for validators in proposer node when block is not published" (106s) (code: 1) group:e2e-p2p-epoch-flakes (\033dbanks12\033: fix them)
\033FLAKED\033 (8;;http://ci.aztec-labs.com/43e8a8abfc6d8a58�43e8a8abfc6d8a588;;�):  yarn-project/end-to-end/scripts/run_test.sh simple src/e2e_p2p/broadcasted_invalid_block_proposal_slash.test.ts (196s) (code: 1) group:e2e-p2p-epoch-flakes (\033dbanks12\033: fix them)

BEGIN_COMMIT_OVERRIDE feat(avm security): add static check for isolated/unused columns (#19489) feat(avm): use noop calldata hasher in fast sim (#19495) chore(avm): rename indirect -> addressing mode (#19491) chore(avm): small cursor optimizations chore(avm):! rename indirect -> addressing mode (PIL) (#19493) fix(avm): constraint when unwinding empty call stack (#19485) feat(avm): Fuzz debug log and refactor env getter (#19494) fix!: ecc add predicate completeness bug (#19471) chore(avm): callstackmetadatacollector clarifications (#19490) chore: sanity assert in execution for bytecode id (#19486) fix!: sstore allowed injection of malicious write rows (#19470) fix!: defensive ghost row constraints in bc_hashing pil (#19481) fix(avm): fix execution::mov for mac? (#19507) chore(avm)!: resolve execution TODOs (#19501) fix!: multiple traces had ghost row injection vulnerabilities (#19480) fix(avm): defensively copy MemoryValues (#19512) feat: align TS and BB log levels (#19518) END_COMMIT_OVERRIDE

fix!: multiple traces had ghost row injection vulnerabilities

867b063

dbanks12 force-pushed the db/ghost-row-injections branch 2 times, most recently from 62a7ade to 5632cdc Compare January 10, 2026 00:05

dbanks12 marked this pull request as ready for review January 10, 2026 00:06

dbanks12 requested review from IlyasRidhuan, Maddiaa0, fcarreiro and jeanmon as code owners January 10, 2026 00:06

dbanks12 requested review from sirasistant and removed request for IlyasRidhuan, Maddiaa0 and fcarreiro January 10, 2026 00:06

sirasistant reviewed Jan 12, 2026

View reviewed changes

jeanmon requested changes Jan 12, 2026

View reviewed changes

dbanks12 force-pushed the db/ghost-row-injections branch from 5632cdc to 1ae3f53 Compare January 12, 2026 15:47

dbanks12 requested review from jeanmon and sirasistant January 12, 2026 15:47

dbanks12 force-pushed the db/ghost-row-injections branch from 1ae3f53 to 1d5608f Compare January 12, 2026 16:10

jeanmon approved these changes Jan 12, 2026

View reviewed changes

dbanks12 force-pushed the db/ghost-row-injections branch from 1d5608f to 5d70edb Compare January 12, 2026 16:53

fix them

c5ce137

dbanks12 force-pushed the db/ghost-row-injections branch from 5d70edb to c5ce137 Compare January 12, 2026 17:28

dbanks12 merged commit dd7eba0 into merge-train/avm Jan 12, 2026
9 checks passed

dbanks12 deleted the db/ghost-row-injections branch January 12, 2026 17:59

AztecBot mentioned this pull request Jan 12, 2026

feat: merge-train/avm #19496

Merged

fix!: multiple traces had ghost row injection vulnerabilities #19480

fix!: multiple traces had ghost row injection vulnerabilities #19480

Uh oh!

Conversation

dbanks12 commented Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

PIL Fixes

Test Updates

Uh oh!

dbanks12 commented Jan 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeanmon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AztecBot commented Jan 12, 2026

Flakey Tests

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dbanks12 commented Jan 9, 2026 •

edited

Loading