Fix vulnerability: potential memory corruption in circuit_parser_qsim.cc#1098
Fix vulnerability: potential memory corruption in circuit_parser_qsim.cc#1098mhucka wants to merge 7 commits into
Conversation
An internal security scan reported a memory corruption vulnerability (CWE-125) in `tensorflow_quantum/core/src/circuit_parser_qsim.cc`. 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. More details from the bug report: The function `SingleConstantGate` and related parsing routines (`TwoConstantGate`, `SingleEigenGate`) extract qubit index targets directly by invoking array offset methods such as `op.qubits(0)` without first verifying `op.qubits_size() >= 1`. An empty array causes `.qubits(0)` to read out-of-bounds of Protobuf structures, resolving dummy arrays randomly. When `absl::SimpleAtoi` takes this dummy garbage to parse an integer, it typically returns `false` (meaning failure) but that result is dynamically cast to `(void)` and thrown away. Consequently, the local stack variable (e.g., `unsigned int q0;`) remains completely uninitialized and retains garbage memory contents. This arbitrary uninitialized variable is then algebraically operated on (`num_qubits - q0 - 1`) and pushed to the Simulator core as a valid bounds operator, prompting direct arbitrary offset out-of-bounds indexing in Simulation arrays. This bypasses preliminary checks in `program_resolution.cc` since an empty `qubits` list will securely skip `for (const Qubit& qubit : operation.qubits())` registering loops. As long as the operation explicitly satisfies other attributes like `"control_qubits"` by passing valid empty strings (avoiding `.at()` exceptions), the uninitialized index memory payload will detonate completely in the core backend.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request enhances the robustness of the Qsim circuit parser by adding extensive input validation and error handling for qubit indices, control arguments, and channel configurations, accompanied by comprehensive unit tests. The review feedback suggests further strengthening this validation by ensuring both control qubits and values are provided if either is specified, and verifying that two-qubit gates operate on distinct qubits to prevent undefined behavior.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Code Review
This pull request introduces robust input validation and error checking across various gate and channel parser functions in circuit_parser_qsim.cc, ensuring qubit indices are parseable, within valid ranges, and distinct where necessary. It also adds comprehensive unit tests in circuit_parser_qsim_test.cc to verify these error-handling paths. The review feedback correctly identifies copy-paste errors in the newly added error messages where TwoEigenGate, FsimGate, and PhasedISwapGate incorrectly refer to TwoConstantGate when reporting duplicate qubits.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
An internal security scan reported a memory corruption vulnerability (CWE-125) in
tensorflow_quantum/core/src/circuit_parser_qsim.cc. (b/510433099)The
ParseAppendGateandParseProtoControlsfunctions accessop.qubits(i)or argument maps without verifying existence. They also useabsl::SimpleAtoiand ignore its failure, using uninitialized stack variables as qubit indices. This leads to heap out-of-bounds reads/writes in the simulator.More details from the bug report:
The function
SingleConstantGateand related parsing routines (TwoConstantGate,SingleEigenGate) extract qubit index targets directly by invoking array offset methods such asop.qubits(0)without first verifyingop.qubits_size() >= 1. An empty array causes.qubits(0)to read out-of-bounds of Protobuf structures, resolving dummy arrays randomly.When
absl::SimpleAtoitakes this dummy garbage to parse an integer, it typically returnsfalse(meaning failure) but that result is dynamically cast to(void)and thrown away. Consequently, the local stack variable (e.g.,unsigned int q0;) remains completely uninitialized and retains garbage memory contents. This arbitrary uninitialized variable is then algebraically operated on (num_qubits - q0 - 1) and pushed to the Simulator core as a valid bounds operator, prompting direct arbitrary offset out-of-bounds indexing in Simulation arrays.This bypasses preliminary checks in
program_resolution.ccsince an emptyqubitslist will securely skipfor (const Qubit& qubit : operation.qubits())registering loops. As long as the operation explicitly satisfies other attributes like"control_qubits"by passing valid empty strings (avoiding.at()exceptions), the uninitialized index memory payload will detonate completely in the core backend.