From f4832cc12eb68c039fd88b4a6507089472211c83 Mon Sep 17 00:00:00 2001 From: Yi-Hsiu Chen Date: Mon, 5 Jan 2026 21:22:24 -0800 Subject: [PATCH] fix: harden ZK verifiers and crypto primitives - Add parameter validation in ZK verifiers (rho, b bounds, overflow checks) - Add range checks for proof elements (e, z values) before use - Add curve point validation for commitment elements (R, A, B arrays) - Fix secp256k1 mul_add to handle edge cases in vartime/verifier paths - Fix bn_t::from_bin_bitlen to handle 0-bit input without OOB access - Reject mod_t deserialization when modulus <= 1 --- src/cbmpc/crypto/base_bn.cpp | 2 ++ src/cbmpc/crypto/base_ecc_secp256k1.cpp | 35 +++++++++++++++++++---- src/cbmpc/crypto/base_mod.cpp | 2 +- src/cbmpc/zk/fischlin.h | 4 +-- src/cbmpc/zk/zk_ec.cpp | 27 +++++++++++++----- src/cbmpc/zk/zk_elgamal_com.cpp | 32 ++++++++++++++++----- src/cbmpc/zk/zk_paillier.cpp | 7 +++++ src/cbmpc/zk/zk_pedersen.cpp | 38 +++++++++++++++++-------- src/cbmpc/zk/zk_unknown_order.cpp | 22 ++++++++++++++ 9 files changed, 135 insertions(+), 34 deletions(-) diff --git a/src/cbmpc/crypto/base_bn.cpp b/src/cbmpc/crypto/base_bn.cpp index 661e4b6d..da956b72 100644 --- a/src/cbmpc/crypto/base_bn.cpp +++ b/src/cbmpc/crypto/base_bn.cpp @@ -503,6 +503,8 @@ std::vector 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); diff --git a/src/cbmpc/crypto/base_ecc_secp256k1.cpp b/src/cbmpc/crypto/base_ecc_secp256k1.cpp index 27f62667..01f3dc00 100644 --- a/src/cbmpc/crypto/base_ecc_secp256k1.cpp +++ b/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); diff --git a/src/cbmpc/crypto/base_mod.cpp b/src/cbmpc/crypto/base_mod.cpp index 6e24012d..c4ecc9ad 100644 --- a/src/cbmpc/crypto/base_mod.cpp +++ b/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; } diff --git a/src/cbmpc/zk/fischlin.h b/src/cbmpc/zk/fischlin.h index f82f2bb1..a14eaf15 100644 --- a/src/cbmpc/zk/fischlin.h +++ b/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 diff --git a/src/cbmpc/zk/zk_ec.cpp b/src/cbmpc/zk/zk_ec.cpp index 8df7c969..409b1806 100644 --- a/src/cbmpc/zk/zk_ec.cpp +++ b/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= 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,6 +245,10 @@ error_t uc_batch_dl_finite_difference_impl_t::verify(const std::vector PQ(n + 1); @@ -243,9 +256,6 @@ error_t uc_batch_dl_finite_difference_impl_t::verify(const std::vector= 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]; diff --git a/src/cbmpc/zk/zk_paillier.cpp b/src/cbmpc/zk/zk_paillier.cpp index f20e99d9..ea8b74f1 100644 --- a/src/cbmpc/zk/zk_paillier.cpp +++ b/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); diff --git a/src/cbmpc/zk/zk_pedersen.cpp b/src/cbmpc/zk/zk_pedersen.cpp index 29cfe85b..0d250536 100644 --- a/src/cbmpc/zk/zk_pedersen.cpp +++ b/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; diff --git a/src/cbmpc/zk/zk_unknown_order.cpp b/src/cbmpc/zk/zk_unknown_order.cpp index 0266618d..c908cd5b 100644 --- a/src/cbmpc/zk/zk_unknown_order.cpp +++ b/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;