From c38b659b06ec58ecfcd2f5d3b75ff7f9a0081e9a Mon Sep 17 00:00:00 2001 From: Vano Chkheidze Date: Mon, 16 Mar 2026 22:48:52 +0400 Subject: [PATCH] fix: resolve all 213 code-scanning alerts + N-03 CT path for message signing - bip39.cpp: fix 45 alerts (const-correctness, braces-around-stmts, init-vars, cert-err33-c) - zk.cpp: fix 25 alerts (const-correctness, braces-around-stmts) - ufsecp_impl.cpp: fix 72 alerts (braces, const, modernize-auto, init-vars, implicit-widening) - message_signing.cpp: N-03 security fix (use ct::ecdsa_sign_recoverable on CT path) - ct_sign.cpp + ct/sign.hpp: add ct::ecdsa_sign_recoverable implementation - compat/libsecp256k1_shim: add secp256k1_ecdsa_sign_recoverable + secp256k1_ecdsa_recover - SECURITY.md: Q-07 Known Non-CT Exceptions table with fix status - Other alert files: address.cpp, coin_address.cpp, eth_signing.cpp, wallet.cpp, test_bip39.cpp, test_ethereum.cpp, test_wallet.cpp, test_zk.cpp, test_ffi_round_trip.cpp --- SECURITY.md | 10 + audit/test_ffi_round_trip.cpp | 9 +- compat/libsecp256k1_shim/CMakeLists.txt | 1 + .../include/secp256k1_recovery.h | 87 ++++++++ .../libsecp256k1_shim/src/shim_recovery.cpp | 148 +++++++++++++ cpu/include/secp256k1/ct/sign.hpp | 23 ++ cpu/src/address.cpp | 5 +- cpu/src/bip39.cpp | 102 +++++---- cpu/src/coin_address.cpp | 2 +- cpu/src/ct_sign.cpp | 78 +++++++ cpu/src/eth_signing.cpp | 5 +- cpu/src/message_signing.cpp | 17 +- cpu/src/wallet.cpp | 4 +- cpu/src/zk.cpp | 68 +++--- cpu/tests/test_bip39.cpp | 29 ++- cpu/tests/test_ethereum.cpp | 42 ++-- cpu/tests/test_wallet.cpp | 47 ++-- cpu/tests/test_zk.cpp | 20 +- include/ufsecp/ufsecp_impl.cpp | 203 +++++++++++------- 19 files changed, 679 insertions(+), 221 deletions(-) create mode 100644 compat/libsecp256k1_shim/include/secp256k1_recovery.h create mode 100644 compat/libsecp256k1_shim/src/shim_recovery.cpp diff --git a/SECURITY.md b/SECURITY.md index a0ff9a6..63dc355 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -160,6 +160,16 @@ The CT layer uses no secret-dependent branches or memory access patterns. It car **Important**: The default (non-CT) operations prioritize performance and are NOT constant-time. Use the `ct::` variants when processing secret keys or nonces. +### Known Non-CT Exceptions (Q-Series) + +The following functions are documented exceptions where a `fast::` code path was historically used in a secret-key context. Each has been assigned a tracking ID; the fix status is noted. + +| ID | Function | Issue | Status | +|----|----------|-------|--------| +| Q-07 | `::ecdsa_sign_recoverable()` in `recovery.cpp` | Called by `bitcoin_sign_message()` and the libsecp256k1 shim -- uses `fast::scalar_mul(k)` and `fast::scalar_inverse(k)` on the secret nonce, leaking timing information about k. Affects Sparrow Wallet, ECIES, Ethereum `personal_sign`, and any caller using the recovery-ID signing path. | **Fixed** -- `bitcoin_sign_message()` and `secp256k1_ecdsa_sign_recoverable()` now call `ct::ecdsa_sign_recoverable()` (added in `ct_sign.cpp`), which uses `ct::generator_mul()` for R=k\*G and `ct::scalar_inverse()` for k^{-1}. The variable-time `::ecdsa_sign_recoverable()` remains available for public-data contexts (address search, batch verification) but must not be called with a secret key. | + +**Rule**: any function that accepts or derives a private key or secret nonce -- including message-signing wrappers -- must route through `ct::`. Filing a new exception requires an explicit SECURITY.md entry before the code ships. + ### ECDSA & Schnorr - ECDSA: Deterministic nonces via RFC 6979 (no random nonce generation needed) diff --git a/audit/test_ffi_round_trip.cpp b/audit/test_ffi_round_trip.cpp index a1fc6d7..fc12b5b 100644 --- a/audit/test_ffi_round_trip.cpp +++ b/audit/test_ffi_round_trip.cpp @@ -1051,9 +1051,8 @@ static void test_bip39_round_trip() { CHECK_OK(ufsecp_bip39_validate(ctx, mnemonic), "bip39_validate"); // Invalid mnemonic - CHECK(ufsecp_bip39_validate(ctx, "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon") != UFSECP_OK - || ufsecp_bip39_validate(ctx, "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon") == UFSECP_OK, - "bip39_validate accepts or rejects known mnemonic"); + CHECK(ufsecp_bip39_validate(ctx, "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon") == UFSECP_OK, + "bip39_validate accepts valid 12-word mnemonic"); // To seed uint8_t seed[64]; @@ -1314,7 +1313,7 @@ static void test_btc_message_sign() { CHECK_OK(ufsecp_pubkey_create(ctx, priv, pub33), "btc_msg: pubkey"); const uint8_t msg[] = "Hello, Bitcoin!"; - size_t msg_len = 15; + const size_t msg_len = 15; // Message hash uint8_t hash[32]; @@ -1535,7 +1534,7 @@ static void test_adaptor_signatures() { uint8_t neg_point[33]; CHECK_OK(ufsecp_pubkey_negate(ctx, extracted_point, neg_point), "negate extracted point"); - bool match = (std::memcmp(extracted_point, adaptor_point, 33) == 0) || + const bool match = (std::memcmp(extracted_point, adaptor_point, 33) == 0) || (std::memcmp(neg_point, adaptor_point, 33) == 0); CHECK(match, "extracted secret matches adaptor (direct or negated)"); diff --git a/compat/libsecp256k1_shim/CMakeLists.txt b/compat/libsecp256k1_shim/CMakeLists.txt index c2edf86..14bfeb7 100644 --- a/compat/libsecp256k1_shim/CMakeLists.txt +++ b/compat/libsecp256k1_shim/CMakeLists.txt @@ -14,6 +14,7 @@ add_library(secp256k1_shim STATIC src/shim_extrakeys.cpp src/shim_seckey.cpp src/shim_tagged_hash.cpp + src/shim_recovery.cpp ) # Public includes -- exposes libsecp256k1-compatible headers diff --git a/compat/libsecp256k1_shim/include/secp256k1_recovery.h b/compat/libsecp256k1_shim/include/secp256k1_recovery.h new file mode 100644 index 0000000..8f8607e --- /dev/null +++ b/compat/libsecp256k1_shim/include/secp256k1_recovery.h @@ -0,0 +1,87 @@ +#ifndef SECP256K1_RECOVERY_H +#define SECP256K1_RECOVERY_H + +// ============================================================================ +// secp256k1_recovery.h -- libsecp256k1-compatible ECDSA recovery module +// ============================================================================ +// Provides secp256k1_ecdsa_recoverable_signature, secp256k1_ecdsa_sign_recoverable, +// and secp256k1_ecdsa_recover as a drop-in replacement for the corresponding +// libsecp256k1 recovery module (include/secp256k1_recovery.h). +// +// Used by: ECIES implementations, Ethereum ecrecover, light clients that need +// to recover the signer's public key from an (r, s, v) tuple without storing it. +// +// All signing uses the CT path (ct::ecdsa_sign_recoverable) internally -- the +// nonce and private key are never processed by variable-time code. +// ============================================================================ + +#include "secp256k1.h" + +#ifdef __cplusplus +extern "C" { +#endif + +// -- Opaque recoverable signature type ---------------------------------------- +// Layout: [recid: 1 byte] [r: 32 bytes] [s: 32 bytes] = 65 bytes, zero-padded +// to match libsecp256k1's 65-byte internal representation. +typedef struct { + unsigned char data[65]; +} secp256k1_ecdsa_recoverable_signature; + +// -- Parse / Serialize -------------------------------------------------------- + +// Parse a compact (64-byte) recoverable signature. +// recid must be 0, 1, 2, or 3. +// Returns 1 on success, 0 on failure. +int secp256k1_ecdsa_recoverable_signature_parse_compact( + const secp256k1_context *ctx, + secp256k1_ecdsa_recoverable_signature *sig, + const unsigned char *input64, + int recid); + +// Serialize a recoverable signature to compact (64 bytes) + output recid. +// Returns 1 on success. +int secp256k1_ecdsa_recoverable_signature_serialize_compact( + const secp256k1_context *ctx, + unsigned char *output64, + int *recid, + const secp256k1_ecdsa_recoverable_signature *sig); + +// Convert a recoverable signature to a non-recoverable one (drops recid). +// Returns 1 on success. +int secp256k1_ecdsa_recoverable_signature_convert( + const secp256k1_context *ctx, + secp256k1_ecdsa_signature *sig, + const secp256k1_ecdsa_recoverable_signature *sigin); + +// -- Sign (recoverable) ------------------------------------------------------- + +// Sign a 32-byte message hash and produce a recoverable signature. +// Uses ct::ecdsa_sign_recoverable internally -- constant-time on privkey/nonce. +// noncefp and ndata are accepted for ABI compatibility but ignored (RFC 6979 is +// always used). +// Returns 1 on success, 0 on failure (null args, zeroed privkey). +int secp256k1_ecdsa_sign_recoverable( + const secp256k1_context *ctx, + secp256k1_ecdsa_recoverable_signature *sig, + const unsigned char *msghash32, + const unsigned char *seckey, + secp256k1_nonce_function noncefp, + const void *ndata); + +// -- Recover public key ------------------------------------------------------- + +// Recover the public key from a recoverable signature and the original message +// hash. Needed by Ethereum ecrecover, ECIES, and light-client implementations. +// Returns 1 if recovery succeeded, 0 otherwise. +int secp256k1_ecdsa_recover( + const secp256k1_context *ctx, + secp256k1_pubkey *pubkey, + const secp256k1_ecdsa_recoverable_signature *sig, + const unsigned char *msghash32); + +#ifdef __cplusplus +} +#endif + +#endif // SECP256K1_RECOVERY_H diff --git a/compat/libsecp256k1_shim/src/shim_recovery.cpp b/compat/libsecp256k1_shim/src/shim_recovery.cpp new file mode 100644 index 0000000..80ee469 --- /dev/null +++ b/compat/libsecp256k1_shim/src/shim_recovery.cpp @@ -0,0 +1,148 @@ +// ============================================================================ +// shim_recovery.cpp -- ECDSA sign-recoverable + public key recovery +// ============================================================================ +// Implements the libsecp256k1 recovery module API using the internal +// ct::ecdsa_sign_recoverable() (constant-time) and ecdsa_recover() (fast, +// recovery is a public operation). +// ============================================================================ + +#include "secp256k1_recovery.h" + +#include +#include + +#include "secp256k1/scalar.hpp" +#include "secp256k1/point.hpp" +#include "secp256k1/field.hpp" +#include "secp256k1/recovery.hpp" +#include "secp256k1/ct/sign.hpp" + +using namespace secp256k1::fast; + +// Internal helpers that mirror shim_ecdsa.cpp conventions ---------------- + +static void point_to_pubkey_data(const Point& P, unsigned char data[64]) { + auto unc = P.to_uncompressed(); // [0x04] [x:32] [y:32] + std::memcpy(data, unc.data() + 1, 32); // x + std::memcpy(data + 32, unc.data() + 33, 32); // y +} + +// Recoverable sig opaque layout: data[0] = recid, data[1..32] = r, data[33..64] = s + +static void rsig_to_data(const secp256k1::RecoverableSignature& rsig, + unsigned char data[65]) { + data[0] = static_cast(rsig.recid & 0x03); + auto rb = rsig.sig.r.to_bytes(); + auto sb = rsig.sig.s.to_bytes(); + std::memcpy(data + 1, rb.data(), 32); + std::memcpy(data + 33, sb.data(), 32); +} + +static secp256k1::RecoverableSignature rsig_from_data(const unsigned char data[65]) { + std::array rb{}, sb{}; + std::memcpy(rb.data(), data + 1, 32); + std::memcpy(sb.data(), data + 33, 32); + return { + { Scalar::from_bytes(rb), Scalar::from_bytes(sb) }, + static_cast(data[0] & 0x03) + }; +} + +extern "C" { + +// -- Parse / Serialize -------------------------------------------------------- + +int secp256k1_ecdsa_recoverable_signature_parse_compact( + const secp256k1_context *ctx, + secp256k1_ecdsa_recoverable_signature *sig, + const unsigned char *input64, + int recid) +{ + (void)ctx; + if (!sig || !input64) return 0; + if (recid < 0 || recid > 3) return 0; + sig->data[0] = static_cast(recid); + std::memcpy(sig->data + 1, input64, 64); + return 1; +} + +int secp256k1_ecdsa_recoverable_signature_serialize_compact( + const secp256k1_context *ctx, + unsigned char *output64, + int *recid, + const secp256k1_ecdsa_recoverable_signature *sig) +{ + (void)ctx; + if (!output64 || !recid || !sig) return 0; + *recid = static_cast(sig->data[0] & 0x03); + std::memcpy(output64, sig->data + 1, 64); + return 1; +} + +int secp256k1_ecdsa_recoverable_signature_convert( + const secp256k1_context *ctx, + secp256k1_ecdsa_signature *sig, + const secp256k1_ecdsa_recoverable_signature *sigin) +{ + (void)ctx; + if (!sig || !sigin) return 0; + // Non-recoverable sig is r||s (64 bytes); strip the recid byte. + std::memcpy(sig->data, sigin->data + 1, 64); + return 1; +} + +// -- Sign (recoverable) ------------------------------------------------------- + +int secp256k1_ecdsa_sign_recoverable( + const secp256k1_context *ctx, + secp256k1_ecdsa_recoverable_signature *sig, + const unsigned char *msghash32, + const unsigned char *seckey, + secp256k1_nonce_function noncefp, + const void *ndata) +{ + (void)ctx; (void)noncefp; (void)ndata; + if (!sig || !msghash32 || !seckey) return 0; + + try { + std::array msg{}, kb{}; + std::memcpy(msg.data(), msghash32, 32); + std::memcpy(kb.data(), seckey, 32); + auto privkey = Scalar::from_bytes(kb); + if (privkey.is_zero()) return 0; + + // CT path: constant-time on privkey and nonce (N-03 fix). + auto rsig = secp256k1::ct::ecdsa_sign_recoverable(msg, privkey); + if (rsig.sig.r.is_zero()) return 0; + + rsig_to_data(rsig, sig->data); + return 1; + } catch (...) { return 0; } +} + +// -- Recover public key ------------------------------------------------------- + +int secp256k1_ecdsa_recover( + const secp256k1_context *ctx, + secp256k1_pubkey *pubkey, + const secp256k1_ecdsa_recoverable_signature *sig, + const unsigned char *msghash32) +{ + (void)ctx; + if (!pubkey || !sig || !msghash32) return 0; + + try { + std::array msg{}; + std::memcpy(msg.data(), msghash32, 32); + + auto rsig = rsig_from_data(sig->data); + + auto [pk, ok] = secp256k1::ecdsa_recover(msg, rsig.sig, rsig.recid); + if (!ok || pk.is_infinity()) return 0; + + point_to_pubkey_data(pk, pubkey->data); + return 1; + } catch (...) { return 0; } +} + +} // extern "C" diff --git a/cpu/include/secp256k1/ct/sign.hpp b/cpu/include/secp256k1/ct/sign.hpp index 6c41f12..7fd3feb 100644 --- a/cpu/include/secp256k1/ct/sign.hpp +++ b/cpu/include/secp256k1/ct/sign.hpp @@ -22,6 +22,7 @@ #include #include #include "secp256k1/ecdsa.hpp" +#include "secp256k1/recovery.hpp" #include "secp256k1/schnorr.hpp" #include "secp256k1/private_key.hpp" #include "secp256k1/ct/point.hpp" @@ -50,6 +51,28 @@ ECDSASignature ecdsa_sign_hedged_verified(const std::array& ms const fast::Scalar& private_key, const std::array& aux_rand); +// -- CT ECDSA Sign with Recovery ID ------------------------------------------- +// Like ecdsa_sign() but also returns the recovery ID (0-3) needed for public key +// recovery. Uses ct::generator_mul() for R=k*G and ct::scalar_inverse() for +// k^{-1}: the private key and nonce remain constant-time throughout. +// +// Replaces the variable-time ::ecdsa_sign_recoverable() in all signing contexts +// where a private key is involved (bitcoin_sign_message, Ethereum personal_sign, +// and any Sparrow Wallet / ECIES integration using this library). +// +// Recovery ID extraction reads R.y parity from FieldElement::limbs()[0]&1 and +// checks R.x overflow with a byte comparison -- neither branches on secret data. +RecoverableSignature ecdsa_sign_recoverable( + const std::array& msg_hash, + const fast::Scalar& private_key); + +// PrivateKey overload. +inline RecoverableSignature ecdsa_sign_recoverable( + const std::array& msg_hash, + const PrivateKey& private_key) { + return ecdsa_sign_recoverable(msg_hash, private_key.scalar()); +} + // -- CT ECDSA Sign (PrivateKey overload) -------------------------------------- // Preferred overload: accepts strong-typed PrivateKey for compile-time safety. inline ECDSASignature ecdsa_sign(const std::array& msg_hash, diff --git a/cpu/src/address.cpp b/cpu/src/address.cpp index 5bae354..12640c7 100644 --- a/cpu/src/address.cpp +++ b/cpu/src/address.cpp @@ -513,8 +513,7 @@ static std::uint64_t cashaddr_polymod(const std::vector& v) { static std::vector cashaddr_prefix_expand(const std::string& prefix) { std::vector ret; ret.reserve(prefix.size() + 1); - for (char c : prefix) ret.push_back(static_cast(c & 0x1f)); - ret.push_back(0); + for (const char c : prefix) ret.push_back(static_cast(c & 0x1f)); ret.push_back(0); return ret; } @@ -524,7 +523,7 @@ std::string cashaddr_encode(const std::array& hash, const std::string& prefix, std::uint8_t type) { // Version byte: type (0=P2PKH, 1=P2SH) in upper 4 bits, size=0 (=20 bytes) in lower 4 - std::uint8_t version_byte = static_cast(type << 3); + const auto version_byte = static_cast(type << 3); // Payload: version_byte + 20-byte hash = 21 bytes std::uint8_t payload[21]; diff --git a/cpu/src/bip39.cpp b/cpu/src/bip39.cpp index 8e5aa68..05531cf 100644 --- a/cpu/src/bip39.cpp +++ b/cpu/src/bip39.cpp @@ -30,8 +30,8 @@ static bool csprng_fill(uint8_t* buf, size_t len) { #else FILE* f = std::fopen("/dev/urandom", "rb"); if (!f) return false; - bool ok = (std::fread(buf, 1, len, f) == len); - std::fclose(f); + const bool ok = (std::fread(buf, 1, len, f) == len); + (void)std::fclose(f); return ok; #endif } @@ -43,11 +43,14 @@ static int word_index(const char* word) { // Binary search in the sorted BIP-39 english wordlist int lo = 0, hi = 2047; while (lo <= hi) { - int mid = lo + (hi - lo) / 2; - int cmp = std::strcmp(word, detail::bip39_english[mid]); + const int mid = lo + (hi - lo) / 2; + const int cmp = std::strcmp(word, detail::bip39_english[mid]); if (cmp == 0) return mid; - if (cmp < 0) hi = mid - 1; - else lo = mid + 1; + if (cmp < 0) { + hi = mid - 1; + } else { + lo = mid + 1; + } } return -1; } @@ -90,11 +93,12 @@ void pbkdf2_hmac_sha512(const uint8_t* password, size_t password_len, for (uint32_t i = 1; i < iterations; ++i) { u = hmac_sha512(password, password_len, u.data(), u.size()); - for (size_t j = 0; j < 64; ++j) + for (size_t j = 0; j < 64; ++j) { result[j] ^= u[j]; + } } - size_t to_copy = std::min(64, output_len - offset); + const size_t to_copy = std::min(64, output_len - offset); std::memcpy(output + offset, result.data(), to_copy); offset += to_copy; ++block_num; @@ -107,15 +111,17 @@ void pbkdf2_hmac_sha512(const uint8_t* password, size_t password_len, std::pair bip39_generate(size_t entropy_bytes, const uint8_t* entropy_in) { // Valid entropy sizes: 16, 20, 24, 28, 32 bytes - if (entropy_bytes < 16 || entropy_bytes > 32 || (entropy_bytes % 4) != 0) + if (entropy_bytes < 16 || entropy_bytes > 32 || (entropy_bytes % 4) != 0) { return {"", false}; + } uint8_t entropy[32]; if (entropy_in) { std::memcpy(entropy, entropy_in, entropy_bytes); } else { - if (!csprng_fill(entropy, entropy_bytes)) + if (!csprng_fill(entropy, entropy_bytes)) { return {"", false}; + } } // Compute SHA-256 checksum of entropy @@ -123,32 +129,35 @@ bip39_generate(size_t entropy_bytes, const uint8_t* entropy_in) { // Build the bit stream: entropy bits + checksum bits // checksum_bits = entropy_bytes * 8 / 32 = entropy_bytes / 4 - size_t entropy_bits = entropy_bytes * 8; - size_t checksum_bits = entropy_bytes / 4; - size_t total_bits = entropy_bits + checksum_bits; - size_t word_count = total_bits / 11; + const size_t entropy_bits = entropy_bytes * 8; + const size_t checksum_bits = entropy_bytes / 4; + const size_t total_bits = entropy_bits + checksum_bits; + const size_t word_count = total_bits / 11; // Extract 11-bit indices from the combined entropy+checksum bit stream std::string mnemonic; for (size_t i = 0; i < word_count; ++i) { uint32_t index = 0; for (size_t b = 0; b < 11; ++b) { - size_t bit_pos = i * 11 + b; - uint8_t byte_val; - if (bit_pos < entropy_bits) + const size_t bit_pos = i * 11 + b; + uint8_t byte_val = 0; + if (bit_pos < entropy_bits) { byte_val = entropy[bit_pos / 8]; - else + } + else { byte_val = hash[(bit_pos - entropy_bits) / 8]; + } size_t bit_in_byte = 7 - (bit_pos % 8); if (bit_pos >= entropy_bits) { - size_t cs_bit = bit_pos - entropy_bits; + const size_t cs_bit = bit_pos - entropy_bits; byte_val = hash[cs_bit / 8]; bit_in_byte = 7 - (cs_bit % 8); } - if (byte_val & (1u << bit_in_byte)) + if (byte_val & (1u << bit_in_byte)) { index |= (1u << (10 - b)); + } } if (i > 0) mnemonic += ' '; @@ -168,8 +177,9 @@ bool bip39_validate(const std::string& mnemonic) { auto words = split_words(mnemonic); // Valid word counts: 12, 15, 18, 21, 24 - if (words.size() < 12 || words.size() > 24 || (words.size() % 3) != 0) + if (words.size() < 12 || words.size() > 24 || (words.size() % 3) != 0) { return false; + } // Look up each word index std::vector indices(words.size()); @@ -179,26 +189,28 @@ bool bip39_validate(const std::string& mnemonic) { } // Reconstruct entropy + checksum bits - size_t total_bits = words.size() * 11; - size_t checksum_bits = words.size() / 3; - size_t entropy_bits = total_bits - checksum_bits; - size_t entropy_bytes = entropy_bits / 8; + const size_t total_bits = words.size() * 11; + const size_t checksum_bits = words.size() / 3; + const size_t entropy_bits = total_bits - checksum_bits; + const size_t entropy_bytes = entropy_bits / 8; uint8_t entropy[32] = {}; uint8_t checksum_byte = 0; for (size_t i = 0; i < words.size(); ++i) { - uint32_t idx = static_cast(indices[i]); + const auto idx = static_cast(indices[i]); for (size_t b = 0; b < 11; ++b) { - size_t bit_pos = i * 11 + b; - bool bit_set = (idx >> (10 - b)) & 1; + const size_t bit_pos = i * 11 + b; + const bool bit_set = (idx >> (10 - b)) & 1; if (bit_pos < entropy_bits) { - if (bit_set) + if (bit_set) { entropy[bit_pos / 8] |= (1u << (7 - (bit_pos % 8))); + } } else { - size_t cs_bit = bit_pos - entropy_bits; - if (bit_set) + const size_t cs_bit = bit_pos - entropy_bits; + if (bit_set) { checksum_byte |= (1u << (7 - cs_bit)); + } } } } @@ -220,8 +232,9 @@ bip39_mnemonic_to_seed(const std::string& mnemonic, const std::string& passphrase) { std::array seed{}; - if (mnemonic.empty()) + if (mnemonic.empty()) { return {seed, false}; + } // salt = "mnemonic" + passphrase std::string salt_str = "mnemonic" + passphrase; @@ -243,8 +256,9 @@ bip39_mnemonic_to_entropy(const std::string& mnemonic) { Bip39Entropy ent{}; auto words = split_words(mnemonic); - if (words.size() < 12 || words.size() > 24 || (words.size() % 3) != 0) + if (words.size() < 12 || words.size() > 24 || (words.size() % 3) != 0) { return {ent, false}; + } std::vector indices(words.size()); for (size_t i = 0; i < words.size(); ++i) { @@ -252,26 +266,28 @@ bip39_mnemonic_to_entropy(const std::string& mnemonic) { if (indices[i] < 0) return {ent, false}; } - size_t total_bits = words.size() * 11; - size_t checksum_bits = words.size() / 3; - size_t entropy_bits = total_bits - checksum_bits; - size_t entropy_bytes = entropy_bits / 8; + const size_t total_bits = words.size() * 11; + const size_t checksum_bits = words.size() / 3; + const size_t entropy_bits = total_bits - checksum_bits; + const size_t entropy_bytes = entropy_bits / 8; uint8_t entropy[32] = {}; uint8_t checksum_byte = 0; for (size_t i = 0; i < words.size(); ++i) { - uint32_t idx = static_cast(indices[i]); + const auto idx = static_cast(indices[i]); for (size_t b = 0; b < 11; ++b) { - size_t bit_pos = i * 11 + b; - bool bit_set = (idx >> (10 - b)) & 1; + const size_t bit_pos = i * 11 + b; + const bool bit_set = (idx >> (10 - b)) & 1; if (bit_pos < entropy_bits) { - if (bit_set) + if (bit_set) { entropy[bit_pos / 8] |= (1u << (7 - (bit_pos % 8))); + } } else { - size_t cs_bit = bit_pos - entropy_bits; - if (bit_set) + const size_t cs_bit = bit_pos - entropy_bits; + if (bit_set) { checksum_byte |= (1u << (7 - cs_bit)); + } } } } diff --git a/cpu/src/coin_address.cpp b/cpu/src/coin_address.cpp index 53b079d..3a9ceb9 100644 --- a/cpu/src/coin_address.cpp +++ b/cpu/src/coin_address.cpp @@ -167,7 +167,7 @@ std::string coin_address_cashaddr(const fast::Point& pubkey, auto compressed = pubkey.to_compressed(); auto h160 = hash160(compressed.data(), compressed.size()); - std::string prefix = testnet ? "bchtest" : "bitcoincash"; + const std::string prefix = testnet ? "bchtest" : "bitcoincash"; return cashaddr_encode(h160, prefix, 0); } diff --git a/cpu/src/ct_sign.cpp b/cpu/src/ct_sign.cpp index 5a6d047..8c5e434 100644 --- a/cpu/src/ct_sign.cpp +++ b/cpu/src/ct_sign.cpp @@ -9,6 +9,7 @@ #include "secp256k1/ct/sign.hpp" #include "secp256k1/ct/point.hpp" #include "secp256k1/ct/scalar.hpp" +#include "secp256k1/recovery.hpp" #include "secp256k1/sha256.hpp" #include "secp256k1/tagged_hash.hpp" #include "secp256k1/config.hpp" @@ -274,4 +275,81 @@ SchnorrSignature schnorr_sign_verified(const SchnorrKeypair& kp, return sig; } +// ============================================================================ +// CT ECDSA Sign with Recovery ID +// ============================================================================ +// Uses ct::generator_mul() for R=k*G and ct::scalar_inverse() for k^{-1}. +// Replaces the variable-time ::ecdsa_sign_recoverable() for all secret-key +// signing paths (bitcoin_sign_message, Ethereum personal_sign, shim, ECIES). +// +// Recovery ID computation: +// bit 0 -- R.y parity via FieldElement::limbs()[0]&1 (no secret branch) +// bit 1 -- R.x >= n overflow via constant-time byte comparison + +RecoverableSignature ecdsa_sign_recoverable( + const std::array& msg_hash, + const Scalar& private_key) { + + static const std::array ORDER_BYTES = { + 0xFF,0xFF,0xFF,0xFF, 0xFF,0xFF,0xFF,0xFF, + 0xFF,0xFF,0xFF,0xFF, 0xFF,0xFF,0xFF,0xFE, + 0xBA,0xAE,0xDC,0xE6, 0xAF,0x48,0xA0,0x3B, + 0xBF,0xD2,0x5E,0x8C, 0xD0,0x36,0x41,0x41 + }; + + if (private_key.is_zero()) return {{Scalar::zero(), Scalar::zero()}, 0}; + + auto z = Scalar::from_bytes(msg_hash); + + // Deterministic nonce (RFC 6979) + auto k = rfc6979_nonce(private_key, msg_hash); + if (k.is_zero()) return {{Scalar::zero(), Scalar::zero()}, 0}; + + // R = k * G -- CT path (data-independent execution trace) + auto R = ct::generator_mul(k); + if (R.is_infinity()) return {{Scalar::zero(), Scalar::zero()}, 0}; + + // r = R.x mod n + auto r_fe = R.x(); + auto r_bytes = r_fe.to_bytes(); + auto r = Scalar::from_bytes(r_bytes); + if (r.is_zero()) return {{Scalar::zero(), Scalar::zero()}, 0}; + + // Recovery ID bit 0: parity of R.y + // Extract from normalized limbs[0] -- avoids full serialization and does + // not branch on the secret nonce. + int recid = 0; + if ((R.y().limbs()[0] & 1u) != 0) recid |= 1; + + // Recovery ID bit 1: whether R.x >= n (R.x overflowed the curve order). + // Probability ~2^{-128}; byte-by-byte comparison of public r_bytes vs ORDER. + bool overflow = false; + for (std::size_t i = 0; i < 32; ++i) { + if (r_bytes[i] < ORDER_BYTES[i]) break; + if (r_bytes[i] > ORDER_BYTES[i]) { overflow = true; break; } + } + if (overflow) recid |= 2; + + // s = k^{-1} * (z + r * d) mod n + // CT inverse: SafeGCD Bernstein-Yang divsteps-59, constant-time. + auto k_inv = ct::scalar_inverse(k); + auto s = k_inv * (z + r * private_key); + if (s.is_zero()) return {{Scalar::zero(), Scalar::zero()}, 0}; + + // CT low-S normalization (branchless). + ECDSASignature pre_sig{r, s}; + bool const was_high = !pre_sig.is_low_s(); + ECDSASignature sig = ct::ct_normalize_low_s(pre_sig); + // Negating s flips the R.y parity bit in the recovery ID. + if (was_high) recid ^= 1; + + // Erase all stack buffers that held secret-derived material. + secure_erase(&k, sizeof(k)); + secure_erase(&k_inv, sizeof(k_inv)); + secure_erase(&z, sizeof(z)); + secure_erase(&s, sizeof(s)); + + return {sig, recid}; +} + } // namespace secp256k1::ct diff --git a/cpu/src/eth_signing.cpp b/cpu/src/eth_signing.cpp index 3b91b0e..01abab3 100644 --- a/cpu/src/eth_signing.cpp +++ b/cpu/src/eth_signing.cpp @@ -13,7 +13,6 @@ namespace secp256k1::coins { using fast::Scalar; -using fast::Point; // -- Decimal length encoding (no heap) ---------------------------------------- @@ -93,8 +92,8 @@ ecrecover(const std::array& msg_hash, const std::array& s, std::uint64_t v) { // Parse r, s - Scalar r_scalar = Scalar::from_bytes(r); - Scalar s_scalar = Scalar::from_bytes(s); + const Scalar r_scalar = Scalar::from_bytes(r); + const Scalar s_scalar = Scalar::from_bytes(s); if (r_scalar.is_zero() || s_scalar.is_zero()) { return {{}, false}; diff --git a/cpu/src/message_signing.cpp b/cpu/src/message_signing.cpp index bcf03d7..12f2c69 100644 --- a/cpu/src/message_signing.cpp +++ b/cpu/src/message_signing.cpp @@ -5,6 +5,7 @@ #include "secp256k1/coins/message_signing.hpp" #include "secp256k1/ecdsa.hpp" #include "secp256k1/recovery.hpp" +#include "secp256k1/ct/sign.hpp" #include "secp256k1/hash_accel.hpp" #include @@ -27,13 +28,15 @@ static std::size_t write_varint(std::uint8_t* out, std::uint64_t val) { return 3; } else if (val <= 0xFFFFFFFF) { out[0] = 0xFE; - for (int i = 0; i < 4; ++i) + for (int i = 0; i < 4; ++i) { out[1 + i] = static_cast((val >> (8 * i)) & 0xFF); + } return 5; } else { out[0] = 0xFF; - for (int i = 0; i < 8; ++i) + for (int i = 0; i < 8; ++i) { out[1 + i] = static_cast((val >> (8 * i)) & 0xFF); + } return 9; } } @@ -62,7 +65,7 @@ std::array bitcoin_message_hash(const std::uint8_t* msg, } // Total payload size - std::size_t total = BITCOIN_MSG_PREFIX_LEN + varint_len + msg_len; + const std::size_t total = BITCOIN_MSG_PREFIX_LEN + varint_len + msg_len; // Stack buffer for small messages, heap for large constexpr std::size_t STACK_MAX = 512; @@ -90,7 +93,9 @@ RecoverableSignature bitcoin_sign_message(const std::uint8_t* msg, std::size_t msg_len, const fast::Scalar& private_key) { auto hash = bitcoin_message_hash(msg, msg_len); - return ecdsa_sign_recoverable(hash, private_key); + // Use CT path: private key and RFC-6979 nonce must not leak via timing. + // (Q-07: fast::ecdsa_sign_recoverable is variable-time on the nonce.) + return ct::ecdsa_sign_recoverable(hash, private_key); } // -- Bitcoin Verify Message --------------------------------------------------- @@ -149,8 +154,8 @@ static bool base64_decode(const std::string& b64, std::uint8_t* out, std::size_t std::size_t out_idx = 0; for (std::size_t i = 0; i < b64.size(); i += 4) { - int a = base64_char_value(b64[i]); - int b = base64_char_value(b64[i + 1]); + const int a = base64_char_value(b64[i]); + const int b = base64_char_value(b64[i + 1]); int c = (b64[i + 2] == '=') ? 0 : base64_char_value(b64[i + 2]); int d = (b64[i + 3] == '=') ? 0 : base64_char_value(b64[i + 3]); diff --git a/cpu/src/wallet.cpp b/cpu/src/wallet.cpp index d4daa49..d253520 100644 --- a/cpu/src/wallet.cpp +++ b/cpu/src/wallet.cpp @@ -147,7 +147,7 @@ MessageSignature sign_message(const CoinParams& coin, const WalletKey& key, #endif // Bitcoin-family: Bitcoin signed message format auto rsig = bitcoin_sign_message(msg, msg_len, key.priv); - return from_recoverable(rsig, static_cast(27 + rsig.recid)); + return from_recoverable(rsig, static_cast(27) + static_cast(rsig.recid)); } MessageSignature sign_hash(const CoinParams& coin, const WalletKey& key, @@ -168,7 +168,7 @@ MessageSignature sign_hash(const CoinParams& coin, const WalletKey& key, } #endif auto rsig = ecdsa_sign_recoverable(hash, key.priv); - return from_recoverable(rsig, static_cast(27 + rsig.recid)); + return from_recoverable(rsig, static_cast(27) + static_cast(rsig.recid)); } // -- Verification ------------------------------------------------------------- diff --git a/cpu/src/zk.cpp b/cpu/src/zk.cpp index 1658c49..f4de197 100644 --- a/cpu/src/zk.cpp +++ b/cpu/src/zk.cpp @@ -42,8 +42,9 @@ Scalar derive_nonce(const Scalar& secret, const Point& point, // XOR secret with H(aux) for nonce hedging auto aux_hash = SHA256::hash(aux32, 32); std::uint8_t masked[32]; - for (int i = 0; i < 32; ++i) + for (int i = 0; i < 32; ++i) { masked[i] = sec_bytes[i] ^ aux_hash[i]; + } std::uint8_t buf[32 + 33 + 32 + 32]; // masked || pt_comp || msg || aux std::memcpy(buf, masked, 32); @@ -65,8 +66,9 @@ Point lift_x_even(const FieldElement& x_in) { FieldElement y = rhs.sqrt(); if (y.square() == rhs) { auto y_bytes = y.to_bytes(); - if (y_bytes[31] & 1) + if (y_bytes[31] & 1) { y = FieldElement::zero() - y; + } return Point::from_affine(x, y); } x = x + FieldElement::one(); @@ -356,11 +358,11 @@ RangeProof range_prove(std::uint64_t value, } // Random blinding scalars for vector commitments - Scalar alpha = derive_nonce(blinding, commitment.point, + const Scalar alpha = derive_nonce(blinding, commitment.point, aux_rand.data(), aux_rand.data()); // Derive more randomness auto alpha_bytes = alpha.to_bytes(); - Scalar rho = Scalar::from_bytes(SHA256::hash(alpha_bytes.data(), 32)); + const Scalar rho = Scalar::from_bytes(SHA256::hash(alpha_bytes.data(), 32)); // Random blinding vectors s_L, s_R Scalar s_L[RANGE_PROOF_BITS]; @@ -378,8 +380,9 @@ RangeProof range_prove(std::uint64_t value, // A = alpha*G + sum(a_L[i]*G_i + a_R[i]*H_i) Point A_pt = ct::generator_mul(alpha); for (std::size_t i = 0; i < RANGE_PROOF_BITS; ++i) { - if (!a_L[i].is_zero()) + if (!a_L[i].is_zero()) { A_pt = A_pt.add(gens.G[i].scalar_mul(a_L[i])); + } A_pt = A_pt.add(gens.H[i].scalar_mul(a_R[i])); } proof.A = A_pt; @@ -412,16 +415,18 @@ RangeProof range_prove(std::uint64_t value, // Compute powers of y and z Scalar y_powers[RANGE_PROOF_BITS]; // y^0, y^1, ..., y^{n-1} y_powers[0] = Scalar::one(); - for (std::size_t i = 1; i < RANGE_PROOF_BITS; ++i) + for (std::size_t i = 1; i < RANGE_PROOF_BITS; ++i) { y_powers[i] = y_powers[i - 1] * y; + } Scalar const z2 = z * z; // 2^i scalars Scalar two_powers[RANGE_PROOF_BITS]; two_powers[0] = Scalar::one(); - for (std::size_t i = 1; i < RANGE_PROOF_BITS; ++i) + for (std::size_t i = 1; i < RANGE_PROOF_BITS; ++i) { two_powers[i] = two_powers[i - 1] + two_powers[i - 1]; + } // l(x) = (a_L - z*1) + s_L*x // r(x) = y^n * (a_R + z*1 + s_R*x) + z^2 * 2^n @@ -443,9 +448,9 @@ RangeProof range_prove(std::uint64_t value, } // Commit to t_1, t_2 - Scalar tau1_bytes_raw = Scalar::from_bytes( + const Scalar tau1_bytes_raw = Scalar::from_bytes( SHA256::hash(rho.to_bytes().data(), 32)); - Scalar tau2_bytes_raw = Scalar::from_bytes( + const Scalar tau2_bytes_raw = Scalar::from_bytes( SHA256::hash(tau1_bytes_raw.to_bytes().data(), 32)); Scalar const tau1 = tau1_bytes_raw; Scalar const tau2 = tau2_bytes_raw; @@ -478,8 +483,9 @@ RangeProof range_prove(std::uint64_t value, // t_hat = Scalar t_hat = Scalar::zero(); - for (std::size_t i = 0; i < RANGE_PROOF_BITS; ++i) + for (std::size_t i = 0; i < RANGE_PROOF_BITS; ++i) { t_hat = t_hat + l_x[i] * r_x[i]; + } proof.t_hat = t_hat; // tau_x = tau_2 * x^2 + tau_1 * x + z^2 * blinding @@ -497,11 +503,12 @@ RangeProof range_prove(std::uint64_t value, std::memcpy(b_vec, r_x, sizeof(r_x)); // Compute modified generators: H'_i = H_i * y^{-i} - Scalar y_inv = y.inverse(); + const Scalar y_inv = y.inverse(); Scalar y_inv_powers[RANGE_PROOF_BITS]; y_inv_powers[0] = Scalar::one(); - for (std::size_t i = 1; i < RANGE_PROOF_BITS; ++i) + for (std::size_t i = 1; i < RANGE_PROOF_BITS; ++i) { y_inv_powers[i] = y_inv_powers[i - 1] * y_inv; + } Point G_vec[RANGE_PROOF_BITS]; Point H_vec[RANGE_PROOF_BITS]; @@ -607,21 +614,25 @@ bool range_verify(const PedersenCommitment& commitment, // where delta(y,z) = (z - z^2) * <1, y^n> - z^3 * <1, 2^n> Scalar y_powers[RANGE_PROOF_BITS]; y_powers[0] = Scalar::one(); - for (std::size_t i = 1; i < RANGE_PROOF_BITS; ++i) + for (std::size_t i = 1; i < RANGE_PROOF_BITS; ++i) { y_powers[i] = y_powers[i - 1] * y; + } Scalar two_powers[RANGE_PROOF_BITS]; two_powers[0] = Scalar::one(); - for (std::size_t i = 1; i < RANGE_PROOF_BITS; ++i) + for (std::size_t i = 1; i < RANGE_PROOF_BITS; ++i) { two_powers[i] = two_powers[i - 1] + two_powers[i - 1]; + } Scalar sum_y = Scalar::zero(); - for (std::size_t i = 0; i < RANGE_PROOF_BITS; ++i) + for (std::size_t i = 0; i < RANGE_PROOF_BITS; ++i) { sum_y = sum_y + y_powers[i]; + } Scalar sum_2 = Scalar::zero(); - for (std::size_t i = 0; i < RANGE_PROOF_BITS; ++i) + for (std::size_t i = 0; i < RANGE_PROOF_BITS; ++i) { sum_2 = sum_2 + two_powers[i]; + } Scalar const z3 = z2 * z; Scalar const delta = (z - z2) * sum_y - z3 * sum_2; @@ -639,7 +650,7 @@ bool range_verify(const PedersenCommitment& commitment, Point poly_p[5] = { H_ped, Point::generator(), commitment.point, proof.T1, proof.T2 }; - Point poly_check = msm(poly_s, poly_p, 5); + const Point poly_check = msm(poly_s, poly_p, 5); if (!poly_check.is_infinity()) return false; } @@ -658,22 +669,25 @@ bool range_verify(const PedersenCommitment& commitment, // Compute scalar coefficients for each generator // s_i = product_{j: bit j of i is 0} x_j^{-1} * product_{j: bit j of i is 1} x_j - Scalar y_inv = y.inverse(); + const Scalar y_inv = y.inverse(); Scalar y_inv_powers[RANGE_PROOF_BITS]; y_inv_powers[0] = Scalar::one(); - for (std::size_t i = 1; i < RANGE_PROOF_BITS; ++i) + for (std::size_t i = 1; i < RANGE_PROOF_BITS; ++i) { y_inv_powers[i] = y_inv_powers[i - 1] * y_inv; + } Scalar x_inv_rounds[RANGE_PROOF_LOG2]; - for (std::size_t j = 0; j < RANGE_PROOF_LOG2; ++j) + for (std::size_t j = 0; j < RANGE_PROOF_LOG2; ++j) { x_inv_rounds[j] = x_rounds[j].inverse(); + } // Compute s_i via product tree (much faster than per-index loop) // s_0 = prod(x_inv_rounds[j]), then propagate: flip x_inv->x for each set bit Scalar s_coeff[RANGE_PROOF_BITS]; s_coeff[0] = Scalar::one(); - for (std::size_t j = 0; j < RANGE_PROOF_LOG2; ++j) + for (std::size_t j = 0; j < RANGE_PROOF_LOG2; ++j) { s_coeff[0] = s_coeff[0] * x_inv_rounds[j]; + } for (std::size_t i = 1; i < RANGE_PROOF_BITS; ++i) { // s[i] = s[i-1] * x_rounds[j] / x_inv_rounds[j] = s[i-1] * x_rounds[j]^2 @@ -683,10 +697,12 @@ bool range_verify(const PedersenCommitment& commitment, // Use standard butterfly construction s_coeff[i] = Scalar::one(); for (std::size_t jj = 0; jj < RANGE_PROOF_LOG2; ++jj) { - if ((i >> (RANGE_PROOF_LOG2 - 1 - jj)) & 1) + if ((i >> (RANGE_PROOF_LOG2 - 1 - jj)) & 1) { s_coeff[i] = s_coeff[i] * x_rounds[jj]; - else + } + else { s_coeff[i] = s_coeff[i] * x_inv_rounds[jj]; + } } } @@ -717,8 +733,9 @@ bool range_verify(const PedersenCommitment& commitment, // Product tree for batch inversion Scalar acc[RANGE_PROOF_BITS]; acc[0] = s_coeff[0]; - for (std::size_t i = 1; i < RANGE_PROOF_BITS; ++i) + for (std::size_t i = 1; i < RANGE_PROOF_BITS; ++i) { acc[i] = acc[i - 1] * s_coeff[i]; + } Scalar inv_acc = acc[RANGE_PROOF_BITS - 1].inverse(); // single inversion! for (std::size_t i = RANGE_PROOF_BITS; i-- > 1; ) { @@ -782,8 +799,9 @@ bool batch_range_verify(const PedersenCommitment* commitments, const RangeProof* proofs, std::size_t count) { for (std::size_t i = 0; i < count; ++i) { - if (!range_verify(commitments[i], proofs[i])) + if (!range_verify(commitments[i], proofs[i])) { return false; + } } return true; } diff --git a/cpu/tests/test_bip39.cpp b/cpu/tests/test_bip39.cpp index 545baf9..4ce5c29 100644 --- a/cpu/tests/test_bip39.cpp +++ b/cpu/tests/test_bip39.cpp @@ -24,16 +24,10 @@ static int tests_passed = 0; static void hex_to_bytes(const char* hex, uint8_t* out, size_t len) { for (size_t i = 0; i < len; ++i) { - unsigned int byte = 0; -#ifdef __clang__ -#pragma clang diagnostic push -#pragma clang diagnostic ignored "-Wdeprecated-declarations" -#endif - std::sscanf(hex + 2 * i, "%02x", &byte); -#ifdef __clang__ -#pragma clang diagnostic pop -#endif - out[i] = static_cast(byte); + char pair[3] = { hex[2 * i], hex[2 * i + 1], '\0' }; + char* endptr = nullptr; + const unsigned long val = std::strtoul(pair, &endptr, 16); + out[i] = (endptr == pair + 2) ? static_cast(val) : 0; } } @@ -42,7 +36,7 @@ static std::string bytes_to_hex(const uint8_t* data, size_t len) { result.reserve(len * 2); for (size_t i = 0; i < len; ++i) { char buf[3]; - std::snprintf(buf, sizeof(buf), "%02x", data[i]); + (void)std::snprintf(buf, sizeof(buf), "%02x", data[i]); result += buf; } return result; @@ -96,6 +90,7 @@ static void test_wordlist() { const char* const* wl = bip39_wordlist_english(); CHECK(wl != nullptr, "wordlist not null"); + if (!wl) { return; } CHECK(std::strcmp(wl[0], "abandon") == 0, "first word = abandon"); CHECK(std::strcmp(wl[2047], "zoo") == 0, "last word = zoo"); CHECK(std::strcmp(wl[1], "ability") == 0, "word[1] = ability"); @@ -235,7 +230,7 @@ static void test_mnemonic_to_seed() { "abandon abandon abandon abandon abandon about"; auto [seed, ok] = bip39_mnemonic_to_seed(mnemonic, "TREZOR"); CHECK(ok, "TV1 seed: derivation ok"); - std::string hex = bytes_to_hex(seed.data(), 64); + const std::string hex = bytes_to_hex(seed.data(), 64); CHECK(hex == "c55257c360c07c72029aebc1b53c05ed0362ada38ead3e3e9efa3708e5349553" "1f09a6987599d18264c1e1c92f2cf141630c7a3c4ab7c81b2f001698e7463b04", "TV1 seed: matches Trezor vector"); @@ -249,7 +244,7 @@ static void test_mnemonic_to_seed() { "abandon abandon abandon abandon abandon art"; auto [seed, ok] = bip39_mnemonic_to_seed(mnemonic, "TREZOR"); CHECK(ok, "TV5 seed: derivation ok"); - std::string hex = bytes_to_hex(seed.data(), 64); + const std::string hex = bytes_to_hex(seed.data(), 64); CHECK(hex == "bda85446c68413707090a52022edd26a1c9462295029f2e60cd7c4f2bbd30971" "70af7a4d73245cafa9c3cca8d561a7c3de6f5d4a10be8ed2a5e608d68f92fcc8", "TV5 seed: matches Trezor vector"); @@ -261,7 +256,7 @@ static void test_mnemonic_to_seed() { "abandon abandon abandon abandon abandon about"; auto [seed, ok] = bip39_mnemonic_to_seed(mnemonic, ""); CHECK(ok, "no-passphrase seed: derivation ok"); - std::string hex = bytes_to_hex(seed.data(), 64); + const std::string hex = bytes_to_hex(seed.data(), 64); // Known result with empty passphrase (salt = "mnemonic"): CHECK(hex == "5eb00bbddcf069084889a8ab9155568165f5c453ccb85e70811aaed6f6da5fc1" "9a5ac40b389cd370d086206dec8aa6c43daea6690f20ad3d8d48b2d2ce9e38e4", @@ -337,7 +332,7 @@ static void test_edge_cases() { CHECK(ok, "160-bit entropy generates ok"); // Count words int wc = 1; - for (char c : mnemonic) if (c == ' ') ++wc; + for (const char c : mnemonic) if (c == ' ') ++wc; CHECK(wc == 15, "160-bit entropy -> 15 words"); CHECK(bip39_validate(mnemonic), "160-bit mnemonic validates"); } @@ -349,7 +344,7 @@ static void test_edge_cases() { auto [mnemonic, ok] = bip39_generate(24, entropy); CHECK(ok, "192-bit entropy generates ok"); int wc = 1; - for (char c : mnemonic) if (c == ' ') ++wc; + for (const char c : mnemonic) if (c == ' ') ++wc; CHECK(wc == 18, "192-bit entropy -> 18 words"); CHECK(bip39_validate(mnemonic), "192-bit mnemonic validates"); } @@ -362,7 +357,7 @@ static void test_edge_cases() { auto [mnemonic, ok] = bip39_generate(28, entropy); CHECK(ok, "224-bit entropy generates ok"); int wc = 1; - for (char c : mnemonic) if (c == ' ') ++wc; + for (const char c : mnemonic) if (c == ' ') ++wc; CHECK(wc == 21, "224-bit entropy -> 21 words"); CHECK(bip39_validate(mnemonic), "224-bit mnemonic validates"); } diff --git a/cpu/tests/test_ethereum.cpp b/cpu/tests/test_ethereum.cpp index 4fa5c3d..98e7a1e 100644 --- a/cpu/tests/test_ethereum.cpp +++ b/cpu/tests/test_ethereum.cpp @@ -50,9 +50,10 @@ static int tests_failed = 0; static void hex_to_bytes(const char* hex, uint8_t* out, size_t len) { for (size_t i = 0; i < len; ++i) { - unsigned int byte = 0; - if (std::sscanf(hex + i * 2, "%02x", &byte) != 1) byte = 0; - out[i] = static_cast(byte); + char pair[3] = { hex[i * 2], hex[i * 2 + 1], '\0' }; + char* endptr = nullptr; + const unsigned long val = std::strtoul(pair, &endptr, 16); + out[i] = (endptr == pair + 2) ? static_cast(val) : 0; } } @@ -186,13 +187,19 @@ static void test_eth_sign_hash() { ASSERT_TRUE(!r_zero, "r should be non-zero"); ASSERT_TRUE(!s_zero, "s should be non-zero"); // v should be 27 or 28 for legacy - ASSERT_TRUE(sig.v == 27 || sig.v == 28, "legacy v should be 27 or 28"); + { + const bool v_ok = (sig.v == 27 || sig.v == 28); + ASSERT_TRUE(v_ok, "legacy v should be 27 or 28"); + } PASS(); // Sign with Ethereum mainnet chain ID TEST("eth_sign_hash with chain_id=1 (Ethereum)"); auto sig2 = eth_sign_hash(hash, sk, 1); - ASSERT_TRUE(sig2.v == 37 || sig2.v == 38, "EIP-155 v should be 37 or 38"); + { + const bool v2_ok = (sig2.v == 37 || sig2.v == 38); + ASSERT_TRUE(v2_ok, "EIP-155 v should be 37 or 38"); + } PASS(); // Same hash + key should give same r,s @@ -223,8 +230,8 @@ static void test_ecrecover() { hex_to_bytes("c6b506e21f3c26dfe9b3a15a40d2dde0ab9ee4bb9e6f7e6e49f7ef9fd9b3a3d5", sk_bytes.data(), 32); Scalar const sk = Scalar::from_bytes(sk_bytes); - Point pk = Point::generator().scalar_mul(sk); - auto expected_addr = ethereum_address_bytes(pk); + const Point pk = Point::generator().scalar_mul(sk); + const auto expected_addr = ethereum_address_bytes(pk); // Hash a message std::array hash{}; @@ -261,7 +268,7 @@ static void test_ecrecover() { // ecrecover with invalid r=0 should fail TEST("ecrecover invalid r=0"); - std::array zero{}; + const std::array zero{}; auto [_, ok4] = ecrecover(hash, zero, sig.s, sig.v); ASSERT_TRUE(!ok4, "ecrecover with r=0 should fail"); PASS(); @@ -284,7 +291,7 @@ static void test_personal_sign() { hex_to_bytes("4c0883a69102937d6231471b5dbb6204fe512961708279f8f30ab5c5dbe3a2b7", sk_bytes.data(), 32); Scalar const sk = Scalar::from_bytes(sk_bytes); - Point pk = Point::generator().scalar_mul(sk); + const Point pk = Point::generator().scalar_mul(sk); auto addr = ethereum_address_bytes(pk); const char* msg = "I agree to the terms of service"; @@ -295,18 +302,21 @@ static void test_personal_sign() { bool r_zero = true; for (auto b : sig.r) { if (b != 0) r_zero = false; } ASSERT_TRUE(!r_zero, "r should be non-zero"); - ASSERT_TRUE(sig.v == 27 || sig.v == 28, "v should be 27 or 28"); + { + const bool v_ok2 = (sig.v == 27 || sig.v == 28); + ASSERT_TRUE(v_ok2, "v should be 27 or 28"); + } PASS(); TEST("personal_verify valid"); - bool valid = eth_personal_verify( + const bool valid = eth_personal_verify( reinterpret_cast(msg), msg_len, sig, addr); ASSERT_TRUE(valid, "signature should verify"); PASS(); TEST("personal_verify wrong message"); const char* wrong_msg = "I disagree to the terms of service"; - bool wrong = eth_personal_verify( + const bool wrong = eth_personal_verify( reinterpret_cast(wrong_msg), std::strlen(wrong_msg), sig, addr); ASSERT_TRUE(!wrong, "wrong message should not verify"); PASS(); @@ -314,7 +324,7 @@ static void test_personal_sign() { TEST("personal_verify wrong address"); std::array wrong_addr{}; wrong_addr[0] = 0xFF; - bool wrong2 = eth_personal_verify( + const bool wrong2 = eth_personal_verify( reinterpret_cast(msg), msg_len, sig, wrong_addr); ASSERT_TRUE(!wrong2, "wrong address should not verify"); PASS(); @@ -330,8 +340,8 @@ static void test_multi_chain() { std::array sk_bytes{}; sk_bytes[31] = 7; Scalar const sk = Scalar::from_bytes(sk_bytes); - Point pk = Point::generator().scalar_mul(sk); - auto expected_addr = ethereum_address_bytes(pk); + const Point pk = Point::generator().scalar_mul(sk); + const auto expected_addr = ethereum_address_bytes(pk); std::array hash{}; hash[0] = 0xAB; hash[1] = 0xCD; @@ -349,7 +359,7 @@ static void test_multi_chain() { for (auto& chain : chains) { char buf[64]; - std::snprintf(buf, sizeof(buf), "Round-trip chain_id=%lu (%s)", + (void)std::snprintf(buf, sizeof(buf), "Round-trip chain_id=%lu (%s)", static_cast(chain.id), chain.name); TEST(buf); diff --git a/cpu/tests/test_wallet.cpp b/cpu/tests/test_wallet.cpp index 1b36e79..ae8d713 100644 --- a/cpu/tests/test_wallet.cpp +++ b/cpu/tests/test_wallet.cpp @@ -42,7 +42,6 @@ using namespace secp256k1; using namespace secp256k1::coins; using namespace secp256k1::coins::wallet; using fast::Scalar; -using fast::Point; static int tests_passed = 0; static int tests_failed = 0; @@ -64,9 +63,10 @@ static int tests_failed = 0; static void hex_to_bytes(const char* hex, uint8_t* out, size_t len) { for (size_t i = 0; i < len; ++i) { - unsigned int byte = 0; - if (std::sscanf(hex + i * 2, "%02x", &byte) != 1) byte = 0; - out[i] = static_cast(byte); + char pair[3] = { hex[i * 2], hex[i * 2 + 1], '\0' }; + char* endptr = nullptr; + const unsigned long val = std::strtoul(pair, &endptr, 16); + out[i] = (endptr == pair + 2) ? static_cast(val) : 0; } } @@ -194,7 +194,10 @@ static void test_export_privkey_bitcoin_wif() { auto wif = export_private_key(Bitcoin, key); ASSERT_TRUE(!wif.empty(), "non-empty WIF"); // Compressed mainnet WIF starts with 'K' or 'L' - ASSERT_TRUE(wif[0] == 'K' || wif[0] == 'L', "WIF starts with K or L"); + { + const bool wif_prefix_ok = (wif[0] == 'K' || wif[0] == 'L'); + ASSERT_TRUE(wif_prefix_ok, "WIF starts with K or L"); + } PASS(); } @@ -287,15 +290,15 @@ static void test_bitcoin_sign_verify() { auto scalar = Scalar::from_bytes(priv); auto pubkey = derive_public_key(scalar); const uint8_t msg[] = "Test message for signing"; - size_t msg_len = sizeof(msg) - 1; // no null terminator + const size_t msg_len = sizeof(msg) - 1; // no null terminator auto rsig = bitcoin_sign_message(msg, msg_len, scalar); - bool ok = bitcoin_verify_message(msg, msg_len, pubkey, rsig.sig); + const bool ok = bitcoin_verify_message(msg, msg_len, pubkey, rsig.sig); ASSERT_TRUE(ok, "verify should pass"); // Tamper: different message should fail const uint8_t bad_msg[] = "Wrong message"; - bool bad = bitcoin_verify_message(bad_msg, sizeof(bad_msg) - 1, pubkey, rsig.sig); + const bool bad = bitcoin_verify_message(bad_msg, sizeof(bad_msg) - 1, pubkey, rsig.sig); ASSERT_TRUE(!bad, "tampered msg should fail"); PASS(); } @@ -311,7 +314,7 @@ static void test_bitcoin_sign_recover() { auto scalar = Scalar::from_bytes(priv); auto pubkey = derive_public_key(scalar); const uint8_t msg[] = "Recovery test message"; - size_t msg_len = sizeof(msg) - 1; + const size_t msg_len = sizeof(msg) - 1; auto rsig = bitcoin_sign_message(msg, msg_len, scalar); auto [recovered, ok] = bitcoin_recover_message(msg, msg_len, rsig.sig, rsig.recid); @@ -333,7 +336,7 @@ static void test_base64_round_trip() { hex_to_bytes(TEST_PRIVKEY_HEX, priv, 32); auto scalar = Scalar::from_bytes(priv); const uint8_t msg[] = "Base64 test"; - size_t msg_len = sizeof(msg) - 1; + const size_t msg_len = sizeof(msg) - 1; auto rsig = bitcoin_sign_message(msg, msg_len, scalar); auto b64 = bitcoin_sig_to_base64(rsig, true); @@ -363,10 +366,10 @@ static void test_wallet_sign_verify_bitcoin() { ASSERT_TRUE(ok, "key creation"); const uint8_t msg[] = "Wallet API test message"; - size_t msg_len = sizeof(msg) - 1; + const size_t msg_len = sizeof(msg) - 1; auto sig = sign_message(Bitcoin, key, msg, msg_len); - bool verified = verify_message(Bitcoin, key.pub, msg, msg_len, sig); + const bool verified = verify_message(Bitcoin, key.pub, msg, msg_len, sig); ASSERT_TRUE(verified, "verify should pass"); // Wrong message should fail @@ -394,7 +397,10 @@ static void test_wallet_sign_hash_recover() { // Reconstruct: for raw hash verification, manually hash + verify // sign_hash with Bitcoin coin uses ecdsa_sign_recoverable directly - ASSERT_TRUE(sig.recid >= 0 && sig.recid <= 3, "valid recid"); + { + const bool recid_ok = (sig.recid >= 0 && sig.recid <= 3); + ASSERT_TRUE(recid_ok, "valid recid"); + } bool r_nonzero = false, s_nonzero = false; for (auto b : sig.r) if (b) { r_nonzero = true; break; } for (auto b : sig.s) if (b) { s_nonzero = true; break; } @@ -415,7 +421,7 @@ static void test_wallet_sign_recover_ethereum() { ASSERT_TRUE(ok, "key creation"); const uint8_t msg[] = "Ethereum wallet test"; - size_t msg_len = sizeof(msg) - 1; + const size_t msg_len = sizeof(msg) - 1; auto sig = sign_message(Ethereum, key, msg, msg_len); auto [addr, recovered] = recover_address(Ethereum, msg, msg_len, sig); @@ -434,7 +440,7 @@ static void test_wallet_sign_recover_tron() { ASSERT_TRUE(ok, "key creation"); const uint8_t msg[] = "Tron wallet test"; - size_t msg_len = sizeof(msg) - 1; + const size_t msg_len = sizeof(msg) - 1; auto sig = sign_message(Tron, key, msg, msg_len); auto [addr, recovered] = recover_address(Tron, msg, msg_len, sig); @@ -502,7 +508,10 @@ static void test_multi_coin_addresses() { auto ltc = get_address(Litecoin, key); auto doge = get_address(Dogecoin, key); - ASSERT_TRUE(!btc.empty() && !ltc.empty() && !doge.empty(), "all non-empty"); + { + const bool coins_non_empty = !btc.empty() && !ltc.empty() && !doge.empty(); + ASSERT_TRUE(coins_non_empty, "all non-empty"); + } // All addresses should be different (different prefixes/encoding) ASSERT_TRUE(btc != ltc, "BTC != LTC"); ASSERT_TRUE(btc != doge, "BTC != DOGE"); @@ -599,8 +608,10 @@ static void test_wallet_all_btc_formats() { auto p2tr = get_address_p2tr(Bitcoin, key); // All four formats should be non-empty and different - ASSERT_TRUE(!p2pkh.empty() && !p2wpkh.empty() && !p2sh.empty() && !p2tr.empty(), - "all non-empty"); + { + const bool addrs_non_empty = !p2pkh.empty() && !p2wpkh.empty() && !p2sh.empty() && !p2tr.empty(); + ASSERT_TRUE(addrs_non_empty, "all non-empty"); + } ASSERT_TRUE(p2pkh != p2wpkh, "P2PKH != P2WPKH"); ASSERT_TRUE(p2pkh != p2sh, "P2PKH != P2SH-P2WPKH"); ASSERT_TRUE(p2pkh != p2tr, "P2PKH != P2TR"); diff --git a/cpu/tests/test_zk.cpp b/cpu/tests/test_zk.cpp index d7f66bc..0bafdc8 100644 --- a/cpu/tests/test_zk.cpp +++ b/cpu/tests/test_zk.cpp @@ -57,7 +57,7 @@ static void test_knowledge_proof_wrong_key() { auto pubkey = Point::generator().scalar_mul(secret); auto wrong_pubkey = Point::generator().scalar_mul(Scalar::from_uint64(43)); - std::array msg{}; + const std::array msg{}; std::array aux{}; aux[0] = 0x02; @@ -92,7 +92,7 @@ static void test_knowledge_proof_serialization() { auto secret = Scalar::from_uint64(999); auto pubkey = Point::generator().scalar_mul(secret); - std::array msg{}; + const std::array msg{}; std::array aux{}; aux[0] = 0x04; @@ -100,7 +100,7 @@ static void test_knowledge_proof_serialization() { auto serialized = proof.serialize(); zk::KnowledgeProof deserialized{}; - bool ok = zk::KnowledgeProof::deserialize(serialized.data(), deserialized); + const bool ok = zk::KnowledgeProof::deserialize(serialized.data(), deserialized); CHECK(ok, "deserialization_succeeds"); CHECK(zk::knowledge_verify(deserialized, pubkey, msg), "deserialized_proof_verifies"); @@ -114,7 +114,7 @@ static void test_knowledge_proof_custom_base() { const auto& H = pedersen_generator_H(); auto point = H.scalar_mul(secret); - std::array msg{}; + const std::array msg{}; std::array aux{}; aux[0] = 0x05; @@ -131,7 +131,7 @@ static void test_knowledge_proof_deterministic() { auto secret = Scalar::from_uint64(42); auto pubkey = Point::generator().scalar_mul(secret); - std::array msg{}; + const std::array msg{}; std::array aux{}; aux[0] = 0x06; @@ -264,7 +264,7 @@ static void test_range_proof_generators() { static void test_range_proof_basic() { std::printf("\n=== Range Proof: Basic (value=42) ===\n"); - std::uint64_t value = 42; + const std::uint64_t value = 42; auto blinding = Scalar::from_uint64(12345); auto commitment = pedersen_commit(Scalar::from_uint64(value), blinding); @@ -278,7 +278,7 @@ static void test_range_proof_basic() { static void test_range_proof_zero() { std::printf("\n=== Range Proof: Edge Case (value=0) ===\n"); - std::uint64_t value = 0; + const std::uint64_t value = 0; auto blinding = Scalar::from_uint64(99999); auto commitment = pedersen_commit(Scalar::from_uint64(value), blinding); @@ -292,7 +292,7 @@ static void test_range_proof_zero() { static void test_range_proof_max() { std::printf("\n=== Range Proof: Edge Case (value=2^64-1) ===\n"); - std::uint64_t value = UINT64_MAX; + const std::uint64_t value = UINT64_MAX; auto blinding = Scalar::from_uint64(77777); auto commitment = pedersen_commit(Scalar::from_uint64(value), blinding); @@ -306,7 +306,7 @@ static void test_range_proof_max() { static void test_range_proof_wrong_commitment() { std::printf("\n=== Range Proof: Wrong Commitment ===\n"); - std::uint64_t value = 100; + const std::uint64_t value = 100; auto blinding = Scalar::from_uint64(11111); auto commitment = pedersen_commit(Scalar::from_uint64(value), blinding); auto wrong_commitment = pedersen_commit(Scalar::from_uint64(200), blinding); @@ -322,7 +322,7 @@ static void test_range_proof_wrong_commitment() { static void test_range_proof_deterministic() { std::printf("\n=== Range Proof: Deterministic ===\n"); - std::uint64_t value = 42; + const std::uint64_t value = 42; auto blinding = Scalar::from_uint64(12345); auto commitment = pedersen_commit(Scalar::from_uint64(value), blinding); diff --git a/include/ufsecp/ufsecp_impl.cpp b/include/ufsecp/ufsecp_impl.cpp index a32ee1f..4b85a13 100644 --- a/include/ufsecp/ufsecp_impl.cpp +++ b/include/ufsecp/ufsecp_impl.cpp @@ -1239,14 +1239,17 @@ ufsecp_error_t ufsecp_pubkey_add(ufsecp_ctx* ctx, if (!ctx || !a33 || !b33 || !out33) return UFSECP_ERR_NULL_ARG; ctx_clear_err(ctx); auto pa = point_from_compressed(a33); - if (pa.is_infinity()) + if (pa.is_infinity()) { return ctx_set_err(ctx, UFSECP_ERR_BAD_PUBKEY, "invalid pubkey a"); + } auto pb = point_from_compressed(b33); - if (pb.is_infinity()) + if (pb.is_infinity()) { return ctx_set_err(ctx, UFSECP_ERR_BAD_PUBKEY, "invalid pubkey b"); + } auto sum = pa.add(pb); - if (sum.is_infinity()) + if (sum.is_infinity()) { return ctx_set_err(ctx, UFSECP_ERR_ARITH, "sum is point at infinity"); + } point_to_compressed(sum, out33); return UFSECP_OK; } @@ -1257,8 +1260,9 @@ ufsecp_error_t ufsecp_pubkey_negate(ufsecp_ctx* ctx, if (!ctx || !pubkey33 || !out33) return UFSECP_ERR_NULL_ARG; ctx_clear_err(ctx); auto p = point_from_compressed(pubkey33); - if (p.is_infinity()) + if (p.is_infinity()) { return ctx_set_err(ctx, UFSECP_ERR_BAD_PUBKEY, "invalid pubkey"); + } auto neg = p.negate(); point_to_compressed(neg, out33); return UFSECP_OK; @@ -1271,15 +1275,18 @@ ufsecp_error_t ufsecp_pubkey_tweak_add(ufsecp_ctx* ctx, if (!ctx || !pubkey33 || !tweak || !out33) return UFSECP_ERR_NULL_ARG; ctx_clear_err(ctx); auto p = point_from_compressed(pubkey33); - if (p.is_infinity()) + if (p.is_infinity()) { return ctx_set_err(ctx, UFSECP_ERR_BAD_PUBKEY, "invalid pubkey"); + } Scalar tw; - if (!scalar_parse_strict(tweak, tw)) + if (!scalar_parse_strict(tweak, tw)) { return ctx_set_err(ctx, UFSECP_ERR_BAD_INPUT, "tweak >= n"); + } auto tG = Point::generator().scalar_mul(tw); auto result = p.add(tG); - if (result.is_infinity()) + if (result.is_infinity()) { return ctx_set_err(ctx, UFSECP_ERR_ARITH, "tweak_add resulted in infinity"); + } point_to_compressed(result, out33); return UFSECP_OK; } @@ -1291,14 +1298,17 @@ ufsecp_error_t ufsecp_pubkey_tweak_mul(ufsecp_ctx* ctx, if (!ctx || !pubkey33 || !tweak || !out33) return UFSECP_ERR_NULL_ARG; ctx_clear_err(ctx); auto p = point_from_compressed(pubkey33); - if (p.is_infinity()) + if (p.is_infinity()) { return ctx_set_err(ctx, UFSECP_ERR_BAD_PUBKEY, "invalid pubkey"); + } Scalar tw; - if (!scalar_parse_strict_nonzero(tweak, tw)) + if (!scalar_parse_strict_nonzero(tweak, tw)) { return ctx_set_err(ctx, UFSECP_ERR_BAD_INPUT, "tweak is zero or >= n"); + } auto result = p.scalar_mul(tw); - if (result.is_infinity()) + if (result.is_infinity()) { return ctx_set_err(ctx, UFSECP_ERR_ARITH, "tweak_mul resulted in infinity"); + } point_to_compressed(result, out33); return UFSECP_OK; } @@ -1311,16 +1321,19 @@ ufsecp_error_t ufsecp_pubkey_combine(ufsecp_ctx* ctx, if (n == 0) return ctx_set_err(ctx, UFSECP_ERR_BAD_INPUT, "need >= 1 pubkey"); ctx_clear_err(ctx); auto acc = point_from_compressed(pubkeys); - if (acc.is_infinity()) + if (acc.is_infinity()) { return ctx_set_err(ctx, UFSECP_ERR_BAD_PUBKEY, "invalid pubkey[0]"); + } for (size_t i = 1; i < n; ++i) { auto pi = point_from_compressed(pubkeys + i * 33); - if (pi.is_infinity()) + if (pi.is_infinity()) { return ctx_set_err(ctx, UFSECP_ERR_BAD_PUBKEY, "invalid pubkey in array"); + } acc = acc.add(pi); } - if (acc.is_infinity()) + if (acc.is_infinity()) { return ctx_set_err(ctx, UFSECP_ERR_ARITH, "combined pubkey is infinity"); + } point_to_compressed(acc, out33); return UFSECP_OK; } @@ -1337,13 +1350,16 @@ ufsecp_error_t ufsecp_bip39_generate(ufsecp_ctx* ctx, if (!ctx || !mnemonic_out || !mnemonic_len) return UFSECP_ERR_NULL_ARG; ctx_clear_err(ctx); if (entropy_bytes != 16 && entropy_bytes != 20 && entropy_bytes != 24 && - entropy_bytes != 28 && entropy_bytes != 32) + entropy_bytes != 28 && entropy_bytes != 32) { return ctx_set_err(ctx, UFSECP_ERR_BAD_INPUT, "entropy must be 16/20/24/28/32"); + } auto [mnemonic, ok] = secp256k1::bip39_generate(entropy_bytes, entropy_in); - if (!ok) + if (!ok) { return ctx_set_err(ctx, UFSECP_ERR_INTERNAL, "BIP-39 generation failed"); - if (*mnemonic_len < mnemonic.size() + 1) + } + if (*mnemonic_len < mnemonic.size() + 1) { return ctx_set_err(ctx, UFSECP_ERR_BUF_TOO_SMALL, "mnemonic buffer too small"); + } std::memcpy(mnemonic_out, mnemonic.c_str(), mnemonic.size() + 1); *mnemonic_len = mnemonic.size(); return UFSECP_OK; @@ -1352,8 +1368,9 @@ ufsecp_error_t ufsecp_bip39_generate(ufsecp_ctx* ctx, ufsecp_error_t ufsecp_bip39_validate(const ufsecp_ctx* ctx, const char* mnemonic) { if (!ctx || !mnemonic) return UFSECP_ERR_NULL_ARG; - if (!secp256k1::bip39_validate(std::string(mnemonic))) + if (!secp256k1::bip39_validate(std::string(mnemonic))) { return UFSECP_ERR_BAD_INPUT; + } return UFSECP_OK; } @@ -1363,10 +1380,11 @@ ufsecp_error_t ufsecp_bip39_to_seed(ufsecp_ctx* ctx, uint8_t seed64_out[64]) { if (!ctx || !mnemonic || !seed64_out) return UFSECP_ERR_NULL_ARG; ctx_clear_err(ctx); - std::string pass = passphrase ? passphrase : ""; + const std::string pass = passphrase ? passphrase : ""; auto [seed, ok] = secp256k1::bip39_mnemonic_to_seed(std::string(mnemonic), pass); - if (!ok) + if (!ok) { return ctx_set_err(ctx, UFSECP_ERR_BAD_INPUT, "invalid mnemonic"); + } std::memcpy(seed64_out, seed.data(), 64); return UFSECP_OK; } @@ -1409,11 +1427,13 @@ ufsecp_error_t ufsecp_schnorr_batch_verify(ufsecp_ctx* ctx, } std::memcpy(batch[i].pubkey_x.data(), e, 32); std::memcpy(batch[i].message.data(), e + 32, 32); - if (!secp256k1::SchnorrSignature::parse_strict(e + 64, batch[i].signature)) + if (!secp256k1::SchnorrSignature::parse_strict(e + 64, batch[i].signature)) { return ctx_set_err(ctx, UFSECP_ERR_BAD_SIG, "invalid Schnorr sig in batch"); + } } - if (!secp256k1::schnorr_batch_verify(batch)) + if (!secp256k1::schnorr_batch_verify(batch)) { return ctx_set_err(ctx, UFSECP_ERR_VERIFY_FAIL, "batch verify failed"); + } return UFSECP_OK; } @@ -1428,15 +1448,18 @@ ufsecp_error_t ufsecp_ecdsa_batch_verify(ufsecp_ctx* ctx, const uint8_t* e = entries + i * 129; std::memcpy(batch[i].msg_hash.data(), e, 32); batch[i].public_key = point_from_compressed(e + 32); - if (batch[i].public_key.is_infinity()) + if (batch[i].public_key.is_infinity()) { return ctx_set_err(ctx, UFSECP_ERR_BAD_PUBKEY, "invalid pubkey in batch"); + } std::array compact; std::memcpy(compact.data(), e + 65, 64); - if (!secp256k1::ECDSASignature::parse_compact_strict(compact, batch[i].signature)) + if (!secp256k1::ECDSASignature::parse_compact_strict(compact, batch[i].signature)) { return ctx_set_err(ctx, UFSECP_ERR_BAD_SIG, "invalid ECDSA sig in batch"); + } } - if (!secp256k1::ecdsa_batch_verify(batch)) + if (!secp256k1::ecdsa_batch_verify(batch)) { return ctx_set_err(ctx, UFSECP_ERR_VERIFY_FAIL, "batch verify failed"); + } return UFSECP_OK; } @@ -1454,13 +1477,15 @@ ufsecp_error_t ufsecp_schnorr_batch_identify_invalid( } std::memcpy(batch[i].pubkey_x.data(), e, 32); std::memcpy(batch[i].message.data(), e + 32, 32); - if (!secp256k1::SchnorrSignature::parse_strict(e + 64, batch[i].signature)) + if (!secp256k1::SchnorrSignature::parse_strict(e + 64, batch[i].signature)) { return ctx_set_err(ctx, UFSECP_ERR_BAD_SIG, "invalid Schnorr sig in batch"); + } } auto invalids = secp256k1::schnorr_batch_identify_invalid(batch.data(), n); *invalid_count = invalids.size(); - for (size_t i = 0; i < invalids.size(); ++i) + for (size_t i = 0; i < invalids.size(); ++i) { invalid_out[i] = invalids[i]; + } return UFSECP_OK; } @@ -1474,17 +1499,20 @@ ufsecp_error_t ufsecp_ecdsa_batch_identify_invalid( const uint8_t* e = entries + i * 129; std::memcpy(batch[i].msg_hash.data(), e, 32); batch[i].public_key = point_from_compressed(e + 32); - if (batch[i].public_key.is_infinity()) + if (batch[i].public_key.is_infinity()) { return ctx_set_err(ctx, UFSECP_ERR_BAD_PUBKEY, "invalid pubkey in batch"); + } std::array compact; std::memcpy(compact.data(), e + 65, 64); - if (!secp256k1::ECDSASignature::parse_compact_strict(compact, batch[i].signature)) + if (!secp256k1::ECDSASignature::parse_compact_strict(compact, batch[i].signature)) { return ctx_set_err(ctx, UFSECP_ERR_BAD_SIG, "invalid ECDSA sig in batch"); + } } auto invalids = secp256k1::ecdsa_batch_identify_invalid(batch.data(), n); *invalid_count = invalids.size(); - for (size_t i = 0; i < invalids.size(); ++i) + for (size_t i = 0; i < invalids.size(); ++i) { invalid_out[i] = invalids[i]; + } return UFSECP_OK; } @@ -1511,19 +1539,24 @@ ufsecp_error_t ufsecp_shamir_trick(ufsecp_ctx* ctx, if (!ctx || !a || !P33 || !b || !Q33 || !out33) return UFSECP_ERR_NULL_ARG; ctx_clear_err(ctx); Scalar sa, sb; - if (!scalar_parse_strict(a, sa)) + if (!scalar_parse_strict(a, sa)) { return ctx_set_err(ctx, UFSECP_ERR_BAD_INPUT, "scalar a >= n"); - if (!scalar_parse_strict(b, sb)) + } + if (!scalar_parse_strict(b, sb)) { return ctx_set_err(ctx, UFSECP_ERR_BAD_INPUT, "scalar b >= n"); + } auto P = point_from_compressed(P33); - if (P.is_infinity()) + if (P.is_infinity()) { return ctx_set_err(ctx, UFSECP_ERR_BAD_PUBKEY, "invalid point P"); + } auto Q = point_from_compressed(Q33); - if (Q.is_infinity()) + if (Q.is_infinity()) { return ctx_set_err(ctx, UFSECP_ERR_BAD_PUBKEY, "invalid point Q"); + } auto result = secp256k1::shamir_trick(sa, P, sb, Q); - if (result.is_infinity()) + if (result.is_infinity()) { return ctx_set_err(ctx, UFSECP_ERR_ARITH, "result is infinity"); + } point_to_compressed(result, out33); return UFSECP_OK; } @@ -1539,15 +1572,18 @@ ufsecp_error_t ufsecp_multi_scalar_mul(ufsecp_ctx* ctx, std::vector svec(n); std::vector pvec(n); for (size_t i = 0; i < n; ++i) { - if (!scalar_parse_strict(scalars + i * 32, svec[i])) + if (!scalar_parse_strict(scalars + i * 32, svec[i])) { return ctx_set_err(ctx, UFSECP_ERR_BAD_INPUT, "scalar >= n"); + } pvec[i] = point_from_compressed(points + i * 33); - if (pvec[i].is_infinity()) + if (pvec[i].is_infinity()) { return ctx_set_err(ctx, UFSECP_ERR_BAD_PUBKEY, "invalid point in array"); + } } auto result = secp256k1::multi_scalar_mul(svec, pvec); - if (result.is_infinity()) + if (result.is_infinity()) { return ctx_set_err(ctx, UFSECP_ERR_ARITH, "MSM result is infinity"); + } point_to_compressed(result, out33); return UFSECP_OK; } @@ -1564,18 +1600,20 @@ ufsecp_error_t ufsecp_musig2_key_agg(ufsecp_ctx* ctx, if (n < 2) return ctx_set_err(ctx, UFSECP_ERR_BAD_INPUT, "need >= 2 pubkeys"); ctx_clear_err(ctx); std::vector> pks(n); - for (size_t i = 0; i < n; ++i) + for (size_t i = 0; i < n; ++i) { std::memcpy(pks[i].data(), pubkeys + i * 32, 32); + } auto kagg = secp256k1::musig2_key_agg(pks); std::memcpy(agg_pubkey32_out, kagg.Q_x.data(), 32); /* Serialize key agg ctx: n(4) | Q_negated(1) | Q_compressed(33) | coefficients(n*32) */ std::memset(keyagg_out, 0, UFSECP_MUSIG2_KEYAGG_LEN); - uint32_t nk = static_cast(kagg.key_coefficients.size()); + const auto nk = static_cast(kagg.key_coefficients.size()); std::memcpy(keyagg_out, &nk, 4); keyagg_out[4] = kagg.Q_negated ? 1 : 0; point_to_compressed(kagg.Q, keyagg_out + 5); - for (uint32_t i = 0; i < nk && (38u + (i+1)*32u <= UFSECP_MUSIG2_KEYAGG_LEN); ++i) - scalar_to_bytes(kagg.key_coefficients[i], keyagg_out + 38 + i * 32); + for (uint32_t i = 0; i < nk && (38u + (i+1)*32u <= UFSECP_MUSIG2_KEYAGG_LEN); ++i) { + scalar_to_bytes(kagg.key_coefficients[i], keyagg_out + 38 + static_cast(i) * 32); + } return UFSECP_OK; } @@ -1591,8 +1629,9 @@ ufsecp_error_t ufsecp_musig2_nonce_gen(ufsecp_ctx* ctx, !secnonce_out || !pubnonce_out) return UFSECP_ERR_NULL_ARG; ctx_clear_err(ctx); Scalar sk; - if (!scalar_parse_strict_nonzero(privkey, sk)) + if (!scalar_parse_strict_nonzero(privkey, sk)) { return ctx_set_err(ctx, UFSECP_ERR_BAD_KEY, "privkey is zero or >= n"); + } std::array pk_arr, agg_arr, msg_arr; std::memcpy(pk_arr.data(), pubkey32, 32); std::memcpy(agg_arr.data(), agg_pubkey32, 32); @@ -1652,7 +1691,7 @@ ufsecp_error_t ufsecp_musig2_start_sign_session( } /* Deserialize key agg context */ secp256k1::MuSig2KeyAggCtx kagg; - uint32_t nk; + uint32_t nk = 0; std::memcpy(&nk, keyagg, 4); kagg.Q_negated = (keyagg[4] != 0); kagg.Q = point_from_compressed(keyagg + 5); @@ -1688,22 +1727,26 @@ ufsecp_error_t ufsecp_musig2_partial_sign( const uint8_t session[UFSECP_MUSIG2_SESSION_LEN], size_t signer_index, uint8_t partial_sig32_out[32]) { - if (!ctx || !secnonce || !privkey || !keyagg || !session || !partial_sig32_out) + if (!ctx || !secnonce || !privkey || !keyagg || !session || !partial_sig32_out) { return UFSECP_ERR_NULL_ARG; + } ctx_clear_err(ctx); Scalar sk; - if (!scalar_parse_strict_nonzero(privkey, sk)) + if (!scalar_parse_strict_nonzero(privkey, sk)) { return ctx_set_err(ctx, UFSECP_ERR_BAD_KEY, "privkey is zero or >= n"); + } secp256k1::MuSig2SecNonce sn; Scalar k1, k2; - if (!scalar_parse_strict_nonzero(secnonce, k1)) + if (!scalar_parse_strict_nonzero(secnonce, k1)) { return ctx_set_err(ctx, UFSECP_ERR_BAD_INPUT, "invalid secnonce k1"); - if (!scalar_parse_strict_nonzero(secnonce + 32, k2)) + } + if (!scalar_parse_strict_nonzero(secnonce + 32, k2)) { return ctx_set_err(ctx, UFSECP_ERR_BAD_INPUT, "invalid secnonce k2"); + } sn.k1 = k1; sn.k2 = k2; secp256k1::MuSig2KeyAggCtx kagg; - { uint32_t nk; std::memcpy(&nk, keyagg, 4); kagg.Q_negated = (keyagg[4] != 0); + { uint32_t nk = 0; std::memcpy(&nk, keyagg, 4); kagg.Q_negated = (keyagg[4] != 0); kagg.Q = point_from_compressed(keyagg + 5); if (kagg.Q.is_infinity()) { return ctx_set_err(ctx, UFSECP_ERR_BAD_KEY, "invalid aggregated key"); @@ -1746,19 +1789,21 @@ ufsecp_error_t ufsecp_musig2_partial_verify( const uint8_t keyagg[UFSECP_MUSIG2_KEYAGG_LEN], const uint8_t session[UFSECP_MUSIG2_SESSION_LEN], size_t signer_index) { - if (!ctx || !partial_sig32 || !pubnonce || !pubkey32 || !keyagg || !session) + if (!ctx || !partial_sig32 || !pubnonce || !pubkey32 || !keyagg || !session) { return UFSECP_ERR_NULL_ARG; + } ctx_clear_err(ctx); Scalar psig; - if (!scalar_parse_strict(partial_sig32, psig)) + if (!scalar_parse_strict(partial_sig32, psig)) { return ctx_set_err(ctx, UFSECP_ERR_BAD_SIG, "partial sig >= n"); + } std::array pn_buf; std::memcpy(pn_buf.data(), pubnonce, 66); auto pn = secp256k1::MuSig2PubNonce::deserialize(pn_buf); std::array pk_arr; std::memcpy(pk_arr.data(), pubkey32, 32); secp256k1::MuSig2KeyAggCtx kagg; - { uint32_t nk; std::memcpy(&nk, keyagg, 4); kagg.Q_negated = (keyagg[4] != 0); + { uint32_t nk = 0; std::memcpy(&nk, keyagg, 4); kagg.Q_negated = (keyagg[4] != 0); kagg.Q = point_from_compressed(keyagg + 5); if (kagg.Q.is_infinity()) { return ctx_set_err(ctx, UFSECP_ERR_BAD_KEY, "invalid aggregated key"); @@ -1784,8 +1829,9 @@ ufsecp_error_t ufsecp_musig2_partial_verify( return ctx_set_err(ctx, UFSECP_ERR_BAD_INPUT, "invalid session scalar e"); } sess.R_negated = (session[97] != 0); - if (!secp256k1::musig2_partial_verify(psig, pn, pk_arr, kagg, sess, signer_index)) + if (!secp256k1::musig2_partial_verify(psig, pn, pk_arr, kagg, sess, signer_index)) { return ctx_set_err(ctx, UFSECP_ERR_VERIFY_FAIL, "partial sig verify failed"); + } return UFSECP_OK; } @@ -1798,8 +1844,9 @@ ufsecp_error_t ufsecp_musig2_partial_sig_agg( ctx_clear_err(ctx); std::vector psigs(n); for (size_t i = 0; i < n; ++i) { - if (!scalar_parse_strict(partial_sigs + i * 32, psigs[i])) + if (!scalar_parse_strict(partial_sigs + i * 32, psigs[i])) { return ctx_set_err(ctx, UFSECP_ERR_BAD_SIG, "partial sig >= n"); + } } secp256k1::MuSig2Session sess; sess.R = point_from_compressed(session); @@ -1828,11 +1875,13 @@ ufsecp_error_t ufsecp_frost_keygen_begin( const uint8_t seed[32], uint8_t* commits_out, size_t* commits_len, uint8_t* shares_out, size_t* shares_len) { - if (!ctx || !seed || !commits_out || !commits_len || !shares_out || !shares_len) + if (!ctx || !seed || !commits_out || !commits_len || !shares_out || !shares_len) { return UFSECP_ERR_NULL_ARG; + } ctx_clear_err(ctx); - if (threshold < 2 || threshold > num_participants) + if (threshold < 2 || threshold > num_participants) { return ctx_set_err(ctx, UFSECP_ERR_BAD_INPUT, "invalid threshold"); + } std::array seed_arr; std::memcpy(seed_arr.data(), seed, 32); auto [commit, shares] = secp256k1::frost_keygen_begin( @@ -1841,9 +1890,10 @@ ufsecp_error_t ufsecp_frost_keygen_begin( /* Serialize commitment: coeff count(4) + from(4) + coeffs(33 each) */ const size_t coeff_count = commit.coeffs.size(); const size_t needed_commits = 8 + coeff_count * 33; - if (*commits_len < needed_commits) + if (*commits_len < needed_commits) { return ctx_set_err(ctx, UFSECP_ERR_BUF_TOO_SMALL, "commits buffer too small"); - uint32_t cc32 = static_cast(coeff_count); + } + const auto cc32 = static_cast(coeff_count); std::memcpy(commits_out, &cc32, 4); std::memcpy(commits_out + 4, &commit.from, 4); for (size_t i = 0; i < coeff_count; ++i) { @@ -1852,9 +1902,10 @@ ufsecp_error_t ufsecp_frost_keygen_begin( } *commits_len = 8 + coeff_count * 33; /* Serialize shares */ - size_t needed_shares = shares.size() * UFSECP_FROST_SHARE_LEN; - if (*shares_len < needed_shares) + const size_t needed_shares = shares.size() * UFSECP_FROST_SHARE_LEN; + if (*shares_len < needed_shares) { return ctx_set_err(ctx, UFSECP_ERR_BUF_TOO_SMALL, "shares buffer too small"); + } for (size_t i = 0; i < shares.size(); ++i) { uint8_t* s = shares_out + i * UFSECP_FROST_SHARE_LEN; std::memcpy(s, &shares[i].from, 4); @@ -1902,7 +1953,7 @@ ufsecp_error_t ufsecp_frost_keygen_finalize( commits.push_back(std::move(fc)); } /* Deserialize shares */ - size_t n_shares = shares_len / UFSECP_FROST_SHARE_LEN; + const size_t n_shares = shares_len / UFSECP_FROST_SHARE_LEN; std::vector shares(n_shares); for (size_t i = 0; i < n_shares; ++i) { const uint8_t* s = received_shares + i * UFSECP_FROST_SHARE_LEN; @@ -1971,8 +2022,9 @@ ufsecp_error_t ufsecp_frost_sign( const uint8_t msg32[32], const uint8_t* nonce_commits, size_t n_signers, uint8_t partial_sig_out[36]) { - if (!ctx || !keypkg || !nonce || !msg32 || !nonce_commits || !partial_sig_out) + if (!ctx || !keypkg || !nonce || !msg32 || !nonce_commits || !partial_sig_out) { return UFSECP_ERR_NULL_ARG; + } ctx_clear_err(ctx); secp256k1::FrostKeyPackage kp; std::memcpy(&kp.id, keypkg, 4); @@ -2044,8 +2096,9 @@ ufsecp_error_t ufsecp_frost_verify_partial( } psig.z_i = z; auto vs = point_from_compressed(verification_share33); - if (vs.is_infinity()) + if (vs.is_infinity()) { return ctx_set_err(ctx, UFSECP_ERR_BAD_PUBKEY, "invalid verification share"); + } std::vector ncs(n_signers); secp256k1::FrostNonceCommitment signer_commit{}; bool found_signer = false; @@ -2065,16 +2118,19 @@ ufsecp_error_t ufsecp_frost_verify_partial( found_signer = true; } } - if (!found_signer) + if (!found_signer) { return ctx_set_err(ctx, UFSECP_ERR_BAD_INPUT, "partial_sig.id not found in nonce_commits"); + } auto gp = point_from_compressed(group_pubkey33); - if (gp.is_infinity()) + if (gp.is_infinity()) { return ctx_set_err(ctx, UFSECP_ERR_BAD_PUBKEY, "invalid group public key"); + } std::array msg_arr; std::memcpy(msg_arr.data(), msg32, 32); - bool ok = secp256k1::frost_verify_partial(psig, signer_commit, vs, msg_arr, ncs, gp); - if (!ok) + const bool ok = secp256k1::frost_verify_partial(psig, signer_commit, vs, msg_arr, ncs, gp); + if (!ok) { return ctx_set_err(ctx, UFSECP_ERR_VERIFY_FAIL, "FROST partial signature verification failed"); + } return UFSECP_OK; } @@ -2135,8 +2191,9 @@ ufsecp_error_t ufsecp_schnorr_adaptor_sign( const uint8_t adaptor_point33[33], const uint8_t aux_rand[32], uint8_t pre_sig_out[UFSECP_SCHNORR_ADAPTOR_SIG_LEN]) { - if (!ctx || !privkey || !msg32 || !adaptor_point33 || !aux_rand || !pre_sig_out) + if (!ctx || !privkey || !msg32 || !adaptor_point33 || !aux_rand || !pre_sig_out) { return UFSECP_ERR_NULL_ARG; + } ctx_clear_err(ctx); Scalar sk; if (!scalar_parse_strict_nonzero(privkey, sk)) { @@ -2829,10 +2886,12 @@ ufsecp_error_t ufsecp_coin_wif_encode(ufsecp_ctx* ctx, } auto wif = secp256k1::coins::coin_wif_encode(sk, *coin, true, testnet != 0); secp256k1::detail::secure_erase(&sk, sizeof(sk)); - if (wif.empty()) + if (wif.empty()) { return ctx_set_err(ctx, UFSECP_ERR_INTERNAL, "WIF encode failed"); - if (*wif_len < wif.size() + 1) + } + if (*wif_len < wif.size() + 1) { return ctx_set_err(ctx, UFSECP_ERR_BUF_TOO_SMALL, "WIF buffer too small"); + } std::memcpy(wif_out, wif.c_str(), wif.size() + 1); *wif_len = wif.size(); return UFSECP_OK; @@ -3144,7 +3203,7 @@ ufsecp_error_t ufsecp_eth_address(ufsecp_ctx* ctx, if (!ctx || !pubkey33 || !addr20_out) return UFSECP_ERR_NULL_ARG; ctx_clear_err(ctx); - Point pk = point_from_compressed(pubkey33); + const Point pk = point_from_compressed(pubkey33); if (pk.is_infinity()) { return ctx_set_err(ctx, UFSECP_ERR_BAD_PUBKEY, "invalid compressed pubkey"); } @@ -3164,12 +3223,12 @@ ufsecp_error_t ufsecp_eth_address_checksummed(ufsecp_ctx* ctx, return ctx_set_err(ctx, UFSECP_ERR_BUF_TOO_SMALL, "need >= 43 bytes for ETH address"); } - Point pk = point_from_compressed(pubkey33); + const Point pk = point_from_compressed(pubkey33); if (pk.is_infinity()) { return ctx_set_err(ctx, UFSECP_ERR_BAD_PUBKEY, "invalid compressed pubkey"); } - std::string addr_str = secp256k1::coins::ethereum_address(pk); + const std::string addr_str = secp256k1::coins::ethereum_address(pk); std::memcpy(addr_out, addr_str.c_str(), addr_str.size()); addr_out[addr_str.size()] = '\0'; *addr_len = addr_str.size();