Skip to content

Check qubit arity & bounds in qsim parser - #1122

Open
mhucka wants to merge 1 commit into
tensorflow:masterfrom
mhucka:fix-bug-parser-bounds
Open

mhucka wants to merge 1 commit into
tensorflow:masterfrom
mhucka:fix-bug-parser-bounds

Conversation

@mhucka

@mhucka mhucka commented Oct 13, 2026

Copy link
Copy Markdown
Member

This fixes b/510433099, "[TFQ] Memory Corruption via Uninitialized or Unchecked Qubit IDs in Parsing":

The ParseAppendGate and ParseProtoControls functions access
op.qubits(i) or argument maps without verifying existence. They also
use absl::SimpleAtoi and ignore its failure, using uninitialized stack
variables as qubit indices. This leads to heap out-of-bounds
reads/writes in the simulator.

This is an instance of CWE-125.

The solution implemented here is in the form of a ParseQubitIds helper, then use that in all 15 gate/channel builders plus control-qubit checks.

With respect to performance, ParseQubitIds is inline in the anonymous namespace of circuit_parser_qsim.cc and is always called with a compile-time-sized 1- or 2-element std::initializer_list<unsigned int*>:({&q0} or {&q0, &q1}). Under -O2/-c opt, the compiler inlines the call, folds qubits.size() to 1 or 2, unrolls the loop, and eliminates the pointer indirection. There is no function-call overhead.

This fixes b/510433099, "[TFQ] Memory Corruption via Uninitialized or
Unchecked Qubit IDs in Parsing":

> The ParseAppendGate and ParseProtoControls functions access
> op.qubits(i) or argument maps without verifying existence. They also
> use absl::SimpleAtoi and ignore its failure, using uninitialized stack
> variables as qubit indices. This leads to heap out-of-bounds
> reads/writes in the simulator.

This is an instance of CWE-125.

The solution implemented here is in the form of a `ParseQubitIds`
helper, then use that in all 15 gate/channel builders plus control-qubit
checks.

With respect to performance, `ParseQubitIds` is inline in the anonymous
namespace of `circuit_parser_qsim.cc` and is always called with a
compile-time-sized 1- or 2-element `std::initializer_list<unsigned
int*>:({&q0} or {&q0, &q1})`. Under -O2/-c opt, the compiler inlines the
call, folds `qubits.size()` to 1 or 2, unrolls the loop, and eliminates
the pointer indirection. There is no function-call overhead.
@mhucka
mhucka marked this pull request as ready for review October 13, 2026 23:20

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces robust validation for qubit and control qubit IDs in the qsim circuit parser. It adds a new helper function ParseQubitIds to verify that qubit IDs are valid integers within the allowed range and are distinct. It also updates control qubit parsing to ensure control qubits are valid and distinct from both each other and target qubits. Comprehensive unit tests are added to verify these validation rules across various gates and channels. I have no feedback to provide as there are no review comments.

@mhucka mhucka added the priority/before-0.7.7 To be done before releasing TFQ version 0.7.7 label Oct 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority/before-0.7.7 To be done before releasing TFQ version 0.7.7

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant