Validate parameters of each gate in a circuit (Qiskit#1834)

This commit adds checks of arguments (A number of qubits, clbits,
and parameters) for each gate to prevent from unexpected memory access
when a user defines wrong custom gate with a number of a standard gate.

* validate parameters of each gate in a circuit
* fix lint error
* fix lint error

qiskit_aer/backends/aer_compiler.py

@@ -544,62 +544,13 @@ def _assemble_op(aer_circ, inst, qubit_indices, clbit_indices, is_conditional, c
                 copied = True
             params[i] = 0.0
 
+    # fmt: off
     if name in {
-        "ccx",
-        "ccz",
-        "cp",
-        "cswap",
-        "csx",
-        "cx",
-        "cy",
-        "cz",
-        "delay",
-        "ecr",
-        "h",
-        "id",
-        "mcp",
-        "mcphase",
-        "mcr",
-        "mcrx",
-        "mcry",
-        "mcrz",
-        "mcswap",
-        "mcsx",
-        "mcu",
-        "mcu1",
-        "mcu2",
-        "mcu3",
-        "mcx",
-        "mcx_gray",
-        "mcy",
-        "mcz",
-        "p",
-        "r",
-        "rx",
-        "rxx",
-        "ry",
-        "ryy",
-        "rz",
-        "rzx",
-        "rzz",
-        "s",
-        "sdg",
-        "swap",
-        "sx",
-        "sxdg",
-        "t",
-        "tdg",
-        "u",
-        "x",
-        "y",
-        "z",
-        "u1",
-        "u2",
-        "u3",
-        "cu",
-        "cu1",
-        "cu2",
-        "cu3",
+        "ccx", "ccz", "cp", "cswap", "csx", "cx", "cy", "cz", "delay", "ecr", "h",
+        "id", "mcp", "mcphase", "mcr", "mcrx", "mcry", "mcrz", "mcswap", "mcsx",
+        "mcu", "mcu1", "mcu2", "mcu3", "mcx", "mcx_gray", "mcy", "mcz", "p", "r",
+        "rx", "rxx", "ry", "ryy", "rz", "rzx", "rzz", "s", "sdg", "swap", "sx", "sxdg",
+        "t", "tdg", "u", "x", "y", "z", "u1", "u2", "u3", "cu", "cu1", "cu2", "cu3",
     }:
         aer_circ.gate(name, qubits, params, [], conditional_reg, label if label else name)
     elif name == "measure":

releasenotes/notes/check_param_length-eb69cd92825bbca4.yaml

@@ -0,0 +1,7 @@
+---
+fixes:
+  - |
+    Previously, parameters for gates are not validate in C++. If parameters are shorter than
+    expected (due to custom gate), segmentaion faults are thrown. This commit adds checks
+    whether parameter lenght is expceted. This commit will fix issues reported in #1612.
+    https://github.com/Qiskit/qiskit-aer/issues/1612

src/framework/circuit.hpp

@@ -130,6 +130,7 @@ class Circuit {
             const int_t cond_regidx = -1, const std::string label = "") {
     ops.push_back(Operations::make_gate(name, qubits, params, string_params,
                                         cond_regidx, label));
+    check_gate_params(ops.back());
   }
 
   void diagonal(const reg_t &qubits, const cvector_t &vec,

src/framework/operations.hpp

@@ -349,32 +349,72 @@ inline void check_empty_name(const Op &op) {
 // Raise an exception if qubits list is empty
 inline void check_empty_qubits(const Op &op) {
   if (op.qubits.empty())
-    throw std::invalid_argument(R"(Invalid qobj ")" + op.name +
-                                R"(" instruction ("qubits" is empty).)");
+    throw std::invalid_argument(R"(Invalid operation ")" + op.name +
+                                R"(" ("qubits" is empty).)");
 }
 
 // Raise an exception if params is empty
 inline void check_empty_params(const Op &op) {
   if (op.params.empty())
-    throw std::invalid_argument(R"(Invalid qobj ")" + op.name +
-                                R"(" instruction ("params" is empty).)");
+    throw std::invalid_argument(R"(Invalid operation ")" + op.name +
+                                R"(" ("params" is empty).)");
+}
+
+// Raise an exception if qubits is more than expected
+inline void check_length_qubits(const Op &op, const size_t size) {
+  if (op.qubits.size() < size)
+    throw std::invalid_argument(R"(Invalid operation ")" + op.name +
+                                R"(" ("qubits" is incorrect length).)");
 }
 
 // Raise an exception if params is empty
 inline void check_length_params(const Op &op, const size_t size) {
-  if (op.params.size() != size)
-    throw std::invalid_argument(
-        R"(Invalid qobj ")" + op.name +
-        R"(" instruction ("params" is incorrect length).)");
+  if (op.params.size() < size)
+    throw std::invalid_argument(R"(Invalid operation ")" + op.name +
+                                R"(" ("params" is incorrect length).)");
 }
 
 // Raise an exception if qubits list contains duplications
 inline void check_duplicate_qubits(const Op &op) {
   auto cpy = op.qubits;
   std::unique(cpy.begin(), cpy.end());
   if (cpy != op.qubits)
-    throw std::invalid_argument(R"(Invalid qobj ")" + op.name +
-                                R"(" instruction ("qubits" are not unique).)");
+    throw std::invalid_argument(R"(Invalid operation ")" + op.name +
+                                R"(" ("qubits" are not unique).)");
+}
+
+inline void check_gate_params(const Op &op) {
+  const stringmap_t<std::tuple<int_t, int_t>> param_tables(
+      {{"u1", {1, 1}},       {"u2", {1, 2}},     {"u3", {1, 3}},
+       {"u", {1, 3}},        {"U", {1, 3}},      {"CX", {2, 0}},
+       {"cx", {2, 0}},       {"cz", {2, 0}},     {"cy", {2, 0}},
+       {"cp", {2, 1}},       {"cu1", {2, 1}},    {"cu2", {2, 2}},
+       {"cu3", {2, 3}},      {"swap", {2, 0}},   {"id", {0, 0}},
+       {"p", {1, 1}},        {"x", {1, 0}},      {"y", {1, 0}},
+       {"z", {1, 0}},        {"h", {1, 0}},      {"s", {1, 0}},
+       {"sdg", {1, 0}},      {"t", {1, 0}},      {"tdg", {1, 0}},
+       {"r", {1, 2}},        {"rx", {1, 1}},     {"ry", {1, 1}},
+       {"rz", {1, 1}},       {"rxx", {2, 1}},    {"ryy", {2, 1}},
+       {"rzz", {2, 1}},      {"rzx", {2, 1}},    {"ccx", {3, 0}},
+       {"cswap", {3, 0}},    {"mcx", {1, 0}},    {"mcy", {1, 0}},
+       {"mcz", {1, 0}},      {"mcu1", {1, 1}},   {"mcu2", {1, 2}},
+       {"mcu3", {1, 3}},     {"mcswap", {2, 0}}, {"mcphase", {1, 1}},
+       {"mcr", {1, 1}},      {"mcrx", {1, 1}},   {"mcry", {1, 1}},
+       {"mcrz", {1, 1}},     {"sx", {1, 0}},     {"sxdg", {1, 0}},
+       {"csx", {2, 0}},      {"mcsx", {1, 0}},   {"csxdg", {2, 0}},
+       {"mcsxdg", {1, 0}},   {"delay", {1, 0}},  {"pauli", {1, 0}},
+       {"mcx_gray", {1, 0}}, {"cu", {2, 4}},     {"mcu", {1, 4}},
+       {"mcp", {1, 1}},      {"ecr", {2, 0}}});
+
+  auto it = param_tables.find(op.name);
+  if (it == param_tables.end()) {
+    std::stringstream msg;
+    msg << "Invalid gate name :\"" << op.name << "\"." << std::endl;
+    throw std::invalid_argument(msg.str());
+  } else {
+    check_length_qubits(op, std::get<0>(it->second));
+    check_length_params(op, std::get<1>(it->second));
+  }
 }
 
 //------------------------------------------------------------------------------
@@ -1155,12 +1195,8 @@ Op input_to_op_gate(const inputdata_t &input) {
   check_empty_name(op);
   check_empty_qubits(op);
   check_duplicate_qubits(op);
-  if (op.name == "u1")
-    check_length_params(op, 1);
-  else if (op.name == "u2")
-    check_length_params(op, 2);
-  else if (op.name == "u3")
-    check_length_params(op, 3);
+  check_gate_params(op);
+
   return op;
 }
 
@@ -1586,8 +1622,8 @@ Op input_to_op_save_expval(const inputdata_t &input, bool variance) {
   }
 
   // Check edge case of all coefficients being empty
-  // In this case the operator had all coefficients zero, or sufficiently close
-  // to zero that they were all truncated.
+  // In this case the operator had all coefficients zero, or sufficiently
+  // close to zero that they were all truncated.
   if (op.expval_params.empty()) {
     std::string pauli(op.qubits.size(), 'I');
     op.expval_params.emplace_back(pauli, 0., 0.);

src/simulators/statevector/statevector_state.hpp

@@ -70,7 +70,7 @@ const Operations::OpSet StateOpSet(
      "sdg",  "t",     "tdg",    "r",     "rx",    "ry",       "rz",
      "rxx",  "ryy",   "rzz",    "rzx",   "ccx",   "cswap",    "mcx",
      "mcy",  "mcz",   "mcu1",   "mcu2",  "mcu3",  "mcswap",   "mcphase",
-     "mcr",  "mcrx",  "mcry",   "mcry",  "sx",    "sxdg",     "csx",
+     "mcr",  "mcrx",  "mcry",   "mcrz",  "sx",    "sxdg",     "csx",
      "mcsx", "csxdg", "mcsxdg", "delay", "pauli", "mcx_gray", "cu",
      "mcu",  "mcp",   "ecr"});
 

test/terra/backends/aer_simulator/test_circuit.py

@@ -16,7 +16,8 @@
 from ddt import ddt
 import numpy as np
 from qiskit import ClassicalRegister, QuantumCircuit, QuantumRegister, assemble
-from qiskit.circuit import CircuitInstruction
+from qiskit.circuit.gate import Gate
+from qiskit.circuit.library.standard_gates import HGate
 from test.terra.reference import ref_algorithms
 
 from test.terra.backends.simulator_test_case import SimulatorTestCase, supported_methods
@@ -237,3 +238,28 @@ def test_floating_shots(self):
             shots = int(shots)
             self.assertSuccess(result)
             self.assertEqual(sum([result.get_counts()[key] for key in result.get_counts()]), shots)
+
+    def test_invalid_parameters(self):
+        """Test gates with invalid parameter length."""
+
+        backend = self.backend()
+
+        class Custom(Gate):
+            def __init__(self, label=None):
+                super().__init__("p", 1, [], label=label)
+
+            def _define(self):
+                q = QuantumRegister(1, "q")
+                qc = QuantumCircuit(q, name=self.name)
+                qc._append(HGate(), [q[0]], [])
+                self.definition = qc
+
+        qc = QuantumCircuit(1)
+        qc.append(Custom(), [0])
+        qc.measure_all()
+
+        try:
+            backend.run(qc).result()
+            self.fail("do not reach here")
+        except Exception as e:
+            self.assertTrue('"params" is incorrect length' in repr(e))

test/terra/backends/aer_simulator/test_standard_gates.py

@@ -20,59 +20,14 @@
 from qiskit import transpile
 import qiskit.quantum_info as qi
 
+# fmt: off
 from qiskit.circuit.library.standard_gates import (
-    CXGate,
-    CYGate,
-    CZGate,
-    DCXGate,
-    HGate,
-    IGate,
-    SGate,
-    SXGate,
-    SXdgGate,
-    SdgGate,
-    SwapGate,
-    XGate,
-    YGate,
-    ZGate,
-    TGate,
-    TdgGate,
-    iSwapGate,
-    C3XGate,
-    C4XGate,
-    CCXGate,
-    CHGate,
-    CSXGate,
-    CSwapGate,
-    CPhaseGate,
-    CRXGate,
-    CRYGate,
-    CRZGate,
-    CUGate,
-    CU1Gate,
-    CU3Gate,
-    CUGate,
-    PhaseGate,
-    RC3XGate,
-    RCCXGate,
-    RGate,
-    RXGate,
-    RXXGate,
-    RYGate,
-    RYYGate,
-    RZGate,
-    RZXGate,
-    RZZGate,
-    UGate,
-    U1Gate,
-    U2Gate,
-    U3Gate,
-    UGate,
-    MCXGate,
-    MCPhaseGate,
-    MCXGrayCode,
-    ECRGate,
-)
+    CXGate, CYGate, CZGate, DCXGate, HGate, IGate, SGate, SXGate, SXdgGate,
+    SdgGate, SwapGate, XGate, YGate, ZGate, TGate, TdgGate, iSwapGate, C3XGate,
+    C4XGate, CCXGate, CHGate, CSXGate, CSwapGate, CPhaseGate, CRXGate, CRYGate,
+    CRZGate, CUGate, CU1Gate, CU3Gate, CUGate, PhaseGate, RC3XGate, RCCXGate, RGate,
+    RXGate, RXXGate, RYGate, RYYGate, RZGate, RZXGate, RZZGate, UGate, U1Gate, U2Gate,
+    U3Gate, UGate, MCXGate, MCPhaseGate, MCXGrayCode, ECRGate)
 
 
 CLIFFORD_GATES = [