sparrowwallet/UltrafastSecp256k1
sparrowwallet/UltrafastSecp256k1
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

security: comprehensive memory safety audit — 28 fixes across 13 files

Browse Source 
Engine-wide audit for secret-material leaks, dangling pointers, buffer
overflows, and missing secure_erase on stack temporaries.

HIGH fixes:
- schnorr_sign: erase all nonce/privkey material (d_bytes, t_hash, k, k_prime)
- wif_encode/wif_decode: erase raw private key bytes on all paths
- chacha20_poly1305 AEAD: fix RFC 8439 Poly1305 padding logic (crypto correctness)
- musig2_nonce_gen: erase 129-byte nonce_input buffers with secret t-derived data
- bip324 encrypt: add integer overflow guard (32-bit) and 3-byte length limit
- frost_sign ABI: consolidate error paths with secret material erasure

MEDIUM fixes:
- ecdh: erase shared_point Jacobian coords in all 3 functions
- ecdsa rfc6979_nonce: erase HMAC midstates (inner_mid/outer_mid)
- hkdf_expand: erase ipad_base/opad_base SHA256 states
- bip324: add Bip324Cipher/Bip324Session destructors for key_ erasure
- bip324: erase sk in complete_handshake and both constructors
- silent_payment_create_output: erase a_sum aggregate private key
- frost derive_scalar: erase hash of secret seed material
- ufsecp_impl: erase k1_bytes/k2_bytes, bip39 seed, scan result keys

LOW fixes:
- Poly1305 block(): defensive len clamp to prevent buf[17] overflow
- musig2 partial_sign: bounds check signer_index
- ecdsa rfc6979: erase buf33 in retry loops
- ecies hmac_sha256: erase inner_hash intermediate
- ct_sign: erase pre_sig copy of secret nonce-derived s value

68/68 tests pass. No regressions.
This commit is contained in:
shrec 2026-03-23 18:27:06 +00:00
parent e9b4f2cdb3
commit a52b81d07c
No known key found for this signature in database
13 changed files with 116 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
cpu/include/secp256k1/bip324.hpp
View File

@ -25,6 +25,7 @@
#include <array>
#include <cstdint>
#include <cstddef>
#include <cstring>
#include <vector>
#include "secp256k1/scalar.hpp"
@ -35,6 +36,11 @@ namespace secp256k1 {
class Bip324Cipher {
public:
Bip324Cipher() noexcept = default;
~Bip324Cipher() noexcept {
// Volatile write to prevent dead-store elimination
volatile std::uint8_t* p = key_;
for (std::size_t i = 0; i < 32; ++i) p[i] = 0;
}
// Initialize with a 32-byte key for this direction
void init(const std::uint8_t key[32]) noexcept;
@ -78,6 +84,12 @@ public:
// Initialize with a specific private key (for testing / deterministic use)
Bip324Session(bool initiator, const std::uint8_t privkey[32]) noexcept;
~Bip324Session() noexcept {
// Erase private key material on destruction
volatile std::uint8_t* p = privkey_.data();
for (std::size_t i = 0; i < 32; ++i) p[i] = 0;
}
// Get our 64-byte ElligatorSwift-encoded public key to send to peer
const std::array<std::uint8_t, 64>& our_ellswift_encoding() const noexcept {
return our_encoding_;

12
cpu/src/address.cpp
View File

@ -7,6 +7,7 @@
#include "secp256k1/schnorr.hpp"
#include "secp256k1/field.hpp"
#include "secp256k1/ct/point.hpp"
#include "secp256k1/detail/secure_erase.hpp"
#include <algorithm>
#include <cstring>
@ -576,7 +577,10 @@ std::string wif_encode(const Scalar& private_key, bool compressed, Network net)
std::memcpy(payload.data() + 1, key_bytes.data(), 32);
if (compressed) payload[33] = 0x01;
return base58check_encode(payload.data(), payload_len);
auto result = base58check_encode(payload.data(), payload_len);
detail::secure_erase(key_bytes.data(), key_bytes.size());
detail::secure_erase(payload.data(), payload.size());
return result;
}
WIFDecodeResult wif_decode(const std::string& wif) {
@ -602,6 +606,8 @@ WIFDecodeResult wif_decode(const std::string& wif) {
std::array<std::uint8_t, 32> key_bytes;
std::memcpy(key_bytes.data(), data.data() + 1, 32);
result.key = Scalar::from_bytes(key_bytes);
detail::secure_erase(key_bytes.data(), key_bytes.size());
detail::secure_erase(data.data(), data.size());
result.valid = true;
return result;
}
@ -690,6 +696,10 @@ silent_payment_create_output(const std::vector<Scalar>& input_privkeys,
// Output key: P_output = B_spend + t_k * G
Point const P_output = recipient.spend_pubkey.add(Point::generator().scalar_mul(t_k));
// Erase secret-derived material: aggregate private key and shared secret
detail::secure_erase(&a_sum, sizeof(a_sum));
detail::secure_erase(S_comp.data(), S_comp.size());
return {P_output, t_k};
}

9
cpu/src/bip324.cpp
View File

@ -95,6 +95,12 @@ std::vector<std::uint8_t> Bip324Cipher::encrypt(
const std::uint8_t* aad, std::size_t aad_len,
const std::uint8_t* plaintext, std::size_t plaintext_len) noexcept {
// BIP-324 length field is 3 bytes: reject plaintext > 0xFFFFFF
if (plaintext_len > 0xFFFFFF) return {};
// Overflow guard: 3 + plaintext_len + 16 must not wrap
if (plaintext_len > SIZE_MAX - 19) return {};
// Output: [3-byte encrypted length] [encrypted payload] [16-byte tag]
std::size_t const ct_len = 3 + plaintext_len;
std::vector<std::uint8_t> output(ct_len + 16);
@ -178,6 +184,7 @@ Bip324Session::Bip324Session(bool initiator) noexcept
csprng_fill(privkey_.data(), 32);
auto sk = fast::Scalar::from_bytes(privkey_);
our_encoding_ = ellswift_create(sk);
detail::secure_erase(&sk, sizeof(sk));
}
Bip324Session::Bip324Session(bool initiator, const std::uint8_t privkey[32]) noexcept
@ -185,6 +192,7 @@ Bip324Session::Bip324Session(bool initiator, const std::uint8_t privkey[32]) noe
std::memcpy(privkey_.data(), privkey, 32);
auto sk = fast::Scalar::from_bytes(privkey_);
our_encoding_ = ellswift_create(sk);
detail::secure_erase(&sk, sizeof(sk));
}
bool Bip324Session::complete_handshake(const std::uint8_t peer_encoding[64]) noexcept {
@ -247,6 +255,7 @@ bool Bip324Session::complete_handshake(const std::uint8_t peer_encoding[64]) noe
detail::secure_erase(prk.data(), prk.size());
detail::secure_erase(initiator_key, sizeof(initiator_key));
detail::secure_erase(responder_key, sizeof(responder_key));
detail::secure_erase(&sk, sizeof(sk));
established_ = true;
return true;

32
cpu/src/chacha20_poly1305.cpp
View File

@ -274,6 +274,7 @@ struct Poly1305State {
}
void block(const std::uint8_t* msg, std::size_t len) noexcept {
if (len > 16) len = 16; // defensive clamp
std::uint8_t buf[17]{};
std::memcpy(buf, msg, len);
buf[len] = 1;
@ -365,6 +366,7 @@ struct Poly1305State {
}
void block(const std::uint8_t* msg, std::size_t len) noexcept {
if (len > 16) len = 16; // defensive clamp
std::uint8_t buf[17]{};
std::memcpy(buf, msg, len);
buf[len] = 1;
@ -517,36 +519,34 @@ namespace {
void aead_poly1305_pad_and_mac(Poly1305State& st,
const std::uint8_t* aad, std::size_t aad_len,
const std::uint8_t* ct, std::size_t ct_len) noexcept {
// Process AAD
// Process AAD in full 16-byte blocks
std::size_t off = 0;
while (off + 16 <= aad_len) {
st.block(aad + off, 16);
off += 16;
}
// Pad last partial AAD block to 16 bytes (RFC 8439 pad16).
// The padding zeros are part of the mac_data stream and must be
// processed in the same Poly1305 block as the trailing AAD bytes,
// NOT as a separate block (a separate block would get its own
// hibit marker and produce a wrong MAC).
if (off < aad_len) {
st.block(aad + off, aad_len - off);
}
// Pad AAD to 16-byte boundary
std::size_t aad_pad = (16 - (aad_len % 16)) % 16;
if (aad_pad > 0) {
std::uint8_t zeros[16]{};
st.block(zeros, aad_pad);
std::uint8_t padded[16]{};
std::memcpy(padded, aad + off, aad_len - off);
st.block(padded, 16);
}
// Process ciphertext
// Process ciphertext in full 16-byte blocks
off = 0;
while (off + 16 <= ct_len) {
st.block(ct + off, 16);
off += 16;
}
// Pad last partial ciphertext block to 16 bytes (RFC 8439 pad16)
if (off < ct_len) {
st.block(ct + off, ct_len - off);
}
// Pad ciphertext to 16-byte boundary
std::size_t ct_pad = (16 - (ct_len % 16)) % 16;
if (ct_pad > 0) {
std::uint8_t zeros[16]{};
st.block(zeros, ct_pad);
std::uint8_t padded[16]{};
std::memcpy(padded, ct + off, ct_len - off);
st.block(padded, 16);
}
// Append lengths as two 64-bit little-endian values

1
cpu/src/ct_sign.cpp
View File

@ -348,6 +348,7 @@ RecoverableSignature ecdsa_sign_recoverable(
secure_erase(&k_inv, sizeof(k_inv));
secure_erase(&z, sizeof(z));
secure_erase(&s, sizeof(s));
secure_erase(const_cast<ECDSASignature*>(&pre_sig), sizeof(pre_sig));
return {sig, recid};
}

6
cpu/src/ecdh.cpp
View File

@ -26,6 +26,7 @@ std::array<std::uint8_t, 32> ecdh_compute(
// Hash with SHA-256
auto result = SHA256::hash(compressed.data(), compressed.size());
secp256k1::detail::secure_erase(compressed.data(), compressed.size());
secp256k1::detail::secure_erase(&shared_point, sizeof(shared_point));
return result;
}
@ -45,6 +46,7 @@ std::array<std::uint8_t, 32> ecdh_compute_xonly(
auto result = SHA256::hash(x_bytes.data(), x_bytes.size());
secp256k1::detail::secure_erase(x_bytes.data(), x_bytes.size());
secp256k1::detail::secure_erase(&shared_point, sizeof(shared_point));
return result;
}
@ -59,7 +61,9 @@ std::array<std::uint8_t, 32> ecdh_compute_raw(
auto shared_point = ct::scalar_mul(public_key, private_key);
if (shared_point.is_infinity()) return {};
return shared_point.x().to_bytes();
auto result = shared_point.x().to_bytes();
secp256k1::detail::secure_erase(&shared_point, sizeof(shared_point));
return result;
}
} // namespace secp256k1

6
cpu/src/ecdsa.cpp
View File

@ -333,6 +333,7 @@ Scalar rfc6979_nonce(const Scalar& private_key,
secure_erase(K, sizeof(K));
secure_erase(x_bytes.data(), x_bytes.size());
secure_erase(buf97, sizeof(buf97));
secure_erase(&hmac, sizeof(hmac));
return candidate;
}
@ -341,6 +342,7 @@ Scalar rfc6979_nonce(const Scalar& private_key,
std::memcpy(buf33, V, 32);
buf33[32] = 0x00;
hmac.compute_short(buf33, 33, K);
secure_erase(buf33, sizeof(buf33));
hmac.init_key32(K);
hmac.compute_short(V, 32, V);
}
@ -350,6 +352,7 @@ Scalar rfc6979_nonce(const Scalar& private_key,
secure_erase(K, sizeof(K));
secure_erase(x_bytes.data(), x_bytes.size());
secure_erase(buf97, sizeof(buf97));
secure_erase(&hmac, sizeof(hmac));
return Scalar::zero();
}
@ -410,12 +413,14 @@ Scalar rfc6979_nonce_hedged(const Scalar& private_key,
secure_erase(K, sizeof(K));
secure_erase(x_bytes.data(), x_bytes.size());
secure_erase(buf129, sizeof(buf129));
secure_erase(&hmac, sizeof(hmac));
return candidate;
}
uint8_t buf33[33];
std::memcpy(buf33, V, 32);
buf33[32] = 0x00;
hmac.compute_short(buf33, 33, K);
secure_erase(buf33, sizeof(buf33));
hmac.init_key32(K);
hmac.compute_short(V, 32, V);
}
@ -424,6 +429,7 @@ Scalar rfc6979_nonce_hedged(const Scalar& private_key,
secure_erase(K, sizeof(K));
secure_erase(x_bytes.data(), x_bytes.size());
secure_erase(buf129, sizeof(buf129));
secure_erase(&hmac, sizeof(hmac));
return Scalar::zero();
}

1
cpu/src/ecies.cpp
View File

@ -236,6 +236,7 @@ hmac_sha256(const std::uint8_t* key, std::size_t key_len,
secp256k1::detail::secure_erase(k_pad, sizeof(k_pad));
secp256k1::detail::secure_erase(ipad, sizeof(ipad));
secp256k1::detail::secure_erase(opad, sizeof(opad));
secp256k1::detail::secure_erase(inner_hash.data(), inner_hash.size());
return outer.finalize();
}

4
cpu/src/frost.cpp
View File

@ -33,7 +33,9 @@ static Scalar derive_scalar(const std::uint8_t* seed, std::size_t seed_len,
};
h.update(idx_be, 4);
auto hash = h.finalize();
return Scalar::from_bytes(hash);
auto result = Scalar::from_bytes(hash);
secure_erase(hash.data(), hash.size());
return result;
}
// Evaluate polynomial f(x) = a_0 + a_1*x + a_2*x^2 + ... at x

2
cpu/src/hkdf.cpp
View File

@ -129,6 +129,8 @@ bool hkdf_sha256_expand(
}
detail::secure_erase(t, sizeof(t));
detail::secure_erase(&ipad_base, sizeof(ipad_base));
detail::secure_erase(&opad_base, sizeof(opad_base));
return true;
}

9
cpu/src/musig2.cpp
View File

@ -193,6 +193,8 @@ std::pair<MuSig2SecNonce, MuSig2PubNonce> musig2_nonce_gen(
nonce_input[128] = 0x01;
auto k1_hash = tagged_hash("MuSig/nonce", nonce_input, 129);
sec.k1 = Scalar::from_bytes(k1_hash);
secure_erase(nonce_input, sizeof(nonce_input));
secure_erase(k1_hash.data(), k1_hash.size());
}
// k2 = tagged_hash("MuSig/nonce", t || pub_key || agg_pub_key || msg || 0x02)
@ -205,6 +207,8 @@ std::pair<MuSig2SecNonce, MuSig2PubNonce> musig2_nonce_gen(
nonce_input[128] = 0x02;
auto k2_hash = tagged_hash("MuSig/nonce", nonce_input, 129);
sec.k2 = Scalar::from_bytes(k2_hash);
secure_erase(nonce_input, sizeof(nonce_input));
secure_erase(k2_hash.data(), k2_hash.size());
}
// Zeroize secret key material now that nonces are derived
@ -306,6 +310,11 @@ Scalar musig2_partial_sign(
const MuSig2Session& session,
std::size_t signer_index) {
// Bounds check: signer_index must be valid for the key_coefficients vector
if (signer_index >= key_agg_ctx.key_coefficients.size()) {
return Scalar::zero();
}
// k = k1 + b * k2
Scalar k = sec_nonce.k1 + session.b * sec_nonce.k2;

12
cpu/src/schnorr.cpp
View File

@ -5,6 +5,7 @@
#include "secp256k1/config.hpp" // SECP256K1_FAST_52BIT
#include "secp256k1/field_52.hpp"
#include "secp256k1/debug_invariants.hpp"
#include "secp256k1/detail/secure_erase.hpp"
#include <cstring>
#include <string_view>
#if defined(_MSC_VER)
@ -309,6 +310,17 @@ SchnorrSignature schnorr_sign(const SchnorrKeypair& kp,
SchnorrSignature sig{};
sig.r = rx;
sig.s = k + e * kp.d;
// Erase all secret-derived stack buffers (matches ct::schnorr_sign cleanup)
detail::secure_erase(d_bytes.data(), d_bytes.size());
detail::secure_erase(t_hash.data(), t_hash.size());
detail::secure_erase(t, sizeof(t));
detail::secure_erase(nonce_input, sizeof(nonce_input));
detail::secure_erase(rand_hash.data(), rand_hash.size());
detail::secure_erase(challenge_input, sizeof(challenge_input));
detail::secure_erase(&k_prime, sizeof(k_prime));
detail::secure_erase(&k, sizeof(k));
return sig;
}

35
include/ufsecp/ufsecp_impl.cpp
View File

@ -2041,6 +2041,7 @@ ufsecp_error_t ufsecp_bip39_to_seed(ufsecp_ctx* ctx,
return ctx_set_err(ctx, UFSECP_ERR_BAD_INPUT, "invalid mnemonic");
}
std::memcpy(seed64_out, seed.data(), 64);
secp256k1::detail::secure_erase(seed.data(), seed.size());
return UFSECP_OK;
}
@ -2311,6 +2312,8 @@ ufsecp_error_t ufsecp_musig2_nonce_gen(ufsecp_ctx* ctx,
auto k2_bytes = sec.k2.to_bytes();
std::memcpy(secnonce_out, k1_bytes.data(), 32);
std::memcpy(secnonce_out + 32, k2_bytes.data(), 32);
secp256k1::detail::secure_erase(k1_bytes.data(), k1_bytes.size());
secp256k1::detail::secure_erase(k2_bytes.data(), k2_bytes.size());
/* Public nonce: R1(33) || R2(33) */
auto pn = pub.serialize();
std::memcpy(pubnonce_out, pn.data(), 66);
@ -2803,31 +2806,45 @@ ufsecp_error_t ufsecp_frost_sign(
std::memcpy(msg_arr.data(), msg32, 32);
std::vector<secp256k1::FrostNonceCommitment> ncs(n_signers);
size_t self_commitment_count = 0;
ufsecp_error_t nc_err = UFSECP_OK;
for (size_t i = 0; i < n_signers; ++i) {
const uint8_t* nc = nonce_commits + i * UFSECP_FROST_NONCE_COMMIT_LEN;
std::memcpy(&ncs[i].id, nc, 4);
if (ncs[i].id == 0 || ncs[i].id > kp.num_participants) {
return ctx_set_err(ctx, UFSECP_ERR_BAD_INPUT, "invalid nonce commitment signer");
nc_err = ctx_set_err(ctx, UFSECP_ERR_BAD_INPUT, "invalid nonce commitment signer");
break;
}
for (size_t j = 0; j < i; ++j) {
if (ncs[j].id == ncs[i].id) {
return ctx_set_err(ctx, UFSECP_ERR_BAD_INPUT, "duplicate nonce commitment signer");
nc_err = ctx_set_err(ctx, UFSECP_ERR_BAD_INPUT, "duplicate nonce commitment signer");
break;
}
}
if (nc_err != UFSECP_OK) break;
if (ncs[i].id == kp.id) {
++self_commitment_count;
}
ncs[i].hiding_point = point_from_compressed(nc + 4);
if (ncs[i].hiding_point.is_infinity()) {
return ctx_set_err(ctx, UFSECP_ERR_BAD_INPUT, "invalid hiding nonce point");
nc_err = ctx_set_err(ctx, UFSECP_ERR_BAD_INPUT, "invalid hiding nonce point");
break;
}
ncs[i].binding_point = point_from_compressed(nc + 37);
if (ncs[i].binding_point.is_infinity()) {
return ctx_set_err(ctx, UFSECP_ERR_BAD_INPUT, "invalid binding nonce point");
nc_err = ctx_set_err(ctx, UFSECP_ERR_BAD_INPUT, "invalid binding nonce point");
break;
}
}
if (self_commitment_count != 1) {
return ctx_set_err(ctx, UFSECP_ERR_BAD_INPUT, "missing signer nonce commitment");
if (nc_err == UFSECP_OK && self_commitment_count != 1) {
nc_err = ctx_set_err(ctx, UFSECP_ERR_BAD_INPUT, "missing signer nonce commitment");
}
if (nc_err != UFSECP_OK) {
secp256k1::detail::secure_erase(&kp.signing_share, sizeof(kp.signing_share));
secp256k1::detail::secure_erase(&fn.hiding_nonce, sizeof(fn.hiding_nonce));
secp256k1::detail::secure_erase(&fn.binding_nonce, sizeof(fn.binding_nonce));
secp256k1::detail::secure_erase(&h, sizeof(h));
secp256k1::detail::secure_erase(&b, sizeof(b));
return nc_err;
}
auto psig = secp256k1::frost_sign(kp, fn, msg_arr, ncs);
secp256k1::detail::secure_erase(&kp.signing_share, sizeof(kp.signing_share));
@ -3950,9 +3967,15 @@ ufsecp_error_t ufsecp_silent_payment_scan(
if (found_privkeys_out) {
auto key_bytes = results[i].second.to_bytes();
std::memcpy(found_privkeys_out + i * 32, key_bytes.data(), 32);
secp256k1::detail::secure_erase(key_bytes.data(), key_bytes.size());
}
}
// Erase result private keys from heap before vector destruction
for (auto& r : results) {
secp256k1::detail::secure_erase(&r.second, sizeof(r.second));
}
cleanup();
return UFSECP_OK;
}