fix: harden ZK verifiers and crypto primitives

src/cbmpc/crypto/base_bn.cpp

@@ -503,6 +503,8 @@ std::vector<bn_t> bn_t::vector_from_bin(mem_t mem, int n, int size, const mod_t&
 
 bn_t bn_t::from_bin_bitlen(mem_t mem, int bits) {  // static
   cb_assert(mem.size == coinbase::bits_to_bytes(bits));
+  // Handle the 0-bit / empty-input case without indexing into `mem`.
+  if (mem.size == 0) return from_bin(mem);
   int unused_bits = bytes_to_bits(mem.size) - bits;
   byte_t mask = 0xff >> unused_bits;
   if (mem[0] == (mem[0] & mask)) return from_bin(mem);

src/cbmpc/crypto/base_ecc_secp256k1.cpp

@@ -117,13 +117,22 @@ void ecurve_secp256k1_t::add(const ecc_point_t& P1, const ecc_point_t& P2, ecc_p
 // This function does not work for some special cases, like when a or b is infinity, or a and b have the same z
 // coordinate. When points are random, the probability of these cases is negligible.
 static void secp256k1_gej_add_const(secp256k1_gej* r, const secp256k1_gej* a, const secp256k1_gej* b) {
-  cb_assert(!a->infinity);
-  cb_assert(!b->infinity);
-
   secp256k1_fe z22, z12, u1, u2, s1, s2, h, i, h2, h3, t;
   SECP256K1_GEJ_VERIFY(a);
   SECP256K1_GEJ_VERIFY(b);
 
+  const bool vartime = is_vartime_scope();
+  if (!vartime) {
+    cb_assert(!a->infinity);
+    cb_assert(!b->infinity);
+  } else {
+    // In verifier code paths we intentionally allow variable-time operations for robustness.
+    if (a->infinity || b->infinity) {
+      secp256k1_gej_add_var(r, a, b, nullptr);
+      return;
+    }
+  }
+
   secp256k1_fe_sqr(&z22, &b->z);
   secp256k1_fe_sqr(&z12, &a->z);
   secp256k1_fe_mul(&u1, &a->x, &z22);
@@ -137,8 +146,16 @@ static void secp256k1_gej_add_const(secp256k1_gej* r, const secp256k1_gej* a, co
   secp256k1_fe_negate(&i, &s2, 1);
   secp256k1_fe_add(&i, &s1);
 
-  cb_assert(!secp256k1_fe_normalizes_to_zero(&h));
-  cb_assert(!secp256k1_fe_normalizes_to_zero(&i));
+  if (!vartime) {
+    cb_assert(!secp256k1_fe_normalizes_to_zero(&h));
+    cb_assert(!secp256k1_fe_normalizes_to_zero(&i));
+  } else {
+    // In verifier code paths we can handle degenerate inputs (rare) via the generic addition.
+    if (secp256k1_fe_normalizes_to_zero(&h) || secp256k1_fe_normalizes_to_zero(&i)) {
+      secp256k1_gej_add_var(r, a, b, nullptr);
+      return;
+    }
+  }
 
   r->infinity = 0;
   secp256k1_fe_mul(&t, &h, &b->z);
@@ -209,6 +226,14 @@ void ecurve_secp256k1_t::mul_add(const bn_t& n, const ecc_point_t& P, const bn_t
   secp256k1_scalar_set_b32(&scalar_n, bin_n.data(), nullptr);
   secp256k1_scalar_set_b32(&scalar_m, bin_m.data(), nullptr);
 
+  if (is_vartime_scope()) {
+    // Verifier code paths allow variable-time operations and require robustness for all inputs.
+    secp256k1_ecmult((secp256k1_gej*)R.secp256k1, (const secp256k1_gej*)P.secp256k1, &scalar_m, &scalar_n);
+    secp256k1_scalar_clear(&scalar_m);
+    secp256k1_scalar_clear(&scalar_n);
+    return;
+  }
+
   secp256k1_gej Rn;
   secp256k1_ecmult_gen(&secp256k1_ecmult_gen_ctx, &Rn, &scalar_n);
 

src/cbmpc/crypto/base_mod.cpp

@@ -24,7 +24,7 @@ void mod_t::convert(coinbase::converter_t& converter) {
   converter.convert(m);
   if (!converter.is_write()) {
     if (converter.is_error()) return;
-    if (m <= 0) {
+    if (m <= 1) {
       converter.set_error();
       return;
     }

src/cbmpc/zk/fischlin.h

@@ -43,11 +43,11 @@ struct fischlin_params_t {
   int rho, b, t;
 
   int e_max() const {
-    assert(t < 32);
+    cb_assert(t < 32);
     return 1 << t;
   }
   uint32_t b_mask() const {
-    assert(b < 32);
+    cb_assert(b < 32);
     return (1 << b) - 1;
   }
   void convert(coinbase::converter_t& c) { c.convert(rho, b); }  // t is not sent

src/cbmpc/zk/zk_ec.cpp

@@ -57,7 +57,11 @@ error_t uc_dl_t::verify(const ecc_point_t& Q, mem_t session_id, uint64_t aux) co
   error_t rv = UNINITIALIZED_ERROR;
   crypto::vartime_scope_t vartime_scope;
   int rho = params.rho;
-  if (params.b * rho < SEC_P_COM) return coinbase::error(E_CRYPTO, "uc_dl_t::verify: b * rho < SEC_P_COM");
+  if (rho <= 0) return coinbase::error(E_CRYPTO, "uc_dl_t::verify: rho <= 0");
+  if (params.b <= 0) return coinbase::error(E_CRYPTO, "uc_dl_t::verify: b <= 0");
+  if (params.b >= 32) return coinbase::error(E_CRYPTO, "uc_dl_t::verify: b >= 32");
+  if (int64_t(params.b) * int64_t(rho) < SEC_P_COM)
+    return coinbase::error(E_CRYPTO, "uc_dl_t::verify: b * rho < SEC_P_COM");
   if (int(A.size()) != rho) return coinbase::error(E_CRYPTO, "uc_dl_t::verify: A.size() != rho");
   if (int(e.size()) != rho) return coinbase::error(E_CRYPTO, "uc_dl_t::verify: e.size() != rho");
   if (int(z.size()) != rho) return coinbase::error(E_CRYPTO, "uc_dl_t::verify: z.size() != rho");
@@ -214,11 +218,16 @@ error_t uc_batch_dl_finite_difference_impl_t::verify(const std::vector<ecc_point
                                                      uint64_t aux) const {
   error_t rv = UNINITIALIZED_ERROR;
   int n = int(Q.size());
+  if (n <= 0) return coinbase::error(E_CRYPTO, "uc_batch_dl_t::verify: Q.size() <= 0");
   crypto::vartime_scope_t vartime_scope;
   int rho = params.rho;
-  if (rho * (params.b - int_log2(n)) < SEC_P_COM)
-    return coinbase::error(E_CRYPTO,
-                           "uc_batch_dl_finite_difference_impl_t::verify: rho * (params.b - int_log2(n)) < SEC_P_COM");
+  if (rho <= 0) return coinbase::error(E_CRYPTO, "uc_batch_dl_t::verify: rho <= 0");
+  if (params.b <= 0) return coinbase::error(E_CRYPTO, "uc_batch_dl_t::verify: b <= 0");
+  if (params.b >= 32) return coinbase::error(E_CRYPTO, "uc_batch_dl_t::verify: b >= 32");
+  const int b_minus_log2n = params.b - int_log2(n);
+  if (b_minus_log2n <= 0) return coinbase::error(E_CRYPTO, "uc_batch_dl_t::verify: b - int_log2(n) <= 0");
+  if (int64_t(rho) * int64_t(b_minus_log2n) < SEC_P_COM)
+    return coinbase::error(E_CRYPTO, "uc_batch_dl_t::verify: rho * (b - int_log2(n)) < SEC_P_COM");
   if (int(R.size()) != rho)
     return coinbase::error(E_CRYPTO, "uc_batch_dl_finite_difference_impl_t::verify: R.size() != rho");
   if (int(e.size()) != rho)
@@ -236,16 +245,17 @@ error_t uc_batch_dl_finite_difference_impl_t::verify(const std::vector<ecc_point
 
   const auto& G = curve.generator();
   uint32_t b_mask = params.b_mask();
+  for (int i = 0; i < rho; i++) {
+    if (rv = curve.check(R[i]))
+      return coinbase::error(rv, "uc_batch_dl_finite_difference_impl_t::verify: R[i] is not on the curve");
+  }
   buf_t common_hash = crypto::ro::hash_string(G, Q, R, session_id, aux).bitlen(2 * SEC_P_COM);
 
   std::vector<ecc_point_t> PQ(n + 1);
   PQ[0] = curve.infinity();
   for (int i = 0; i < n; i++) PQ[i + 1] = Q[i];
 
   for (int i = 0; i < rho; i++) {
-    if (rv = curve.check(R[i]))
-      return coinbase::error(rv, "uc_batch_dl_finite_difference_impl_t::verify: R[i] is not on the curve");
-
     bn_t ei = e[i];
     if (ei < 0) ei += q;
 
@@ -291,6 +301,9 @@ error_t dh_t::verify(const ecc_point_t& Q, const ecc_point_t& A, const ecc_point
   const auto& G = curve.generator();
   const mod_t& q = curve.order();
 
+  if (rv = crypto::check_right_open_range(0, e, q)) return rv;
+  if (rv = crypto::check_right_open_range(0, z, q)) return rv;
+
   ecc_point_t X = z * G - e * A;
   ecc_point_t Y = z * Q - e * B;
 

src/cbmpc/zk/zk_elgamal_com.cpp

@@ -64,7 +64,11 @@ error_t uc_elgamal_com_t::verify(const ecc_point_t& Q, const elg_com_t& UV, mem_
   error_t rv = UNINITIALIZED_ERROR;
   crypto::vartime_scope_t vartime_scope;
   int rho = params.rho;
-  if (params.b * rho < SEC_P_COM) return coinbase::error(E_CRYPTO);
+  if (rho <= 0) return coinbase::error(E_CRYPTO, "uc_elgamal_com_t::verify: rho <= 0");
+  if (params.b <= 0) return coinbase::error(E_CRYPTO, "uc_elgamal_com_t::verify: b <= 0");
+  if (params.b >= 32) return coinbase::error(E_CRYPTO, "uc_elgamal_com_t::verify: b >= 32");
+  if (int64_t(params.b) * int64_t(rho) < SEC_P_COM)
+    return coinbase::error(E_CRYPTO, "uc_elgamal_com_t::verify: b * rho < SEC_P_COM");
   if (int(AB.size()) != rho) return coinbase::error(E_CRYPTO);
   if (int(e.size()) != rho) return coinbase::error(E_CRYPTO);
   if (int(z1.size()) != rho) return coinbase::error(E_CRYPTO);
@@ -160,6 +164,11 @@ error_t elgamal_com_mult_t::verify(const ecc_point_t& Q, const elg_com_t& A, con
 
   const mod_t& q = curve.order();
 
+  if (rv = crypto::check_right_open_range(0, e, q)) return rv;
+  if (rv = crypto::check_right_open_range(0, z1, q)) return rv;
+  if (rv = crypto::check_right_open_range(0, z2, q)) return rv;
+  if (rv = crypto::check_right_open_range(0, z3, q)) return rv;
+
   auto R = crypto::ec_elgamal_commitment_t::commit(Q, z1).rand(z2) - e * B;
   auto A_tag = (z1 * A).rerand(Q, z3) - e * C;
   bn_t e_tag = crypto::ro::hash_number(Q, R, A_tag, A, B, C, session_id, aux).mod(q);
@@ -232,7 +241,11 @@ error_t uc_elgamal_com_mult_private_scalar_t::verify(const ecc_point_t& Q, const
   error_t rv = UNINITIALIZED_ERROR;
   crypto::vartime_scope_t vartime_scope;
   int rho = params.rho;
-  if (params.b * rho < SEC_P_COM) return coinbase::error(E_CRYPTO);
+  if (rho <= 0) return coinbase::error(E_CRYPTO, "uc_elgamal_com_mult_private_scalar_t::verify: rho <= 0");
+  if (params.b <= 0) return coinbase::error(E_CRYPTO, "uc_elgamal_com_mult_private_scalar_t::verify: b <= 0");
+  if (params.b >= 32) return coinbase::error(E_CRYPTO, "uc_elgamal_com_mult_private_scalar_t::verify: b >= 32");
+  if (int64_t(params.b) * int64_t(rho) < SEC_P_COM)
+    return coinbase::error(E_CRYPTO, "uc_elgamal_com_mult_private_scalar_t::verify: b * rho < SEC_P_COM");
   if (int(A1_tag.size()) != rho) return coinbase::error(E_CRYPTO);
   if (int(A2_tag.size()) != rho) return coinbase::error(E_CRYPTO);
   if (int(e.size()) != rho) return coinbase::error(E_CRYPTO);
@@ -249,6 +262,16 @@ error_t uc_elgamal_com_mult_private_scalar_t::verify(const ecc_point_t& Q, const
   const mod_t& q = curve.order();
   const auto& G = curve.generator();
   uint16_t b_mask = params.b_mask();
+
+  for (int i = 0; i < rho; i++) {
+    if (rv = curve.check(A1_tag[i]))
+      return coinbase::error(rv, "uc_elgamal_com_mult_private_scalar_t::verify: check A1_tag failed");
+    if (rv = curve.check(A2_tag[i]))
+      return coinbase::error(rv, "uc_elgamal_com_mult_private_scalar_t::verify: check A2_tag failed");
+    if (rv = crypto::check_right_open_range(0, z1[i], q)) return rv;
+    if (rv = crypto::check_right_open_range(0, z2[i], q)) return rv;
+  }
+
   buf_t common_hash = crypto::ro::hash_string(Q, A, B, A1_tag, A2_tag, session_id, aux).bitlen(2 * SEC_P_COM);
 
   bn_t z1_sum = 0;
@@ -258,11 +281,6 @@ error_t uc_elgamal_com_mult_private_scalar_t::verify(const ecc_point_t& Q, const
   ecc_point_t A2_sum = curve.infinity();
 
   for (int i = 0; i < rho; i++) {
-    if (rv = curve.check(A1_tag[i]))
-      return coinbase::error(rv, "uc_elgamal_com_mult_private_scalar_t::verify: check A1_tag failed");
-    if (rv = curve.check(A2_tag[i]))
-      return coinbase::error(rv, "uc_elgamal_com_mult_private_scalar_t::verify: check A2_tag failed");
-
     bn_t sigma = bn_t::rand_bitlen(SEC_P_STAT);
     MODULO(q) {
       z1_sum += sigma * z1[i];

src/cbmpc/zk/zk_paillier.cpp

@@ -528,9 +528,16 @@ error_t pdl_t::verify(const bn_t& c_key, const crypto::paillier_t& paillier, con
 
   const mod_t& N = paillier.get_N();
   ecurve_t curve = Q1.get_curve();
+  if (!curve) return coinbase::error(E_CRYPTO, "pdl_t::verify: Q1 curve is null");
   const mod_t& q = curve.order();
   const auto& G = curve.generator();
 
+  if (rv = curve.check(Q1)) return coinbase::error(rv, "pdl_t::verify: Q1 is not a valid curve point");
+  {
+    crypto::allow_ecc_infinity_t allow_infinity;
+    if (rv = curve.check(R)) return coinbase::error(rv, "pdl_t::verify: R is not a valid curve point");
+  }
+
   bn_t e = crypto::ro::hash_number(c_key, N, Q1, c_r, R, sid, aux).mod(q);
 
   if (paillier_valid_key == zk_flag::unverified) return coinbase::error(E_CRYPTO);

src/cbmpc/zk/zk_pedersen.cpp

@@ -336,34 +336,43 @@ error_t paillier_pedersen_equal_t::verify(const crypto::paillier_t& paillier, co
     paillier_no_small_factors = zk_flag::verified;
   }
 
+  if (e < 0) return coinbase::error(E_CRYPTO, "paillier_pedersen_equal_t::verify: e < 0");
+  if (nu < 0) return coinbase::error(E_CRYPTO, "paillier_pedersen_equal_t::verify: nu < 0");
+
   // The following verification of `paillier.verify_cipher(c))` is removed and instead done with `D_prod`
   // later on to increase efficiency and save a GCD operation.
 
   bn_t q_with_slack = q << (param::log_alpha + SEC_P_STAT);
   const mod_t& NN = paillier.get_NN();
 
-  crypto::paillier_t::elem_t c_tilde[param::t];
-  bn_t c_inv = NN.inv(c);
-
   bn_t e_temp = e;
   bn_t radix = 1 << param::log_alpha;
 
+  if (rv = coinbase::crypto::check_open_range(0, c, NN))
+    return coinbase::error(rv, "paillier_pedersen_equal_t::verify: invalid ciphertext c");
+
   bn_t D_prod = c;
   bn_t d = 0;
   for (int i = 0; i < param::t; i++) {
     MODULO(N) D_prod *= D[i];
 
+    if (rv = coinbase::crypto::check_open_range(0, D[i], N)) return rv;
+
     if (rv = coinbase::crypto::check_open_range(0, di[i], q_with_slack)) return rv;
     d += di[i] << (i * param::log_alpha);
+  }
+  if (D_prod == 0) return coinbase::error(E_CRYPTO);
+  if (!mod_t::coprime(D_prod, N)) return coinbase::error(E_CRYPTO);
 
+  bn_t c_inv = NN.inv(c);
+  crypto::paillier_t::elem_t c_tilde[param::t];
+  e_temp = e;
+  for (int i = 0; i < param::t; i++) {
     bn_t ei = mod_t::mod(e_temp, radix);
     e_temp >>= param::log_alpha;
-
     crypto::paillier_t::elem_t c_tag(paillier, c_inv.pow_mod(ei, NN));
     c_tilde[i] = c_tag + paillier.enc(di[i], D[i]);
   }
-  if (D_prod == 0) return coinbase::error(E_CRYPTO);
-  if (!mod_t::coprime(D_prod, N)) return coinbase::error(E_CRYPTO);
 
   buf_t e_buf =
       crypto::ro::hash_string(N, c, p, q, g, h, Com, c_tilde, Com_tilde, session_id, aux).bitlen(param::lambda);
@@ -451,6 +460,9 @@ error_t paillier_pedersen_equal_interactive_t::verify(const crypto::paillier_t&
     paillier_no_small_factors = zk_flag::verified;
   }
 
+  if (e < 0) return coinbase::error(E_CRYPTO, "paillier_pedersen_equal_interactive_t::verify: e < 0");
+  if (nu < 0) return coinbase::error(E_CRYPTO, "paillier_pedersen_equal_interactive_t::verify: nu < 0");
+
   const pedersen_commitment_params_t& params = pedersen_commitment_params_t::get();
   const mod_t& p = params.p;
   const bn_t& g = params.g;
@@ -473,15 +485,20 @@ error_t paillier_pedersen_equal_interactive_t::verify(const crypto::paillier_t&
 
   bn_t d = 0;
   bn_t CD = c;
-  bn_t e_temp = e;
-  bn_t radix = 1 << param::log_alpha;
   bn_t q_with_slack = q << (param::log_alpha + SEC_P_STAT);
 
   for (int i = 0; i < param::t; i++) {
+    if (rv = coinbase::crypto::check_right_open_range(0, di[i], q_with_slack)) return rv;
+    if (rv = coinbase::crypto::check_open_range(0, Di[i], N)) return rv;
     MODULO(N) CD *= Di[i] * c_tilde[i];
+  }
 
-    if (rv = coinbase::crypto::check_right_open_range(0, di[i], q_with_slack)) return rv;
+  if (CD == 0) return coinbase::error(E_CRYPTO);
+  if (!mod_t::coprime(CD, N)) return coinbase::error(E_CRYPTO);
 
+  bn_t e_temp = e;
+  bn_t radix = 1 << param::log_alpha;
+  for (int i = 0; i < param::t; i++) {
     bn_t ei = mod_t::mod(e_temp, radix);
     e_temp >>= param::log_alpha;
 
@@ -492,9 +509,6 @@ error_t paillier_pedersen_equal_interactive_t::verify(const crypto::paillier_t&
     d += di[i] << (i * param::log_alpha);
   }
 
-  if (CD == 0) return coinbase::error(E_CRYPTO);
-  if (!mod_t::coprime(CD, N)) return coinbase::error(E_CRYPTO);
-
   bn_t C1, C2;
   MODULO(p) {
     C1 = Com.pow(e) * Com_tilde;

src/cbmpc/zk/zk_unknown_order.cpp

@@ -29,6 +29,28 @@ void unknown_order_dl_t::prove(const bn_t& a, const bn_t& b, const mod_t& N, con
 error_t unknown_order_dl_t::verify(const bn_t& a, const bn_t& b, const mod_t& N, const int l, mem_t sid,
                                    uint64_t aux) const {
   crypto::vartime_scope_t vartime_scope;
+  error_t rv = UNINITIALIZED_ERROR;
+
+  if (l <= 0) return coinbase::error(E_CRYPTO, "unknown_order_dl_t::verify: l <= 0");
+  if (e.size() != coinbase::bits_to_bytes(SEC_P_COM))
+    return coinbase::error(E_CRYPTO, "unknown_order_dl_t::verify: invalid e size");
+
+  // Ensure `b` is invertible mod N before attempting BN_mod_inverse (which asserts on failure).
+  if (rv = coinbase::crypto::check_open_range(0, b, N))
+    return coinbase::error(rv, "unknown_order_dl_t::verify: invalid b");
+  if (!mod_t::coprime(b, N)) return coinbase::error(E_CRYPTO, "unknown_order_dl_t::verify: gcd(b, N) != 1");
+
+  const int max_z_bits = l + SEC_P_STAT + 2;
+  for (int i = 0; i < SEC_P_COM; i++) {
+    if (z[i] < 0) return coinbase::error(E_CRYPTO, "unknown_order_dl_t::verify: z[i] < 0");
+    if (z[i].get_bits_count() > max_z_bits)
+      return coinbase::error(E_CRYPTO, "unknown_order_dl_t::verify: z[i] too large");
+  }
+
+  bn_t gcd_test;
+  MODULO(N) gcd_test = a * b;
+  if (!mod_t::coprime(gcd_test, N)) return coinbase::error(E_CRYPTO, "unknown_order_dl_t::verify: gcd(a*b, N) != 1");
+
   bn_t b_inv = N.inv(b);
 
   bn_t R_tag;