From f3606d9539096e7b1feccf8eaee8f5f084595776 Mon Sep 17 00:00:00 2001 From: mhucka Date: Wed, 24 Jun 2026 22:57:40 +0000 Subject: [PATCH 1/7] Mitigate potential memory corruption vulnerability 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. --- .../core/src/circuit_parser_qsim.cc | 292 +++++++++++++++--- .../core/src/circuit_parser_qsim_test.cc | 140 +++++++++ 2 files changed, 390 insertions(+), 42 deletions(-) diff --git a/tensorflow_quantum/core/src/circuit_parser_qsim.cc b/tensorflow_quantum/core/src/circuit_parser_qsim.cc index d9f8d80b3..260de5332 100644 --- a/tensorflow_quantum/core/src/circuit_parser_qsim.cc +++ b/tensorflow_quantum/core/src/circuit_parser_qsim.cc @@ -85,10 +85,16 @@ inline Status ParseProtoControls(const Operation& op, const unsigned int num_qubits, std::vector* control_qubits, std::vector* control_values) { + const auto control_qubits_it = op.args().find("control_qubits"); + const auto control_values_it = op.args().find("control_values"); + if (control_qubits_it == op.args().end() || control_values_it == op.args().end()) { + return ::tensorflow::Status(); + } + absl::string_view control_str = - op.args().at("control_qubits").arg_value().string_value(); + control_qubits_it->second.arg_value().string_value(); absl::string_view control_v_str = - op.args().at("control_values").arg_value().string_value(); + control_values_it->second.arg_value().string_value(); if (control_str == "" && control_v_str == "") { // empty default value set in serializer.py @@ -108,12 +114,18 @@ inline Status ParseProtoControls(const Operation& op, return ::tensorflow::Status(); } bool valid; - unsigned int tmp; + unsigned int tmp = 0; control_qubits->reserve(control_toks.size()); for (auto tok : control_toks) { - // don't bother error checking since this is done earlier - // in program_resolution. valid = absl::SimpleAtoi(tok, &tmp); + if (!valid) { + return Status(absl::StatusCode::kInvalidArgument, + "Unparseable control qubit index: " + std::string(tok)); + } + if (tmp >= num_qubits) { + return Status(absl::StatusCode::kInvalidArgument, + "Control qubit index out of range: " + std::to_string(tmp)); + } control_qubits->push_back(num_qubits - tmp - 1); } control_values->reserve(control_v_toks.size()); @@ -146,9 +158,6 @@ inline Status OptionalInsertControls(const Operation& op, } // series of fixed signature gate builders. -// there is no need to error check for unparseable symbols -// or proto args not being present. Those errors are caught -// upstream. // single qubit gate Create(time, q0) inline Status SingleConstantGate( @@ -156,8 +165,19 @@ inline Status SingleConstantGate( const std::function& create_f, const unsigned int num_qubits, const unsigned int time, QsimCircuit* circuit, std::vector* metadata) { - unsigned int q0; - (void)absl::SimpleAtoi(op.qubits(0).id(), &q0); + unsigned int q0 = 0; + if (op.qubits_size() < 1) { + return Status(absl::StatusCode::kInvalidArgument, + "SingleConstantGate requires at least 1 qubit."); + } + if (!absl::SimpleAtoi(op.qubits(0).id(), &q0)) { + return Status(absl::StatusCode::kInvalidArgument, + "Unparseable qubit index: " + op.qubits(0).id()); + } + if (q0 >= num_qubits) { + return Status(absl::StatusCode::kInvalidArgument, + "Qubit index out of range: " + std::to_string(q0)); + } auto gate = create_f(time, num_qubits - q0 - 1); Status s = OptionalInsertControls(op, num_qubits, &gate); if (!s.ok()) { @@ -181,9 +201,27 @@ inline Status TwoConstantGate( create_f, const unsigned int num_qubits, const unsigned int time, QsimCircuit* circuit, std::vector* metadata) { - unsigned int q0, q1; - (void)absl::SimpleAtoi(op.qubits(0).id(), &q0); - (void)absl::SimpleAtoi(op.qubits(1).id(), &q1); + unsigned int q0 = 0, q1 = 0; + if (op.qubits_size() < 2) { + return Status(absl::StatusCode::kInvalidArgument, + "TwoConstantGate requires at least 2 qubits."); + } + if (!absl::SimpleAtoi(op.qubits(0).id(), &q0)) { + return Status(absl::StatusCode::kInvalidArgument, + "Unparseable qubit index: " + op.qubits(0).id()); + } + if (!absl::SimpleAtoi(op.qubits(1).id(), &q1)) { + return Status(absl::StatusCode::kInvalidArgument, + "Unparseable qubit index: " + op.qubits(1).id()); + } + if (q0 >= num_qubits) { + return Status(absl::StatusCode::kInvalidArgument, + "Qubit index out of range: " + std::to_string(q0)); + } + if (q1 >= num_qubits) { + return Status(absl::StatusCode::kInvalidArgument, + "Qubit index out of range: " + std::to_string(q1)); + } auto gate = create_f(time, num_qubits - q0 - 1, num_qubits - q1 - 1); Status s = OptionalInsertControls(op, num_qubits, &gate); if (!s.ok()) { @@ -207,10 +245,21 @@ inline Status SingleEigenGate( create_f, const unsigned int num_qubits, const unsigned int time, QsimCircuit* circuit, std::vector* metadata) { - unsigned int q0; + unsigned int q0 = 0; float exp, exp_s, gs; Status u; - (void)absl::SimpleAtoi(op.qubits(0).id(), &q0); + if (op.qubits_size() < 1) { + return Status(absl::StatusCode::kInvalidArgument, + "SingleEigenGate requires at least 1 qubit."); + } + if (!absl::SimpleAtoi(op.qubits(0).id(), &q0)) { + return Status(absl::StatusCode::kInvalidArgument, + "Unparseable qubit index: " + op.qubits(0).id()); + } + if (q0 >= num_qubits) { + return Status(absl::StatusCode::kInvalidArgument, + "Qubit index out of range: " + std::to_string(q0)); + } absl::optional exponent_symbol; u = ParseProtoArg(op, "exponent", param_map, &exp, &exponent_symbol); @@ -255,11 +304,29 @@ inline Status TwoEigenGate( float, float)>& create_f, const unsigned int num_qubits, const unsigned int time, QsimCircuit* circuit, std::vector* metadata) { - unsigned int q0, q1; + unsigned int q0 = 0, q1 = 0; float exp, exp_s, gs; Status u; - (void)absl::SimpleAtoi(op.qubits(0).id(), &q0); - (void)absl::SimpleAtoi(op.qubits(1).id(), &q1); + if (op.qubits_size() < 2) { + return Status(absl::StatusCode::kInvalidArgument, + "TwoEigenGate requires at least 2 qubits."); + } + if (!absl::SimpleAtoi(op.qubits(0).id(), &q0)) { + return Status(absl::StatusCode::kInvalidArgument, + "Unparseable qubit index: " + op.qubits(0).id()); + } + if (!absl::SimpleAtoi(op.qubits(1).id(), &q1)) { + return Status(absl::StatusCode::kInvalidArgument, + "Unparseable qubit index: " + op.qubits(1).id()); + } + if (q0 >= num_qubits) { + return Status(absl::StatusCode::kInvalidArgument, + "Qubit index out of range: " + std::to_string(q0)); + } + if (q1 >= num_qubits) { + return Status(absl::StatusCode::kInvalidArgument, + "Qubit index out of range: " + std::to_string(q1)); + } absl::optional exponent_symbol; u = ParseProtoArg(op, "exponent", param_map, &exp, &exponent_symbol); @@ -394,10 +461,21 @@ inline Status PhasedXGate(const Operation& op, const SymbolMap& param_map, const unsigned int num_qubits, const unsigned int time, QsimCircuit* circuit, std::vector* metadata) { - int q0; + unsigned int q0 = 0; float pexp, pexp_s, exp, exp_s, gs; Status u; - (void)absl::SimpleAtoi(op.qubits(0).id(), &q0); + if (op.qubits_size() < 1) { + return Status(absl::StatusCode::kInvalidArgument, + "PhasedXGate requires at least 1 qubit."); + } + if (!absl::SimpleAtoi(op.qubits(0).id(), &q0)) { + return Status(absl::StatusCode::kInvalidArgument, + "Unparseable qubit index: " + op.qubits(0).id()); + } + if (q0 >= num_qubits) { + return Status(absl::StatusCode::kInvalidArgument, + "Qubit index out of range: " + std::to_string(q0)); + } absl::optional exponent_symbol; u = ParseProtoArg(op, "exponent", param_map, &exp, &exponent_symbol); @@ -453,11 +531,29 @@ inline Status FsimGate(const Operation& op, const SymbolMap& param_map, const unsigned int num_qubits, const unsigned int time, QsimCircuit* circuit, std::vector* metadata) { - int q0, q1; + unsigned int q0 = 0, q1 = 0; float theta, theta_s, phi, phi_s; Status u; - (void)absl::SimpleAtoi(op.qubits(0).id(), &q0); - (void)absl::SimpleAtoi(op.qubits(1).id(), &q1); + if (op.qubits_size() < 2) { + return Status(absl::StatusCode::kInvalidArgument, + "FsimGate requires at least 2 qubits."); + } + if (!absl::SimpleAtoi(op.qubits(0).id(), &q0)) { + return Status(absl::StatusCode::kInvalidArgument, + "Unparseable qubit index: " + op.qubits(0).id()); + } + if (!absl::SimpleAtoi(op.qubits(1).id(), &q1)) { + return Status(absl::StatusCode::kInvalidArgument, + "Unparseable qubit index: " + op.qubits(1).id()); + } + if (q0 >= num_qubits) { + return Status(absl::StatusCode::kInvalidArgument, + "Qubit index out of range: " + std::to_string(q0)); + } + if (q1 >= num_qubits) { + return Status(absl::StatusCode::kInvalidArgument, + "Qubit index out of range: " + std::to_string(q1)); + } absl::optional theta_symbol; u = ParseProtoArg(op, "theta", param_map, &theta, &theta_symbol); @@ -509,11 +605,29 @@ inline Status PhasedISwapGate(const Operation& op, const SymbolMap& param_map, const unsigned int num_qubits, const unsigned int time, QsimCircuit* circuit, std::vector* metadata) { - int q0, q1; + unsigned int q0 = 0, q1 = 0; float pexp, pexp_s, exp, exp_s; Status u; - (void)absl::SimpleAtoi(op.qubits(0).id(), &q0); - (void)absl::SimpleAtoi(op.qubits(1).id(), &q1); + if (op.qubits_size() < 2) { + return Status(absl::StatusCode::kInvalidArgument, + "PhasedISwapGate requires at least 2 qubits."); + } + if (!absl::SimpleAtoi(op.qubits(0).id(), &q0)) { + return Status(absl::StatusCode::kInvalidArgument, + "Unparseable qubit index: " + op.qubits(0).id()); + } + if (!absl::SimpleAtoi(op.qubits(1).id(), &q1)) { + return Status(absl::StatusCode::kInvalidArgument, + "Unparseable qubit index: " + op.qubits(1).id()); + } + if (q0 >= num_qubits) { + return Status(absl::StatusCode::kInvalidArgument, + "Qubit index out of range: " + std::to_string(q0)); + } + if (q1 >= num_qubits) { + return Status(absl::StatusCode::kInvalidArgument, + "Qubit index out of range: " + std::to_string(q1)); + } absl::optional exponent_symbol; u = ParseProtoArg(op, "exponent", param_map, &exp, &exponent_symbol); @@ -599,13 +713,30 @@ inline Status AsymmetricDepolarizingChannel(const Operation& op, const unsigned int num_qubits, const unsigned int time, NoisyQsimCircuit* ncircuit) { - int q; + unsigned int q = 0; float p_x, p_y, p_z; Status u; - (void)absl::SimpleAtoi(op.qubits(0).id(), &q); + if (op.qubits_size() < 1) { + return Status(absl::StatusCode::kInvalidArgument, + "Channel requires at least 1 qubit."); + } + if (!absl::SimpleAtoi(op.qubits(0).id(), &q)) { + return Status(absl::StatusCode::kInvalidArgument, + "Unparseable qubit index: " + op.qubits(0).id()); + } + if (q >= num_qubits) { + return Status(absl::StatusCode::kInvalidArgument, + "Qubit index out of range: " + std::to_string(q)); + } u = ParseProtoArg(op, "p_x", {}, &p_x); + if (!u.ok()) { + return u; + } u = ParseProtoArg(op, "p_y", {}, &p_y); + if (!u.ok()) { + return u; + } u = ParseProtoArg(op, "p_z", {}, &p_z); if (!u.ok()) { return u; @@ -620,10 +751,21 @@ inline Status DepolarizingChannel(const Operation& op, const unsigned int num_qubits, const unsigned int time, NoisyQsimCircuit* ncircuit) { - int q; + unsigned int q = 0; float p; Status u; - (void)absl::SimpleAtoi(op.qubits(0).id(), &q); + if (op.qubits_size() < 1) { + return Status(absl::StatusCode::kInvalidArgument, + "Channel requires at least 1 qubit."); + } + if (!absl::SimpleAtoi(op.qubits(0).id(), &q)) { + return Status(absl::StatusCode::kInvalidArgument, + "Unparseable qubit index: " + op.qubits(0).id()); + } + if (q >= num_qubits) { + return Status(absl::StatusCode::kInvalidArgument, + "Qubit index out of range: " + std::to_string(q)); + } u = ParseProtoArg(op, "p", {}, &p); if (!u.ok()) { @@ -637,10 +779,21 @@ inline Status DepolarizingChannel(const Operation& op, inline Status GADChannel(const Operation& op, const unsigned int num_qubits, const unsigned int time, NoisyQsimCircuit* ncircuit) { - int q; + unsigned int q = 0; float p, gamma; Status u; - (void)absl::SimpleAtoi(op.qubits(0).id(), &q); + if (op.qubits_size() < 1) { + return Status(absl::StatusCode::kInvalidArgument, + "Channel requires at least 1 qubit."); + } + if (!absl::SimpleAtoi(op.qubits(0).id(), &q)) { + return Status(absl::StatusCode::kInvalidArgument, + "Unparseable qubit index: " + op.qubits(0).id()); + } + if (q >= num_qubits) { + return Status(absl::StatusCode::kInvalidArgument, + "Qubit index out of range: " + std::to_string(q)); + } u = ParseProtoArg(op, "p", {}, &p); if (!u.ok()) { @@ -660,8 +813,19 @@ inline Status GADChannel(const Operation& op, const unsigned int num_qubits, inline Status ResetChannel(const Operation& op, const unsigned int num_qubits, const unsigned int time, NoisyQsimCircuit* ncircuit) { - int q; - (void)absl::SimpleAtoi(op.qubits(0).id(), &q); + unsigned int q = 0; + if (op.qubits_size() < 1) { + return Status(absl::StatusCode::kInvalidArgument, + "Channel requires at least 1 qubit."); + } + if (!absl::SimpleAtoi(op.qubits(0).id(), &q)) { + return Status(absl::StatusCode::kInvalidArgument, + "Unparseable qubit index: " + op.qubits(0).id()); + } + if (q >= num_qubits) { + return Status(absl::StatusCode::kInvalidArgument, + "Qubit index out of range: " + std::to_string(q)); + } auto chan = qsim::Cirq::ResetChannel::Create(time, num_qubits - q - 1); ncircuit->channels.push_back(chan); @@ -672,10 +836,21 @@ inline Status AmplitudeDampingChannel(const Operation& op, const unsigned int num_qubits, const unsigned int time, NoisyQsimCircuit* ncircuit) { - int q; + unsigned int q = 0; float gamma; Status u; - (void)absl::SimpleAtoi(op.qubits(0).id(), &q); + if (op.qubits_size() < 1) { + return Status(absl::StatusCode::kInvalidArgument, + "Channel requires at least 1 qubit."); + } + if (!absl::SimpleAtoi(op.qubits(0).id(), &q)) { + return Status(absl::StatusCode::kInvalidArgument, + "Unparseable qubit index: " + op.qubits(0).id()); + } + if (q >= num_qubits) { + return Status(absl::StatusCode::kInvalidArgument, + "Qubit index out of range: " + std::to_string(q)); + } u = ParseProtoArg(op, "gamma", {}, &gamma); if (!u.ok()) { @@ -691,10 +866,21 @@ inline Status PhaseDampingChannel(const Operation& op, const unsigned int num_qubits, const unsigned int time, NoisyQsimCircuit* ncircuit) { - int q; + unsigned int q = 0; float gamma; Status u; - (void)absl::SimpleAtoi(op.qubits(0).id(), &q); + if (op.qubits_size() < 1) { + return Status(absl::StatusCode::kInvalidArgument, + "Channel requires at least 1 qubit."); + } + if (!absl::SimpleAtoi(op.qubits(0).id(), &q)) { + return Status(absl::StatusCode::kInvalidArgument, + "Unparseable qubit index: " + op.qubits(0).id()); + } + if (q >= num_qubits) { + return Status(absl::StatusCode::kInvalidArgument, + "Qubit index out of range: " + std::to_string(q)); + } u = ParseProtoArg(op, "gamma", {}, &gamma); if (!u.ok()) { @@ -711,10 +897,21 @@ inline Status PhaseFlipChannel(const Operation& op, const unsigned int num_qubits, const unsigned int time, NoisyQsimCircuit* ncircuit) { - int q; + unsigned int q = 0; float p; Status u; - (void)absl::SimpleAtoi(op.qubits(0).id(), &q); + if (op.qubits_size() < 1) { + return Status(absl::StatusCode::kInvalidArgument, + "Channel requires at least 1 qubit."); + } + if (!absl::SimpleAtoi(op.qubits(0).id(), &q)) { + return Status(absl::StatusCode::kInvalidArgument, + "Unparseable qubit index: " + op.qubits(0).id()); + } + if (q >= num_qubits) { + return Status(absl::StatusCode::kInvalidArgument, + "Qubit index out of range: " + std::to_string(q)); + } u = ParseProtoArg(op, "p", {}, &p); if (!u.ok()) { @@ -730,10 +927,21 @@ inline Status PhaseFlipChannel(const Operation& op, inline Status BitFlipChannel(const Operation& op, const unsigned int num_qubits, const unsigned int time, NoisyQsimCircuit* ncircuit) { - int q; + unsigned int q = 0; float p; Status u; - (void)absl::SimpleAtoi(op.qubits(0).id(), &q); + if (op.qubits_size() < 1) { + return Status(absl::StatusCode::kInvalidArgument, + "Channel requires at least 1 qubit."); + } + if (!absl::SimpleAtoi(op.qubits(0).id(), &q)) { + return Status(absl::StatusCode::kInvalidArgument, + "Unparseable qubit index: " + op.qubits(0).id()); + } + if (q >= num_qubits) { + return Status(absl::StatusCode::kInvalidArgument, + "Qubit index out of range: " + std::to_string(q)); + } u = ParseProtoArg(op, "p", {}, &p); if (!u.ok()) { diff --git a/tensorflow_quantum/core/src/circuit_parser_qsim_test.cc b/tensorflow_quantum/core/src/circuit_parser_qsim_test.cc index a158d2ae3..7405dad3a 100644 --- a/tensorflow_quantum/core/src/circuit_parser_qsim_test.cc +++ b/tensorflow_quantum/core/src/circuit_parser_qsim_test.cc @@ -1706,5 +1706,145 @@ TEST(QsimCircuitParserTest, ZBasisCircuitFromPauliTermEmpty) { ASSERT_EQ(test_circuit.gates.size(), 0); } +TEST(QsimCircuitParserTest, InvalidCircuitInputs) { + Program program_proto; + Circuit* circuit_proto = program_proto.mutable_circuit(); + circuit_proto->set_scheduling_strategy(circuit_proto->MOMENT_BY_MOMENT); + Moment* moments_proto = circuit_proto->add_moments(); + + // 1. Single qubit gate HP (HGate) with NO qubits + Operation* op = moments_proto->add_operations(); + op->mutable_gate()->set_id("HP"); + (*op->mutable_args())["exponent"] = MakeArg(1.0); + (*op->mutable_args())["exponent_scalar"] = MakeArg(1.0); + (*op->mutable_args())["global_shift"] = MakeArg(0.0); + (*op->mutable_args())["control_qubits"] = MakeControlArg(""); + (*op->mutable_args())["control_values"] = MakeControlArg(""); + + QsimCircuit test_circuit; + std::vector> fused_circuit; + SymbolMap empty_map; + + // SingleEigenGate requires at least 1 qubit + ASSERT_EQ(QsimCircuitFromProgram(program_proto, empty_map, 1, &test_circuit, &fused_circuit), + tensorflow::Status(absl::StatusCode::kInvalidArgument, + "SingleEigenGate requires at least 1 qubit.")); + + // 2. Clear operations, add a gate with unparseable qubit id + moments_proto->clear_operations(); + op = moments_proto->add_operations(); + op->mutable_gate()->set_id("HP"); + (*op->mutable_args())["exponent"] = MakeArg(1.0); + (*op->mutable_args())["exponent_scalar"] = MakeArg(1.0); + (*op->mutable_args())["global_shift"] = MakeArg(0.0); + (*op->mutable_args())["control_qubits"] = MakeControlArg(""); + (*op->mutable_args())["control_values"] = MakeControlArg(""); + op->add_qubits()->set_id("invalid_index"); + + ASSERT_EQ(QsimCircuitFromProgram(program_proto, empty_map, 1, &test_circuit, &fused_circuit), + tensorflow::Status(absl::StatusCode::kInvalidArgument, + "Unparseable qubit index: invalid_index")); + + // 3. Qubit index out of range (q0 >= num_qubits) + moments_proto->clear_operations(); + op = moments_proto->add_operations(); + op->mutable_gate()->set_id("HP"); + (*op->mutable_args())["exponent"] = MakeArg(1.0); + (*op->mutable_args())["exponent_scalar"] = MakeArg(1.0); + (*op->mutable_args())["global_shift"] = MakeArg(0.0); + (*op->mutable_args())["control_qubits"] = MakeControlArg(""); + (*op->mutable_args())["control_values"] = MakeControlArg(""); + op->add_qubits()->set_id("5"); // num_qubits is 2, so 5 is out of range + + ASSERT_EQ(QsimCircuitFromProgram(program_proto, empty_map, 2, &test_circuit, &fused_circuit), + tensorflow::Status(absl::StatusCode::kInvalidArgument, + "Qubit index out of range: 5")); + + // 4. Two qubit gate with only 1 qubit + moments_proto->clear_operations(); + op = moments_proto->add_operations(); + op->mutable_gate()->set_id("CZP"); + (*op->mutable_args())["exponent"] = MakeArg(1.0); + (*op->mutable_args())["exponent_scalar"] = MakeArg(1.0); + (*op->mutable_args())["global_shift"] = MakeArg(0.0); + (*op->mutable_args())["control_qubits"] = MakeControlArg(""); + (*op->mutable_args())["control_values"] = MakeControlArg(""); + op->add_qubits()->set_id("0"); + + ASSERT_EQ(QsimCircuitFromProgram(program_proto, empty_map, 2, &test_circuit, &fused_circuit), + tensorflow::Status(absl::StatusCode::kInvalidArgument, + "TwoEigenGate requires at least 2 qubits.")); + + // 5. Unparseable control qubit + moments_proto->clear_operations(); + op = moments_proto->add_operations(); + op->mutable_gate()->set_id("HP"); + (*op->mutable_args())["exponent"] = MakeArg(1.0); + (*op->mutable_args())["exponent_scalar"] = MakeArg(1.0); + (*op->mutable_args())["global_shift"] = MakeArg(0.0); + (*op->mutable_args())["control_qubits"] = MakeControlArg("bad"); + (*op->mutable_args())["control_values"] = MakeControlArg("0"); + op->add_qubits()->set_id("0"); + + ASSERT_EQ(QsimCircuitFromProgram(program_proto, empty_map, 2, &test_circuit, &fused_circuit), + tensorflow::Status(absl::StatusCode::kInvalidArgument, + "Unparseable control qubit index: bad")); + + // 6. Control qubit out of range + moments_proto->clear_operations(); + op = moments_proto->add_operations(); + op->mutable_gate()->set_id("HP"); + (*op->mutable_args())["exponent"] = MakeArg(1.0); + (*op->mutable_args())["exponent_scalar"] = MakeArg(1.0); + (*op->mutable_args())["global_shift"] = MakeArg(0.0); + (*op->mutable_args())["control_qubits"] = MakeControlArg("4"); // num_qubits is 2 + (*op->mutable_args())["control_values"] = MakeControlArg("0"); + op->add_qubits()->set_id("0"); + + ASSERT_EQ(QsimCircuitFromProgram(program_proto, empty_map, 2, &test_circuit, &fused_circuit), + tensorflow::Status(absl::StatusCode::kInvalidArgument, + "Control qubit index out of range: 4")); +} + +TEST(QsimCircuitParserTest, InvalidChannelInputs) { + Program program_proto; + Circuit* circuit_proto = program_proto.mutable_circuit(); + circuit_proto->set_scheduling_strategy(circuit_proto->MOMENT_BY_MOMENT); + Moment* moments_proto = circuit_proto->add_moments(); + + // 1. Bit flip channel with empty qubits list + Operation* op = moments_proto->add_operations(); + op->mutable_gate()->set_id("BF"); + (*op->mutable_args())["p"] = MakeArg(0.1); + + NoisyQsimCircuit test_circuit; + + ASSERT_EQ(NoisyQsimCircuitFromProgram(program_proto, {}, 2, false, &test_circuit), + tensorflow::Status(absl::StatusCode::kInvalidArgument, + "Channel requires at least 1 qubit.")); + + // 2. Bit flip channel with unparseable qubit index + moments_proto->clear_operations(); + op = moments_proto->add_operations(); + op->mutable_gate()->set_id("BF"); + (*op->mutable_args())["p"] = MakeArg(0.1); + op->add_qubits()->set_id("xyz"); + + ASSERT_EQ(NoisyQsimCircuitFromProgram(program_proto, {}, 2, false, &test_circuit), + tensorflow::Status(absl::StatusCode::kInvalidArgument, + "Unparseable qubit index: xyz")); + + // 3. Bit flip channel with qubit index out of bounds + moments_proto->clear_operations(); + op = moments_proto->add_operations(); + op->mutable_gate()->set_id("BF"); + (*op->mutable_args())["p"] = MakeArg(0.1); + op->add_qubits()->set_id("3"); // num_qubits = 2 + + ASSERT_EQ(NoisyQsimCircuitFromProgram(program_proto, {}, 2, false, &test_circuit), + tensorflow::Status(absl::StatusCode::kInvalidArgument, + "Qubit index out of range: 3")); +} + } // namespace } // namespace tfq From 47c077d6f26ebb6d5a46ae846a67c3dc7ed7d249 Mon Sep 17 00:00:00 2001 From: mhucka Date: Wed, 24 Jun 2026 22:59:36 +0000 Subject: [PATCH 2/7] Run format_all.sh --- .../core/src/circuit_parser_qsim.cc | 3 +- .../core/src/circuit_parser_qsim_test.cc | 46 +++++++++++-------- 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/tensorflow_quantum/core/src/circuit_parser_qsim.cc b/tensorflow_quantum/core/src/circuit_parser_qsim.cc index 260de5332..c1cbcda96 100644 --- a/tensorflow_quantum/core/src/circuit_parser_qsim.cc +++ b/tensorflow_quantum/core/src/circuit_parser_qsim.cc @@ -87,7 +87,8 @@ inline Status ParseProtoControls(const Operation& op, std::vector* control_values) { const auto control_qubits_it = op.args().find("control_qubits"); const auto control_values_it = op.args().find("control_values"); - if (control_qubits_it == op.args().end() || control_values_it == op.args().end()) { + if (control_qubits_it == op.args().end() || + control_values_it == op.args().end()) { return ::tensorflow::Status(); } diff --git a/tensorflow_quantum/core/src/circuit_parser_qsim_test.cc b/tensorflow_quantum/core/src/circuit_parser_qsim_test.cc index 7405dad3a..e7b03e2b5 100644 --- a/tensorflow_quantum/core/src/circuit_parser_qsim_test.cc +++ b/tensorflow_quantum/core/src/circuit_parser_qsim_test.cc @@ -1726,7 +1726,8 @@ TEST(QsimCircuitParserTest, InvalidCircuitInputs) { SymbolMap empty_map; // SingleEigenGate requires at least 1 qubit - ASSERT_EQ(QsimCircuitFromProgram(program_proto, empty_map, 1, &test_circuit, &fused_circuit), + ASSERT_EQ(QsimCircuitFromProgram(program_proto, empty_map, 1, &test_circuit, + &fused_circuit), tensorflow::Status(absl::StatusCode::kInvalidArgument, "SingleEigenGate requires at least 1 qubit.")); @@ -1741,7 +1742,8 @@ TEST(QsimCircuitParserTest, InvalidCircuitInputs) { (*op->mutable_args())["control_values"] = MakeControlArg(""); op->add_qubits()->set_id("invalid_index"); - ASSERT_EQ(QsimCircuitFromProgram(program_proto, empty_map, 1, &test_circuit, &fused_circuit), + ASSERT_EQ(QsimCircuitFromProgram(program_proto, empty_map, 1, &test_circuit, + &fused_circuit), tensorflow::Status(absl::StatusCode::kInvalidArgument, "Unparseable qubit index: invalid_index")); @@ -1754,9 +1756,10 @@ TEST(QsimCircuitParserTest, InvalidCircuitInputs) { (*op->mutable_args())["global_shift"] = MakeArg(0.0); (*op->mutable_args())["control_qubits"] = MakeControlArg(""); (*op->mutable_args())["control_values"] = MakeControlArg(""); - op->add_qubits()->set_id("5"); // num_qubits is 2, so 5 is out of range + op->add_qubits()->set_id("5"); // num_qubits is 2, so 5 is out of range - ASSERT_EQ(QsimCircuitFromProgram(program_proto, empty_map, 2, &test_circuit, &fused_circuit), + ASSERT_EQ(QsimCircuitFromProgram(program_proto, empty_map, 2, &test_circuit, + &fused_circuit), tensorflow::Status(absl::StatusCode::kInvalidArgument, "Qubit index out of range: 5")); @@ -1771,7 +1774,8 @@ TEST(QsimCircuitParserTest, InvalidCircuitInputs) { (*op->mutable_args())["control_values"] = MakeControlArg(""); op->add_qubits()->set_id("0"); - ASSERT_EQ(QsimCircuitFromProgram(program_proto, empty_map, 2, &test_circuit, &fused_circuit), + ASSERT_EQ(QsimCircuitFromProgram(program_proto, empty_map, 2, &test_circuit, + &fused_circuit), tensorflow::Status(absl::StatusCode::kInvalidArgument, "TwoEigenGate requires at least 2 qubits.")); @@ -1786,7 +1790,8 @@ TEST(QsimCircuitParserTest, InvalidCircuitInputs) { (*op->mutable_args())["control_values"] = MakeControlArg("0"); op->add_qubits()->set_id("0"); - ASSERT_EQ(QsimCircuitFromProgram(program_proto, empty_map, 2, &test_circuit, &fused_circuit), + ASSERT_EQ(QsimCircuitFromProgram(program_proto, empty_map, 2, &test_circuit, + &fused_circuit), tensorflow::Status(absl::StatusCode::kInvalidArgument, "Unparseable control qubit index: bad")); @@ -1797,11 +1802,13 @@ TEST(QsimCircuitParserTest, InvalidCircuitInputs) { (*op->mutable_args())["exponent"] = MakeArg(1.0); (*op->mutable_args())["exponent_scalar"] = MakeArg(1.0); (*op->mutable_args())["global_shift"] = MakeArg(0.0); - (*op->mutable_args())["control_qubits"] = MakeControlArg("4"); // num_qubits is 2 + (*op->mutable_args())["control_qubits"] = + MakeControlArg("4"); // num_qubits is 2 (*op->mutable_args())["control_values"] = MakeControlArg("0"); op->add_qubits()->set_id("0"); - ASSERT_EQ(QsimCircuitFromProgram(program_proto, empty_map, 2, &test_circuit, &fused_circuit), + ASSERT_EQ(QsimCircuitFromProgram(program_proto, empty_map, 2, &test_circuit, + &fused_circuit), tensorflow::Status(absl::StatusCode::kInvalidArgument, "Control qubit index out of range: 4")); } @@ -1819,9 +1826,10 @@ TEST(QsimCircuitParserTest, InvalidChannelInputs) { NoisyQsimCircuit test_circuit; - ASSERT_EQ(NoisyQsimCircuitFromProgram(program_proto, {}, 2, false, &test_circuit), - tensorflow::Status(absl::StatusCode::kInvalidArgument, - "Channel requires at least 1 qubit.")); + ASSERT_EQ( + NoisyQsimCircuitFromProgram(program_proto, {}, 2, false, &test_circuit), + tensorflow::Status(absl::StatusCode::kInvalidArgument, + "Channel requires at least 1 qubit.")); // 2. Bit flip channel with unparseable qubit index moments_proto->clear_operations(); @@ -1830,20 +1838,22 @@ TEST(QsimCircuitParserTest, InvalidChannelInputs) { (*op->mutable_args())["p"] = MakeArg(0.1); op->add_qubits()->set_id("xyz"); - ASSERT_EQ(NoisyQsimCircuitFromProgram(program_proto, {}, 2, false, &test_circuit), - tensorflow::Status(absl::StatusCode::kInvalidArgument, - "Unparseable qubit index: xyz")); + ASSERT_EQ( + NoisyQsimCircuitFromProgram(program_proto, {}, 2, false, &test_circuit), + tensorflow::Status(absl::StatusCode::kInvalidArgument, + "Unparseable qubit index: xyz")); // 3. Bit flip channel with qubit index out of bounds moments_proto->clear_operations(); op = moments_proto->add_operations(); op->mutable_gate()->set_id("BF"); (*op->mutable_args())["p"] = MakeArg(0.1); - op->add_qubits()->set_id("3"); // num_qubits = 2 + op->add_qubits()->set_id("3"); // num_qubits = 2 - ASSERT_EQ(NoisyQsimCircuitFromProgram(program_proto, {}, 2, false, &test_circuit), - tensorflow::Status(absl::StatusCode::kInvalidArgument, - "Qubit index out of range: 3")); + ASSERT_EQ( + NoisyQsimCircuitFromProgram(program_proto, {}, 2, false, &test_circuit), + tensorflow::Status(absl::StatusCode::kInvalidArgument, + "Qubit index out of range: 3")); } } // namespace From 96b685922fbcacf76623a350bb787f3185b92518 Mon Sep 17 00:00:00 2001 From: Michael Hucka Date: Wed, 24 Jun 2026 16:04:13 -0700 Subject: [PATCH 3/7] Update tensorflow_quantum/core/src/circuit_parser_qsim.cc Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- tensorflow_quantum/core/src/circuit_parser_qsim.cc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tensorflow_quantum/core/src/circuit_parser_qsim.cc b/tensorflow_quantum/core/src/circuit_parser_qsim.cc index c1cbcda96..5e715eddc 100644 --- a/tensorflow_quantum/core/src/circuit_parser_qsim.cc +++ b/tensorflow_quantum/core/src/circuit_parser_qsim.cc @@ -87,10 +87,15 @@ inline Status ParseProtoControls(const Operation& op, std::vector* control_values) { const auto control_qubits_it = op.args().find("control_qubits"); const auto control_values_it = op.args().find("control_values"); - if (control_qubits_it == op.args().end() || + if (control_qubits_it == op.args().end() && control_values_it == op.args().end()) { return ::tensorflow::Status(); } + if (control_qubits_it == op.args().end() || + control_values_it == op.args().end()) { + return Status(absl::StatusCode::kInvalidArgument, + "Both control_qubits and control_values must be specified if either is present."); + } absl::string_view control_str = control_qubits_it->second.arg_value().string_value(); From b52b472e13af781e88804891eaad2f3233eb8d40 Mon Sep 17 00:00:00 2001 From: mhucka Date: Wed, 24 Jun 2026 23:10:01 +0000 Subject: [PATCH 4/7] Address review comments by Gemini Code Assist --- .../core/src/circuit_parser_qsim.cc | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/tensorflow_quantum/core/src/circuit_parser_qsim.cc b/tensorflow_quantum/core/src/circuit_parser_qsim.cc index 5e715eddc..e0a765617 100644 --- a/tensorflow_quantum/core/src/circuit_parser_qsim.cc +++ b/tensorflow_quantum/core/src/circuit_parser_qsim.cc @@ -94,7 +94,8 @@ inline Status ParseProtoControls(const Operation& op, if (control_qubits_it == op.args().end() || control_values_it == op.args().end()) { return Status(absl::StatusCode::kInvalidArgument, - "Both control_qubits and control_values must be specified if either is present."); + "Both control_qubits and control_values must be specified if " + "either is present."); } absl::string_view control_str = @@ -228,6 +229,11 @@ inline Status TwoConstantGate( return Status(absl::StatusCode::kInvalidArgument, "Qubit index out of range: " + std::to_string(q1)); } + if (q0 == q1) { + return Status( + absl::StatusCode::kInvalidArgument, + "TwoConstantGate requires distinct qubits: " + std::to_string(q0)); + } auto gate = create_f(time, num_qubits - q0 - 1, num_qubits - q1 - 1); Status s = OptionalInsertControls(op, num_qubits, &gate); if (!s.ok()) { @@ -333,6 +339,11 @@ inline Status TwoEigenGate( return Status(absl::StatusCode::kInvalidArgument, "Qubit index out of range: " + std::to_string(q1)); } + if (q0 == q1) { + return Status( + absl::StatusCode::kInvalidArgument, + "TwoConstantGate requires distinct qubits: " + std::to_string(q0)); + } absl::optional exponent_symbol; u = ParseProtoArg(op, "exponent", param_map, &exp, &exponent_symbol); @@ -560,6 +571,11 @@ inline Status FsimGate(const Operation& op, const SymbolMap& param_map, return Status(absl::StatusCode::kInvalidArgument, "Qubit index out of range: " + std::to_string(q1)); } + if (q0 == q1) { + return Status( + absl::StatusCode::kInvalidArgument, + "TwoConstantGate requires distinct qubits: " + std::to_string(q0)); + } absl::optional theta_symbol; u = ParseProtoArg(op, "theta", param_map, &theta, &theta_symbol); @@ -634,6 +650,11 @@ inline Status PhasedISwapGate(const Operation& op, const SymbolMap& param_map, return Status(absl::StatusCode::kInvalidArgument, "Qubit index out of range: " + std::to_string(q1)); } + if (q0 == q1) { + return Status( + absl::StatusCode::kInvalidArgument, + "TwoConstantGate requires distinct qubits: " + std::to_string(q0)); + } absl::optional exponent_symbol; u = ParseProtoArg(op, "exponent", param_map, &exp, &exponent_symbol); From 5d949b301ba8af269f7382aeaae75d420360b098 Mon Sep 17 00:00:00 2001 From: Michael Hucka Date: Wed, 24 Jun 2026 16:29:30 -0700 Subject: [PATCH 5/7] Update tensorflow_quantum/core/src/circuit_parser_qsim.cc Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- tensorflow_quantum/core/src/circuit_parser_qsim.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tensorflow_quantum/core/src/circuit_parser_qsim.cc b/tensorflow_quantum/core/src/circuit_parser_qsim.cc index e0a765617..3436f1c2b 100644 --- a/tensorflow_quantum/core/src/circuit_parser_qsim.cc +++ b/tensorflow_quantum/core/src/circuit_parser_qsim.cc @@ -342,7 +342,7 @@ inline Status TwoEigenGate( if (q0 == q1) { return Status( absl::StatusCode::kInvalidArgument, - "TwoConstantGate requires distinct qubits: " + std::to_string(q0)); + "TwoEigenGate requires distinct qubits: " + std::to_string(q0)); } absl::optional exponent_symbol; From 6ee4ebd31ae30a99b935af0ab03af16b8f180c54 Mon Sep 17 00:00:00 2001 From: Michael Hucka Date: Wed, 24 Jun 2026 16:29:38 -0700 Subject: [PATCH 6/7] Update tensorflow_quantum/core/src/circuit_parser_qsim.cc Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- tensorflow_quantum/core/src/circuit_parser_qsim.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tensorflow_quantum/core/src/circuit_parser_qsim.cc b/tensorflow_quantum/core/src/circuit_parser_qsim.cc index 3436f1c2b..b5dd47703 100644 --- a/tensorflow_quantum/core/src/circuit_parser_qsim.cc +++ b/tensorflow_quantum/core/src/circuit_parser_qsim.cc @@ -653,7 +653,7 @@ inline Status PhasedISwapGate(const Operation& op, const SymbolMap& param_map, if (q0 == q1) { return Status( absl::StatusCode::kInvalidArgument, - "TwoConstantGate requires distinct qubits: " + std::to_string(q0)); + "PhasedISwapGate requires distinct qubits: " + std::to_string(q0)); } absl::optional exponent_symbol; From c786281c3a417070ef2b2a2d9d9134d383cf46e1 Mon Sep 17 00:00:00 2001 From: Michael Hucka Date: Wed, 24 Jun 2026 16:29:47 -0700 Subject: [PATCH 7/7] Update tensorflow_quantum/core/src/circuit_parser_qsim.cc Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- tensorflow_quantum/core/src/circuit_parser_qsim.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tensorflow_quantum/core/src/circuit_parser_qsim.cc b/tensorflow_quantum/core/src/circuit_parser_qsim.cc index b5dd47703..d95c64f66 100644 --- a/tensorflow_quantum/core/src/circuit_parser_qsim.cc +++ b/tensorflow_quantum/core/src/circuit_parser_qsim.cc @@ -574,7 +574,7 @@ inline Status FsimGate(const Operation& op, const SymbolMap& param_map, if (q0 == q1) { return Status( absl::StatusCode::kInvalidArgument, - "TwoConstantGate requires distinct qubits: " + std::to_string(q0)); + "FsimGate requires distinct qubits: " + std::to_string(q0)); } absl::optional theta_symbol;