fix: security hardening for ZK proofs and protocol input validation

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_hash.h

@@ -167,7 +167,11 @@ T& update_state(T& state, const V (&v)[N]) {
 }
 template <class T, typename V>
 T& update_state(T& state, const std::vector<V>& v) {
-  for (std::size_t i = 0; i < (int)v.size(); i++) update_state(state, v[i]);
+  update_state(state, uint64_t(v.size()));
+  for (std::size_t i = 0; i < v.size(); i++) {
+    update_state(state, uint64_t(get_bin_size(v[i])));
+    update_state(state, v[i]);
+  }
   return state;
 }
 template <class T, typename V>
@@ -178,7 +182,11 @@ T& update_state(T& state, const V& v) {
 
 template <class T, typename V>
 T& update_state(T& state, const coinbase::array_view_t<V>& v) {
-  for (int i = 0; i < v.count; i++) update_state(state, v.ptr[i]);
+  update_state(state, uint64_t(v.count));
+  for (int i = 0; i < v.count; i++) {
+    update_state(state, uint64_t(get_bin_size(v.ptr[i])));
+    update_state(state, v.ptr[i]);
+  }
   return state;
 }
 

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/protocol/ec_dkg.cpp

@@ -223,6 +223,8 @@ error_t key_share_mp_t::refresh(job_mp_t& job, buf_t& sid, const key_share_mp_t&
     // Curve check of R._j[l] is done inside the zk verify function further below
 
     if (h._j != h) return coinbase::error(E_CRYPTO);
+    if (R._j.size() != size_t(n)) return coinbase::error(E_CRYPTO, "ec_dkg: inconsistent batch size (R)");
+    if (pi_r._j.size() != size_t(n)) return coinbase::error(E_CRYPTO, "ec_dkg: inconsistent batch size (pi_r)");
 
     if (rv = com_R.id(sid, job.get_pid(j)).set(rho._j, c._j).open(R._j, pi_r._j)) return rv;
     for (int l = 0; l < n; l++) {

src/cbmpc/protocol/ecdsa_2p.cpp

@@ -102,6 +102,7 @@ error_t dkg(job_2p_t& job, ecurve_t curve, key_t& key) {
   }
 
   if (rv = job.p1_to_p2(ec_dkg.msg1, paillier_gen.msg1, key.c_key)) return rv;
+  if (paillier_gen.c_key != key.c_key) return coinbase::error(E_CRYPTO, "paillier_gen.c_key != key.c_key");
 
   if (job.is_p2()) {
     ec_dkg.step2_p2_to_p1(key.x_share);
@@ -268,6 +269,7 @@ error_t sign_batch_impl(job_2p_t& job, buf_t& sid, const key_t& key, const std::
   }
 
   if (rv = job.p2_to_p1(R2, pi_2)) return rv;
+  if (job.is_p1() && R2.size() != n_sigs) return coinbase::error(E_CRYPTO, "ecdsa_2p: inconsistent batch size (R2)");
 
   std::vector<ecc_point_t> R(n_sigs);
 
@@ -289,6 +291,7 @@ error_t sign_batch_impl(job_2p_t& job, buf_t& sid, const key_t& key, const std::
   if (job.is_p2()) {
     const mod_t& N = key.paillier.get_N();
 
+    if (R1.size() != n_sigs) return coinbase::error(E_CRYPTO, "ecdsa_2p: inconsistent batch size (R1)");
     if (rv = com.open(msgs, R1, pi_1)) return rv;
 
     // Checking that R1 values are valid is done in the verify function.
@@ -331,6 +334,9 @@ error_t sign_batch_impl(job_2p_t& job, buf_t& sid, const key_t& key, const std::
   }
 
   if (job.is_p1()) {
+    if (c.size() != n_sigs) return coinbase::error(E_CRYPTO, "ecdsa_2p: inconsistent batch size (c)");
+    if (!global_abort_mode && zk_ecdsa.size() != n_sigs)
+      return coinbase::error(E_CRYPTO, "ecdsa_2p: inconsistent batch size (zk_ecdsa)");
     for (int i = 0; i < n_sigs; i++) {
       r[i] = R[i].get_x() % q;
 

src/cbmpc/protocol/schnorr_2p.cpp

@@ -50,6 +50,8 @@ error_t sign_batch(job_2p_t& job, key_t& key, const std::vector<mem_t>& msgs, st
     zk_dl2.prove(R2, k2, sid, 2);
   }
   if (rv = job.p2_to_p1(R2, zk_dl2, sid2)) return rv;
+  if (job.is_p1() && R2.size() != size_t(n_sigs))
+    return coinbase::error(E_CRYPTO, "schnorr_2p: inconsistent batch size (R2)");
 
   if (job.is_p1()) {
     // point checks are covered by the zk proof
@@ -60,6 +62,7 @@ error_t sign_batch(job_2p_t& job, key_t& key, const std::vector<mem_t>& msgs, st
   if (rv = job.p1_to_p2(zk_dl1, R1, com.rand)) return rv;
 
   if (job.is_p2()) {
+    if (R1.size() != size_t(n_sigs)) return coinbase::error(E_CRYPTO, "schnorr_2p: inconsistent batch size (R1)");
     // point checks are covered by the zk proof
     if (rv = com.id(sid1, job.get_pid(party_t::p1)).open(R1)) return rv;
     if (rv = zk_dl1.verify(R1, sid, 1)) return rv;
@@ -102,6 +105,7 @@ error_t sign_batch(job_2p_t& job, key_t& key, const std::vector<mem_t>& msgs, st
   if (rv = job.p2_to_p1(s2)) return rv;
 
   if (job.is_p1()) {
+    if (s2.size() != size_t(n_sigs)) return coinbase::error(E_CRYPTO, "schnorr_2p: inconsistent batch size (s2)");
     for (int i = 0; i < n_sigs; i++) {
       bn_t s, s1;
       MODULO(q) {

src/cbmpc/protocol/schnorr_mp.cpp

@@ -104,6 +104,8 @@ error_t sign_batch(job_mp_t& job, key_t& key, const std::vector<mem_t>& msgs, pa
 
     if (sid._j != sid._i) return coinbase::error(E_CRYPTO);
     if (h._j != h._i) return coinbase::error(E_CRYPTO);
+    if (Ri._j.size() != msgs.size())
+      return coinbase::error(E_CRYPTO, "schnorrmp::sign_batch: inconsistent batch size (Ri)");
     // Verification of Ri._j is done in the zk verify function
     if (rv = pi._j.verify(Ri._j, sid._i, j)) return coinbase::error(rv, "schnorr_mp_t::sign_batch: verify pi failed");
 
@@ -151,6 +153,8 @@ error_t sign_batch(job_mp_t& job, key_t& key, const std::vector<mem_t>& msgs, pa
   if (job.is_party_idx(sig_receiver)) {
     std::vector<bn_t> ss(msgs.size(), 0);
     for (int j = 0; j < n; j++) {
+      if (ssi._j.size() != msgs.size())
+        return coinbase::error(E_CRYPTO, "schnorrmp::sign_batch: inconsistent batch size (ssi)");
       for (size_t l = 0; l < msgs.size(); l++) MODULO(q) ss[l] += ssi._j[l];
     }
 

src/cbmpc/zk/fischlin.h

@@ -43,13 +43,31 @@ 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;
   }
+  error_t check() const {
+    if (rho <= 0) return coinbase::error(E_CRYPTO, "rho <= 0");
+    if (b <= 0) return coinbase::error(E_CRYPTO, "b <= 0");
+    if (b >= 32) return coinbase::error(E_CRYPTO, "b >= 32");
+    if (int64_t(b) * int64_t(rho) < SEC_P_COM) return coinbase::error(E_CRYPTO, "b * rho < SEC_P_COM");
+    return SUCCESS;
+  }
+
+  error_t check_with_effective_b(int effective_b) const {
+    if (rho <= 0) return coinbase::error(E_CRYPTO, "rho <= 0");
+    if (b <= 0) return coinbase::error(E_CRYPTO, "b <= 0");
+    if (b >= 32) return coinbase::error(E_CRYPTO, "b >= 32");
+
+    if (effective_b <= 0) return coinbase::error(E_CRYPTO, "effective_b <= 0");
+    if (int64_t(rho) * int64_t(effective_b) < SEC_P_COM)
+      return coinbase::error(E_CRYPTO, "rho * effective_b < SEC_P_COM");
+    return SUCCESS;
+  }
   void convert(coinbase::converter_t& c) { c.convert(rho, b); }  // t is not sent
 };
 

src/cbmpc/zk/zk_ec.cpp

@@ -56,8 +56,8 @@ void uc_dl_t::prove(const ecc_point_t& Q, const bn_t& w, mem_t session_id, uint6
 error_t uc_dl_t::verify(const ecc_point_t& Q, mem_t session_id, uint64_t aux) const {
   error_t rv = UNINITIALIZED_ERROR;
   crypto::vartime_scope_t vartime_scope;
+  if (rv = params.check()) return rv;
   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 (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 +214,11 @@ 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;
+  const int b_minus_log2n = params.b - int_log2(n);
+  if (rv = params.check_with_effective_b(b_minus_log2n)) return rv;
   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 (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 +236,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 +292,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

@@ -63,8 +63,8 @@ void uc_elgamal_com_t::prove(const ecc_point_t& Q, elg_com_t UV, const bn_t& x,
 error_t uc_elgamal_com_t::verify(const ecc_point_t& Q, const elg_com_t& UV, mem_t session_id, uint64_t aux) const {
   error_t rv = UNINITIALIZED_ERROR;
   crypto::vartime_scope_t vartime_scope;
+  if (rv = params.check()) return rv;
   int rho = params.rho;
-  if (params.b * rho < SEC_P_COM) return coinbase::error(E_CRYPTO);
   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 +160,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);
@@ -231,8 +236,8 @@ error_t uc_elgamal_com_mult_private_scalar_t::verify(const ecc_point_t& Q, const
                                                      mem_t session_id, uint64_t aux) {
   error_t rv = UNINITIALIZED_ERROR;
   crypto::vartime_scope_t vartime_scope;
+  if (rv = params.check()) return rv;
   int rho = params.rho;
-  if (params.b * rho < SEC_P_COM) return coinbase::error(E_CRYPTO);
   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 +254,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 +273,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);