Skip to content

Commit 99ecc49

Browse files
authored
fix: harden Paillier validation, fix buf_t aliasing, and add bn256_t … (#103)
- Fix critical use-after-free in buf_t::operator=(mem_t) and operator+=(mem_t) where a mem_t view aliasing the destination buffer would read zeroed/freed memory after the buffer was destroyed during reassignment. - Harden Paillier create_pub to validate modulus (odd, >1, within bit_size) and return error_t instead of void, preventing acceptance of malicious moduli from counterparties. - Enforce upper-bound check on Paillier modulus size in ECDSA 2PC keygen and refresh to prevent potential buffer overflows in constant-time decryption. - Fix out-of-bounds write in HD key derivation by resizing derived_keys inside derive_keys() rather than relying on caller pre-allocation. - Add bn256_t: fixed-width 256-bit modular arithmetic with constant-time Barrett reduction, Montgomery multiplication, and x86_64 MULX/ADX assembly. Replaces the old uint256_t from extended_uint.cpp. - Fix const-correctness in secp256k1 point conversion to avoid mutating input Jacobian coordinates through cast-away-const UB. - Add vartime_scope_t to paillier_t::verify_cipher for correct side-channel annotation on public ciphertext validation. - Add bits_t::ref_t copy assignment operator for correct value semantics. - Fix static initialization order for test channel fuzzing DRBG.
1 parent 3217a88 commit 99ecc49

26 files changed

Lines changed: 1780 additions & 101 deletions

include-internal/cbmpc/internal/crypto/base_bn.h

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,21 @@ struct bignum_st {
1717
int flags;
1818
};
1919

20+
struct bn_mont_ctx_st {
21+
#if OPENSSL_VERSION_NUMBER < 0x30600000L
22+
int ri; /* number of bits in R */
23+
#endif
24+
BIGNUM RR; /* used to convert to montgomery form,
25+
possibly zero-padded */
26+
BIGNUM N; /* The modulus */
27+
BIGNUM Ni; /* R*(1/R mod N) - N*Ni = 1 (Ni is only
28+
* stored for bignum algorithm) */
29+
BN_ULONG n0[2]; /* least significant word(s) of Ni; (type
30+
* changed with 0.9.9, was "BN_ULONG n0;"
31+
* before) */
32+
int flags;
33+
};
34+
2035
namespace coinbase::crypto {
2136

2237
constexpr int div_ceil(int a, int b) { return (a + b - 1) / b; }
@@ -40,6 +55,7 @@ class bn_t {
4055
friend class crypto::paillier_t;
4156
friend class crypto::ecdsa_signature_t;
4257
friend class montgomery_t;
58+
friend class bn256_t;
4359
friend std::ostream& operator<<(std::ostream& os, const bn_t& obj);
4460

4561
public:
@@ -215,6 +231,8 @@ class bn_t {
215231
for (coinbase::crypto::bn_t::set_modulo(n); coinbase::crypto::bn_t::check_modulo(n); \
216232
coinbase::crypto::bn_t::reset_modulo(n))
217233

234+
const mod_t* thread_local_storage_mod();
235+
218236
bn_t operator+(const bn_t& b1, const bn_t& b2);
219237
bn_t operator-(const bn_t& b1, const bn_t& b2);
220238
bn_t operator*(const bn_t& b1, const bn_t& b2);
Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
#pragma once
2+
#include "base_bn.h"
3+
4+
using uint128_t = unsigned __int128;
5+
6+
namespace coinbase::crypto {
7+
8+
#ifdef __x86_64__
9+
bool support_x64_mulx();
10+
#endif
11+
12+
inline uint64_t addx(uint64_t x, uint64_t y, uint64_t& carry) {
13+
#ifdef __x86_64__
14+
unsigned long long z;
15+
carry = _addcarry_u64(uint8_t(carry), x, y, &z);
16+
return z;
17+
#else
18+
#if __has_builtin(__builtin_addcl)
19+
return __builtin_addcl(x, y, carry, (unsigned long int*)&carry);
20+
#else
21+
auto r = uint128_t(x) + y + carry;
22+
carry = uint64_t(r >> 64);
23+
return uint64_t(r);
24+
#endif
25+
#endif
26+
}
27+
28+
inline uint64_t subx(uint64_t x, uint64_t y, uint64_t& borrow) {
29+
#ifdef __x86_64__
30+
unsigned long long z;
31+
borrow = _subborrow_u64(uint8_t(borrow), x, y, &z);
32+
return z;
33+
#else
34+
#if __has_builtin(__builtin_subcl)
35+
return __builtin_subcl(x, y, borrow, (unsigned long int*)&borrow);
36+
#else
37+
auto r = uint128_t(x) - y - borrow;
38+
borrow = uint64_t(r >> 64) & 1;
39+
return uint64_t(r);
40+
#endif
41+
#endif
42+
}
43+
44+
struct alignas(32) uint256_t {
45+
uint64_t w0, w1, w2, w3;
46+
static uint256_t from_hex(const std::string& hex) { return from_bn(bn_t::from_hex(hex.c_str())); }
47+
static uint256_t from_str(const std::string& str) { return from_bn(bn_t::from_string(str.c_str())); }
48+
49+
void to_bin(byte_ptr bin) const;
50+
buf_t to_bin() const;
51+
static uint256_t from_bin(mem_t bin);
52+
void to_bin_le(byte_ptr bin) const;
53+
buf_t to_bin_le() const;
54+
static uint256_t from_bin_le(mem_t bin);
55+
bn_t to_bn() const;
56+
static uint256_t from_bn(const bn_t& bn);
57+
58+
bool is_zero() const;
59+
bool is_odd() const;
60+
bool operator==(const uint256_t& b) const;
61+
bool operator!=(const uint256_t& b) const;
62+
void cnd_assign(bool flag, const uint256_t& b);
63+
uint64_t mul_add_regular(const uint256_t& a, uint64_t b);
64+
uint64_t div_by_two();
65+
void inv_mod(const uint256_t& x, const uint256_t& m);
66+
67+
static void mul_noasm(uint64_t r[8], const uint256_t& a, const uint256_t& b);
68+
static void mul(uint64_t r[8], const uint256_t& a, const uint256_t& b);
69+
static void sqr_noasm(uint64_t r[8], const uint256_t& a);
70+
static void sqr(uint64_t r[8], const uint256_t& a);
71+
72+
static uint256_t get_mont_rr(const uint256_t& mod);
73+
static uint256_t make(uint64_t w0, uint64_t w1 = 0, uint64_t w2 = 0, uint64_t w3 = 0);
74+
75+
private:
76+
uint64_t add_raw(const uint256_t& a, const uint256_t& b);
77+
uint64_t sub_raw(const uint256_t& a, const uint256_t& b);
78+
uint64_t cnd_add_raw(bool flag, const uint256_t& a);
79+
uint64_t cnd_sub_raw(bool flag, const uint256_t& a);
80+
uint64_t cnd_neg_raw(bool flag);
81+
static void cnd_swap(bool flag, uint256_t& a, uint256_t& b);
82+
};
83+
84+
struct uint320_t {
85+
uint64_t w0, w1, w2, w3, w4;
86+
void from_bn(const bn_t& bn);
87+
static uint320_t get_barrett_mu(const uint256_t& mod);
88+
};
89+
90+
class alignas(32) bn256_t {
91+
public:
92+
bn256_t();
93+
~bn256_t();
94+
bn256_t(const bn256_t& x);
95+
bn256_t(const bn_t& x);
96+
bn256_t(uint64_t x);
97+
98+
bn256_t& operator=(const bn256_t& x);
99+
bn256_t& operator=(const bn_t& x);
100+
bn256_t& operator=(uint64_t x);
101+
operator bn_t() const;
102+
103+
bool operator==(uint64_t x) const;
104+
bool operator!=(uint64_t x) const;
105+
106+
bool operator==(const bn256_t& b) const;
107+
bool operator!=(const bn256_t& b) const;
108+
109+
bn256_t operator+(const bn256_t& b) const;
110+
bn256_t operator-(const bn256_t& b) const;
111+
bn256_t operator*(const bn256_t& b) const;
112+
bn256_t operator/(const bn256_t& b) const;
113+
114+
bn256_t& operator+=(const bn256_t& b);
115+
bn256_t& operator-=(const bn256_t& b);
116+
bn256_t& operator*=(const bn256_t& b);
117+
bn256_t& operator/=(const bn256_t& b);
118+
119+
bn256_t operator-() const;
120+
bn256_t inv_mod(const mod_t& q) const;
121+
122+
void to_bin(byte_ptr m) const;
123+
buf_t to_bin() const;
124+
static bn256_t from_bin(mem_t m);
125+
void convert(coinbase::converter_t& c);
126+
static bn256_t rand(const mod_t& q);
127+
int get_bit(int b) const;
128+
129+
bn256_t to_mont() const;
130+
bn256_t from_mont() const;
131+
static bn256_t mont_mul(const bn256_t& a, const bn256_t& b);
132+
133+
static void mul_add_no_reduce(uint64_t r[8], const bn256_t& a, const bn256_t& b);
134+
static void add_no_reduce(uint64_t r[8], const bn256_t& a);
135+
static bn256_t reduce(uint64_t r[8]);
136+
static bn256_t two_to_pow(int n);
137+
138+
private:
139+
uint64_t w0, w1, w2, w3;
140+
void barrett_reduce(uint64_t x[8], const uint64_t* m, const uint64_t* mu);
141+
void mont_mul(const bn256_t& a, const bn256_t& b, const uint64_t* m, uint64_t mont_prime);
142+
void mont_reduce(uint64_t u[8], const uint64_t* m, uint64_t mont_prime);
143+
};
144+
145+
} // namespace coinbase::crypto
146+
147+
using coinbase::crypto::bn256_t;

include-internal/cbmpc/internal/crypto/base_mod.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ class mod_t {
3232
mod_t(const bn_t& m, bool multiplicative_dense = true) : multiplicative_dense(multiplicative_dense) { init(m); }
3333

3434
void convert(coinbase::converter_t&);
35+
static bool is_valid_modulus(const bn_t& m);
3536

3637
// clang-format off
3738
bn_t add(const bn_t& a, const bn_t& b) const { bn_t r; _add(r, a, b); return r; }
@@ -94,6 +95,7 @@ class mod_t {
9495

9596
BN_MONT_CTX* get_mont_ctx() const { return mont; }
9697
const bn_t& value() const { return m; }
98+
const bn_t& get_barrett_mu() const { return mu; }
9799

98100
private:
99101
bn_t m;

include-internal/cbmpc/internal/crypto/base_paillier.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ class paillier_t {
3535
*/
3636
void generate();
3737
void create_prv(const bn_t& N, const bn_t& p, const bn_t& q);
38-
void create_pub(const bn_t& N);
38+
// Returns an error instead of asserting when rebuilding a public key from externally supplied material.
39+
error_t create_pub(const bn_t& N);
3940

4041
/**
4142
* @specs:
@@ -64,6 +65,8 @@ class paillier_t {
6465

6566
bn_t get_cipher_randomness(const bn_t& plain, const bn_t& cipher) const;
6667

68+
// Private-key deserialization is intended for trusted local state only; callers must not feed it untrusted
69+
// private-key material unless they validate it before use.
6770
void convert(coinbase::converter_t& converter);
6871

6972
bool has_private_key() const { return has_private; }

include-internal/cbmpc/internal/crypto/ro.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22

33
#include <cbmpc/internal/crypto/base.h>
44

5+
namespace coinbase::crypto {
6+
class bn256_t;
7+
}
8+
59
namespace coinbase::crypto::ro { // random oracle
610
struct hmac_state_t {
711
crypto::hmac_sha256_t hmac;
@@ -114,6 +118,7 @@ class hash_numbers_t : public hmac_state_t {
114118
return *this;
115119
}
116120
std::vector<bn_t> mod(const mod_t& q);
121+
std::vector<bn256_t> mod256(const mod_t& p);
117122

118123
private:
119124
int l;

include/cbmpc/core/buf.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,10 @@ class bits_t {
266266
set(value);
267267
return *this;
268268
}
269+
ref_t& operator=(const ref_t& src) noexcept {
270+
set(src.get());
271+
return *this;
272+
}
269273
operator bool() const { return get(); }
270274

271275
private:

src/cbmpc/core/CMakeLists.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,5 +12,4 @@ target_sources(cbmpc_core PRIVATE
1212
convert.cpp
1313
error.cpp
1414
strext.cpp
15-
extended_uint.cpp
1615
)

src/cbmpc/core/buf.cpp

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -90,14 +90,12 @@ int buf_t::size() const { return s; }
9090
bool buf_t::empty() const { return s == 0; }
9191

9292
buf_t& buf_t::operator=(mem_t src) {
93-
if (s != src.size || data() != src.data) {
94-
free();
93+
if (s == src.size && data() == src.data) return *this;
9594

96-
if (src.size <= short_size)
97-
assign_short(src.data, src.size);
98-
else
99-
assign_long(src.data, src.size);
100-
}
95+
// `mem_t` is a view and may alias this buffer (for example `buf = buf.take(n)`),
96+
// so copy it before zeroizing or reallocating the current storage.
97+
buf_t tmp(src);
98+
*this = std::move(tmp);
10199
return *this;
102100
}
103101

@@ -261,9 +259,11 @@ buf_t operator+(mem_t src1, mem_t src2) {
261259
buf_t& buf_t::operator+=(mem_t src) {
262260
cb_assert(src.size >= 0);
263261
cb_assert(s <= INT_MAX - src.size); // overflow check
262+
// `mem_t` may point into this buffer, and resize() can zeroize/reallocate it.
263+
buf_t tmp(src);
264264
int old_size = s;
265-
byte_ptr new_ptr = resize(old_size + src.size);
266-
memmove(new_ptr + old_size, src.data, src.size);
265+
byte_ptr new_ptr = resize(old_size + tmp.size());
266+
memmove(new_ptr + old_size, tmp.data(), tmp.size());
267267
return *this;
268268
}
269269

src/cbmpc/core/extended_uint.cpp

Lines changed: 0 additions & 46 deletions
This file was deleted.

src/cbmpc/crypto/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ target_sources(cbmpc_crypto PRIVATE
88

99
base.cpp
1010
base_bn.cpp
11+
base_bn256.cpp
1112
base_mod.cpp
1213
base_ec_core.cpp
1314
base_ecc.cpp

0 commit comments

Comments
 (0)