From 408959e02656f16902070049d300a2becd70f2c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kyle=20=F0=9F=90=86?= Date: Thu, 12 Feb 2026 14:02:45 -0500 Subject: [PATCH 1/6] Harden codebase per NASA/JPL Power of Ten rules - Add KEEP_ASSERT macro with 58 precondition checks on crypto/storage paths - Replace all hot-path malloc/free with bounded stack/static buffers - Split 4 oversized functions (frost_sign, frost_commit, frost_session_resume, app_main) - Consolidate duplicate logging macros into log_compat.h --- main/error_codes.c | 8 ++ main/error_codes.h | 28 +++++ main/frost_coordinator.c | 157 ++++++++++++------------ main/frost_crypto_ops.c | 46 ++++--- main/frost_dkg.c | 23 +--- main/frost_signer.c | 233 +++++++++++++++++++++++------------- main/log_compat.h | 22 ++++ main/main.c | 10 +- main/nostr_frost.h | 3 +- main/nostr_frost_sign.c | 34 ++---- main/policy.c | 6 + main/protocol.c | 12 +- main/protocol.h | 2 +- main/random_utils.c | 9 +- main/storage.c | 9 ++ main/storage_checkpoint.c | 45 +++---- main/storage_crypto.c | 24 ++-- main/storage_export.c | 5 +- main/storage_metadata.c | 6 + test/native/test_protocol.c | 4 +- 20 files changed, 405 insertions(+), 281 deletions(-) create mode 100644 main/log_compat.h diff --git a/main/error_codes.c b/main/error_codes.c index 7ce0bf2..3083ee8 100644 --- a/main/error_codes.c +++ b/main/error_codes.c @@ -27,6 +27,8 @@ const char *error_category_name(int code) { return "psbt"; case ERR_CAT_NOSTR: return "nostr"; + case ERR_CAT_ASSERT: + return "assert"; default: return "unknown"; } @@ -185,6 +187,9 @@ const char *error_name(int code) { case ERR_NOSTR_RELAY: return "NOSTR_RELAY"; + case ERR_ASSERT: + return "ASSERT"; + default: return "UNKNOWN"; } @@ -343,6 +348,9 @@ const char *error_to_string(int code) { case ERR_NOSTR_RELAY: return "Nostr relay error"; + case ERR_ASSERT: + return "Assertion failed"; + default: return "Unknown error"; } diff --git a/main/error_codes.h b/main/error_codes.h index 52133f6..eedd81b 100644 --- a/main/error_codes.h +++ b/main/error_codes.h @@ -17,6 +17,7 @@ #define ERR_CAT_SE 0x0900 #define ERR_CAT_PSBT 0x0A00 #define ERR_CAT_NOSTR 0x0B00 +#define ERR_CAT_ASSERT 0x0C00 #define ERROR_CATEGORY(code) ((code) & 0xFF00) #define ERROR_DETAIL(code) ((code) & 0x00FF) @@ -102,6 +103,33 @@ #define ERR_NOSTR_SIGN (ERR_CAT_NOSTR | 0x03) #define ERR_NOSTR_RELAY (ERR_CAT_NOSTR | 0x04) +#define ERR_ASSERT (ERR_CAT_ASSERT | 0x01) + +#ifdef ESP_PLATFORM +#include "esp_log.h" +#define KEEP_ASSERT_LOG(expr_str) ESP_LOGE("ASSERT", "%s:%d: %s", __FILE__, __LINE__, expr_str) +#else +#include +#define KEEP_ASSERT_LOG(expr_str) \ + fprintf(stderr, "E (ASSERT): %s:%d: %s\n", __FILE__, __LINE__, expr_str) +#endif + +#define KEEP_ASSERT(expr) \ + do { \ + if (!(expr)) { \ + KEEP_ASSERT_LOG(#expr); \ + return ERR_ASSERT; \ + } \ + } while (0) + +#define KEEP_ASSERT_VOID(expr) \ + do { \ + if (!(expr)) { \ + KEEP_ASSERT_LOG(#expr); \ + return; \ + } \ + } while (0) + const char *error_to_string(int code); const char *error_name(int code); const char *error_category_name(int code); diff --git a/main/frost_coordinator.c b/main/frost_coordinator.c index c40c13b..ef9aafd 100644 --- a/main/frost_coordinator.c +++ b/main/frost_coordinator.c @@ -11,19 +11,17 @@ #include #include +#include "log_compat.h" + #ifdef ESP_PLATFORM -#include "esp_log.h" #include "esp_websocket_client.h" #include #include -#else -#include -#define ESP_LOGI(tag, fmt, ...) printf("[%s] " fmt "\n", tag, ##__VA_ARGS__) -#define ESP_LOGE(tag, fmt, ...) printf("[%s] ERROR: " fmt "\n", tag, ##__VA_ARGS__) -#define ESP_LOGW(tag, fmt, ...) printf("[%s] WARN: " fmt "\n", tag, ##__VA_ARGS__) #endif -#define TAG "frost_coord" +#define TAG "frost_coord" +#define MAX_WS_MESSAGE_SIZE 4096 +#define MAX_PUBLISH_MSG_SIZE 4108 typedef struct { char url[RELAY_URL_LEN]; @@ -71,83 +69,80 @@ static void websocket_event_handler(void *handler_args, esp_event_base_t base, i break; case WEBSOCKET_EVENT_DATA: - if (data->op_code == 0x01 && data->data_len > 0) { - char *msg = malloc(data->data_len + 1); - if (msg) { - memcpy(msg, data->data_ptr, data->data_len); - msg[data->data_len] = '\0'; - - cJSON *arr = cJSON_Parse(msg); - if (arr && cJSON_IsArray(arr) && cJSON_GetArraySize(arr) >= 1) { - cJSON *type = cJSON_GetArrayItem(arr, 0); - if (type && cJSON_IsString(type)) { - if (strcmp(type->valuestring, "EVENT") == 0 && - cJSON_GetArraySize(arr) >= 3) { - cJSON *event = cJSON_GetArrayItem(arr, 2); - if (event && cJSON_IsObject(event)) { - cJSON *kind = cJSON_GetObjectItem(event, "kind"); - if (kind && cJSON_IsNumber(kind)) { - char *event_str = cJSON_PrintUnformatted(event); - if (event_str) { - int k = kind->valueint; - if (k == FROST_KIND_SIGN_REQUEST && - g_ctx.callbacks.on_sign_request) { - frost_sign_request_t req; - if (frost_parse_sign_request( - event_str, &g_ctx.current_group, g_ctx.privkey, - &req) == 0) { - g_ctx.callbacks.on_sign_request( - &req, g_ctx.callbacks.user_ctx); - frost_sign_request_free(&req); - } - } else if (k == FROST_KIND_SIGN_RESPONSE && - g_ctx.callbacks.on_sign_response) { - frost_sign_response_t resp; - if (frost_parse_sign_response( - event_str, &g_ctx.current_group, g_ctx.privkey, - &resp) == 0) { - g_ctx.callbacks.on_sign_response( - &resp, g_ctx.callbacks.user_ctx); - } - } else if (k == FROST_KIND_DKG_ROUND1 && - g_ctx.callbacks.on_dkg_round1) { - frost_dkg_round1_t r1; - if (frost_parse_dkg_round1_event( - event_str, &g_ctx.current_group, g_ctx.privkey, - &r1) == 0) { - g_ctx.callbacks.on_dkg_round1( - &r1, g_ctx.callbacks.user_ctx); - } - } else if (k == FROST_KIND_DKG_ROUND2 && - g_ctx.callbacks.on_dkg_round2) { - frost_dkg_round2_t r2; - if (frost_parse_dkg_round2_event( - event_str, &g_ctx.current_group, g_ctx.privkey, - &r2) == 0) { - g_ctx.callbacks.on_dkg_round2( - &r2, g_ctx.callbacks.user_ctx); - } - } else if (k == NIP46_KIND_NOSTR_CONNECT && - g_ctx.callbacks.on_nip46_request) { - nip46_request_t nip46_req; - if (frost_parse_nip46_event(event_str, g_ctx.privkey, - &nip46_req) == 0) { - g_ctx.callbacks.on_nip46_request( - &nip46_req, g_ctx.callbacks.user_ctx); - frost_nip46_request_free(&nip46_req); - } + if (data->op_code == 0x01 && data->data_len > 0 && + (size_t)data->data_len < MAX_WS_MESSAGE_SIZE) { + char msg[MAX_WS_MESSAGE_SIZE]; + memcpy(msg, data->data_ptr, data->data_len); + msg[data->data_len] = '\0'; + + cJSON *arr = cJSON_Parse(msg); + if (arr && cJSON_IsArray(arr) && cJSON_GetArraySize(arr) >= 1) { + cJSON *type = cJSON_GetArrayItem(arr, 0); + if (type && cJSON_IsString(type)) { + if (strcmp(type->valuestring, "EVENT") == 0 && cJSON_GetArraySize(arr) >= 3) { + cJSON *event = cJSON_GetArrayItem(arr, 2); + if (event && cJSON_IsObject(event)) { + cJSON *kind = cJSON_GetObjectItem(event, "kind"); + if (kind && cJSON_IsNumber(kind)) { + char *event_str = cJSON_PrintUnformatted(event); + if (event_str) { + int k = kind->valueint; + if (k == FROST_KIND_SIGN_REQUEST && + g_ctx.callbacks.on_sign_request) { + frost_sign_request_t req; + if (frost_parse_sign_request(event_str, + &g_ctx.current_group, + g_ctx.privkey, &req) == 0) { + g_ctx.callbacks.on_sign_request( + &req, g_ctx.callbacks.user_ctx); + frost_sign_request_free(&req); + } + } else if (k == FROST_KIND_SIGN_RESPONSE && + g_ctx.callbacks.on_sign_response) { + frost_sign_response_t resp; + if (frost_parse_sign_response(event_str, + &g_ctx.current_group, + g_ctx.privkey, &resp) == 0) { + g_ctx.callbacks.on_sign_response( + &resp, g_ctx.callbacks.user_ctx); + } + } else if (k == FROST_KIND_DKG_ROUND1 && + g_ctx.callbacks.on_dkg_round1) { + frost_dkg_round1_t r1; + if (frost_parse_dkg_round1_event(event_str, + &g_ctx.current_group, + g_ctx.privkey, &r1) == 0) { + g_ctx.callbacks.on_dkg_round1(&r1, + g_ctx.callbacks.user_ctx); + } + } else if (k == FROST_KIND_DKG_ROUND2 && + g_ctx.callbacks.on_dkg_round2) { + frost_dkg_round2_t r2; + if (frost_parse_dkg_round2_event(event_str, + &g_ctx.current_group, + g_ctx.privkey, &r2) == 0) { + g_ctx.callbacks.on_dkg_round2(&r2, + g_ctx.callbacks.user_ctx); + } + } else if (k == NIP46_KIND_NOSTR_CONNECT && + g_ctx.callbacks.on_nip46_request) { + nip46_request_t nip46_req; + if (frost_parse_nip46_event(event_str, g_ctx.privkey, + &nip46_req) == 0) { + g_ctx.callbacks.on_nip46_request( + &nip46_req, g_ctx.callbacks.user_ctx); + frost_nip46_request_free(&nip46_req); } - free(event_str); } + free(event_str); } } } } - cJSON_Delete(arr); - } else if (arr) { - cJSON_Delete(arr); } - free(msg); + cJSON_Delete(arr); + } else if (arr) { + cJSON_Delete(arr); } } break; @@ -402,11 +397,11 @@ static int publish_event(const char *event_json) { return -1; size_t msg_len = strlen(event_json) + 12; - char *msg = malloc(msg_len); - if (!msg) + if (msg_len > MAX_PUBLISH_MSG_SIZE) return -1; + char msg[MAX_PUBLISH_MSG_SIZE]; - snprintf(msg, msg_len, "[\"EVENT\",%s]", event_json); + snprintf(msg, sizeof(msg), "[\"EVENT\",%s]", event_json); int published = 0; #ifdef ESP_PLATFORM @@ -418,8 +413,6 @@ static int publish_event(const char *event_json) { } } #endif - - free(msg); ESP_LOGI(TAG, "Published to %d relays", published); return published; } diff --git a/main/frost_crypto_ops.c b/main/frost_crypto_ops.c index 42a6ef1..d6423ad 100644 --- a/main/frost_crypto_ops.c +++ b/main/frost_crypto_ops.c @@ -2,13 +2,13 @@ // SPDX-License-Identifier: AGPL-3.0-or-later #include "nostr_frost.h" +#include "error_codes.h" #include "random_utils.h" #include "crypto_asm.h" #include #include #include #include -#include static void safe_str_copy(char *dest, size_t dest_size, const char *src) { if (dest_size == 0) @@ -67,6 +67,13 @@ static secp256k1_context *get_secp_ctx(void) { int frost_dkg_round1_generate(const frost_group_t *group, uint8_t our_index, frost_dkg_round1_t *round1, uint8_t *secret_shares_out, size_t *share_count) { + KEEP_ASSERT(group != NULL); + KEEP_ASSERT(round1 != NULL); + KEEP_ASSERT(secret_shares_out != NULL); + KEEP_ASSERT(share_count != NULL); + KEEP_ASSERT(group->participant_count > 0); + KEEP_ASSERT(group->threshold > 0); + secp256k1_context *ctx = get_secp_ctx(); if (!ctx || !group || !round1 || !secret_shares_out || !share_count) return -1; @@ -77,19 +84,17 @@ int frost_dkg_round1_generate(const frost_group_t *group, uint8_t our_index, if (!vss) return -3; - secp256k1_frost_keygen_secret_share *shares = - malloc(sizeof(secp256k1_frost_keygen_secret_share) * group->participant_count); - if (!shares) { + if (group->participant_count > MAX_GROUP_PARTICIPANTS) { secp256k1_frost_vss_commitments_destroy(vss); return -4; } + secp256k1_frost_keygen_secret_share shares[MAX_GROUP_PARTICIPANTS]; int ret = secp256k1_frost_keygen_dkg_begin( ctx, vss, shares, group->participant_count, group->threshold, our_index, (const unsigned char *)DKG_CONTEXT_TAG, strlen(DKG_CONTEXT_TAG)); if (ret != 1) { - free(shares); secp256k1_frost_vss_commitments_destroy(vss); return -5; } @@ -113,7 +118,6 @@ int frost_dkg_round1_generate(const frost_group_t *group, uint8_t our_index, } *share_count = group->participant_count; - free(shares); secp256k1_frost_vss_commitments_destroy(vss); return 0; } @@ -149,29 +153,34 @@ int frost_dkg_finalize(const frost_group_t *group, const frost_dkg_round1_t *all size_t round1_count, const frost_dkg_share_t *received_shares, size_t share_count, uint8_t our_index, uint8_t our_share[32], uint8_t group_pubkey[33]) { + KEEP_ASSERT(group != NULL); + KEEP_ASSERT(all_round1 != NULL); + KEEP_ASSERT(received_shares != NULL); + KEEP_ASSERT(our_share != NULL); + KEEP_ASSERT(group_pubkey != NULL); + KEEP_ASSERT(round1_count > 0); + KEEP_ASSERT(share_count > 0); + secp256k1_context *ctx = get_secp_ctx(); if (!ctx || !group || !all_round1 || !received_shares || !our_share || !group_pubkey) return -1; if (round1_count != group->participant_count || share_count != group->participant_count) return -2; - secp256k1_frost_vss_commitments **commitments = (secp256k1_frost_vss_commitments **)malloc( - sizeof(secp256k1_frost_vss_commitments *) * round1_count); - if (!commitments) + if (round1_count > MAX_GROUP_PARTICIPANTS) return -3; + secp256k1_frost_vss_commitments *commitments[MAX_GROUP_PARTICIPANTS]; for (size_t i = 0; i < round1_count; i++) { if (all_round1[i].num_coefficients == 0 || all_round1[i].num_coefficients > MAX_THRESHOLD) { for (size_t j = 0; j < i; j++) secp256k1_frost_vss_commitments_destroy(commitments[j]); - free(commitments); return -4; } commitments[i] = secp256k1_frost_vss_commitments_create(all_round1[i].num_coefficients); if (!commitments[i]) { for (size_t j = 0; j < i; j++) secp256k1_frost_vss_commitments_destroy(commitments[j]); - free(commitments); return -5; } commitments[i]->index = all_round1[i].participant_index; @@ -184,14 +193,12 @@ int frost_dkg_finalize(const frost_group_t *group, const frost_dkg_round1_t *all memcpy(commitments[i]->zkp_z, all_round1[i].zkp_z, 32); } - secp256k1_frost_keygen_secret_share *shares = - malloc(sizeof(secp256k1_frost_keygen_secret_share) * share_count); - if (!shares) { + if (share_count > MAX_GROUP_PARTICIPANTS) { for (size_t i = 0; i < round1_count; i++) secp256k1_frost_vss_commitments_destroy(commitments[i]); - free(commitments); return -6; } + secp256k1_frost_keygen_secret_share shares[MAX_GROUP_PARTICIPANTS]; for (size_t i = 0; i < share_count; i++) { shares[i].generator_index = received_shares[i].generator_index; @@ -201,10 +208,8 @@ int frost_dkg_finalize(const frost_group_t *group, const frost_dkg_round1_t *all secp256k1_frost_keypair *keypair = secp256k1_frost_keypair_create(our_index); if (!keypair) { - free(shares); for (size_t i = 0; i < round1_count; i++) secp256k1_frost_vss_commitments_destroy(commitments[i]); - free(commitments); return -7; } @@ -220,10 +225,8 @@ int frost_dkg_finalize(const frost_group_t *group, const frost_dkg_round1_t *all } secp256k1_frost_keypair_destroy(keypair); - free(shares); for (size_t i = 0; i < round1_count; i++) secp256k1_frost_vss_commitments_destroy(commitments[i]); - free(commitments); return ret == 1 ? 0 : -8; } @@ -231,6 +234,11 @@ int frost_dkg_finalize(const frost_group_t *group, const frost_dkg_round1_t *all int frost_sign_partial(const frost_group_t *group, const frost_sign_request_t *request, const uint8_t our_share[32], uint8_t our_index, frost_sign_response_t *response) { + KEEP_ASSERT(group != NULL); + KEEP_ASSERT(request != NULL); + KEEP_ASSERT(our_share != NULL); + KEEP_ASSERT(response != NULL); + if (!group || !request || !our_share || !response) { return -1; } diff --git a/main/frost_dkg.c b/main/frost_dkg.c index 61fbe11..fc73357 100644 --- a/main/frost_dkg.c +++ b/main/frost_dkg.c @@ -9,18 +9,15 @@ #include "random_utils.h" #include #include +#include "log_compat.h" #include #include #include #ifdef ESP_PLATFORM -#include "esp_log.h" #include #else #include -#define ESP_LOGI(tag, fmt, ...) printf("[%s] " fmt "\n", tag, ##__VA_ARGS__) -#define ESP_LOGW(tag, fmt, ...) printf("[%s] WARN: " fmt "\n", tag, ##__VA_ARGS__) -#define ESP_LOGE(tag, fmt, ...) printf("[%s] ERROR: " fmt "\n", tag, ##__VA_ARGS__) #endif static uint64_t get_unix_time(void) { @@ -237,11 +234,13 @@ void dkg_round1_peer(const rpc_request_t *req, rpc_response_t *resp) { memset(peer, 0, sizeof(*peer)); peer->participant_index = req->peer_index; - char *data = strdup(req->dkg_data); - if (!data) { - PROTOCOL_ERROR(resp, req->id, -1, "Memory error"); + size_t dkg_data_len = strlen(req->dkg_data); + if (dkg_data_len >= sizeof(req->dkg_data)) { + PROTOCOL_ERROR(resp, req->id, -1, "dkg_data too long"); return; } + char data[sizeof(req->dkg_data)]; + memcpy(data, req->dkg_data, dkg_data_len + 1); char *num_coeff_str = strstr(data, "num_coefficients\":"); char *coeffs_str = strstr(data, "coefficient_commitments\":\""); @@ -249,7 +248,6 @@ void dkg_round1_peer(const rpc_request_t *req, rpc_response_t *resp) { char *zkp_z_str = strstr(data, "zkp_z\":\""); if (!num_coeff_str || !coeffs_str || !zkp_r_str || !zkp_z_str) { - free(data); PROTOCOL_ERROR(resp, req->id, -1, "Malformed dkg_data"); return; } @@ -257,7 +255,6 @@ void dkg_round1_peer(const rpc_request_t *req, rpc_response_t *resp) { char *endptr; long num_coeff_tmp = strtol(num_coeff_str + 18, &endptr, 10); if (endptr == num_coeff_str + 18 || num_coeff_tmp <= 0 || num_coeff_tmp > MAX_THRESHOLD) { - free(data); PROTOCOL_ERROR(resp, req->id, -1, "Invalid num_coefficients"); return; } @@ -266,7 +263,6 @@ void dkg_round1_peer(const rpc_request_t *req, rpc_response_t *resp) { char *coeffs_start = coeffs_str + 26; char *coeffs_end = strchr(coeffs_start, '"'); if (!coeffs_end) { - free(data); PROTOCOL_ERROR(resp, req->id, -1, "Parse error"); return; } @@ -281,7 +277,6 @@ void dkg_round1_peer(const rpc_request_t *req, rpc_response_t *resp) { strncpy(coeff_hex, coeffs_start + coeff_offset, 128); coeff_hex[128] = '\0'; if (hex_to_bytes(coeff_hex, peer->coefficient_commitments[i], 64) != 64) { - free(data); PROTOCOL_ERROR(resp, req->id, -1, "Invalid coefficient hex"); return; } @@ -293,7 +288,6 @@ void dkg_round1_peer(const rpc_request_t *req, rpc_response_t *resp) { char *zkp_r_start = zkp_r_str + 8; size_t zkp_r_remaining = strlen(zkp_r_start); if (zkp_r_remaining < 128) { - free(data); PROTOCOL_ERROR(resp, req->id, -1, "zkp_r too short"); return; } @@ -301,7 +295,6 @@ void dkg_round1_peer(const rpc_request_t *req, rpc_response_t *resp) { strncpy(zkp_r_hex, zkp_r_start, 128); zkp_r_hex[128] = '\0'; if (hex_to_bytes(zkp_r_hex, peer->zkp_r, 64) != 64) { - free(data); PROTOCOL_ERROR(resp, req->id, -1, "Invalid zkp_r hex"); return; } @@ -309,7 +302,6 @@ void dkg_round1_peer(const rpc_request_t *req, rpc_response_t *resp) { char *zkp_z_start = zkp_z_str + 8; size_t zkp_z_remaining = strlen(zkp_z_start); if (zkp_z_remaining < 64) { - free(data); PROTOCOL_ERROR(resp, req->id, -1, "zkp_z too short"); return; } @@ -317,13 +309,10 @@ void dkg_round1_peer(const rpc_request_t *req, rpc_response_t *resp) { strncpy(zkp_z_hex, zkp_z_start, 64); zkp_z_hex[64] = '\0'; if (hex_to_bytes(zkp_z_hex, peer->zkp_z, 32) != 32) { - free(data); PROTOCOL_ERROR(resp, req->id, -1, "Invalid zkp_z hex"); return; } - free(data); - int ret = frost_dkg_round1_validate(peer); if (ret != 0) { PROTOCOL_ERROR(resp, req->id, -1, "Round 1 validation failed"); diff --git a/main/frost_signer.c b/main/frost_signer.c index 865d4a1..6b0da45 100644 --- a/main/frost_signer.c +++ b/main/frost_signer.c @@ -13,9 +13,8 @@ #include "crypto_asm.h" #include "secresult.h" #include "anti_glitch.h" -#include "esp_log.h" +#include "log_compat.h" #include -#include #ifdef ESP_PLATFORM #include "esp_timer.h" @@ -71,13 +70,8 @@ static void record_consumed_session(const uint8_t *session_id) { do { \ } while (0) #else -#ifdef ESP_PLATFORM #define FROST_LOGI(tag, ...) ESP_LOGI(tag, __VA_ARGS__) #define FROST_LOGW(tag, ...) ESP_LOGW(tag, __VA_ARGS__) -#else -#define FROST_LOGI(tag, fmt, ...) printf("[%s] " fmt "\n", tag, ##__VA_ARGS__) -#define FROST_LOGW(tag, fmt, ...) printf("[%s] WARN: " fmt "\n", tag, ##__VA_ARGS__) -#endif #endif typedef struct { @@ -243,42 +237,46 @@ void frost_get_share_info(const char *group, rpc_response_t *resp) { frost_free(&state); } -void frost_commit(const char *group, const char *session_id_hex, const char *message_hex, - rpc_response_t *resp) { +static int frost_commit_validate(const char *session_id_hex, const char *message_hex, + uint8_t *session_id, uint8_t *message, rpc_response_t *resp) { secresult_t rng_health = rng_is_healthy_secure(); rng_health = ag_verify_condition_secure(rng_health); if (!SECRESULT_IS_TRUE(rng_health)) { PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_INTERNAL, "RNG health check failed, device in safe mode"); - return; + return -1; } - uint8_t session_id[SESSION_ID_LEN]; if (parse_session_id(session_id_hex, session_id, resp) != 0) { - return; + return -1; } if (strlen(message_hex) != SESSION_ID_HEX_LEN) { PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_PARAMS, "message must be 32 bytes"); - return; + return -1; } - uint8_t message[SESSION_ID_LEN]; if (hex_to_bytes(message_hex, message, SESSION_ID_LEN) != SESSION_ID_LEN) { PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_PARAMS, "Invalid message hex"); - return; + return -1; } if (is_session_consumed(session_id)) { PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_SIGN, "Session ID already used"); - return; + return -1; } if (find_session(session_id) != NULL) { PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_SIGN, "Session ID already active"); - return; + return -1; } + return 0; +} + +static int frost_commit_generate(const char *group, const char *session_id_hex, + const uint8_t *session_id, const uint8_t *message, + rpc_response_t *resp) { ag_random_delay_us(100, 1000); bool has_policy = false; @@ -288,14 +286,14 @@ void frost_commit(const char *group, const char *session_id_hex, const char *mes if (!SECRESULT_IS_TRUE(policy_ret)) { secure_memzero(policy_hash, sizeof(policy_hash)); PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_SIGN, "Policy bundle verification failed"); - return; + return -1; } signing_session_t *s = alloc_session(session_id); if (!s) { secure_memzero(policy_hash, sizeof(policy_hash)); PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_SIGN, "No free session slots"); - return; + return -1; } s->has_policy = has_policy; @@ -306,7 +304,7 @@ void frost_commit(const char *group, const char *session_id_hex, const char *mes if (share_store_load_frost_state(store, group, &s->frost_state) != 0) { free_session(s); PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_SHARE, "Share not found"); - return; + return -1; } strncpy(s->group, group, STORAGE_GROUP_LEN); @@ -317,14 +315,14 @@ void frost_commit(const char *group, const char *session_id_hex, const char *mes threshold) != 0) { free_session(s); PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_SIGN, "Failed to init session"); - return; + return -1; } frost_commitment_result_t commit_result; if (frost_create_commitment_pure(&s->frost_state, &s->session, &commit_result) != 0) { free_session(s); PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_SIGN, "Failed to create commitment"); - return; + return -1; } int cp_ret = session_checkpoint_save(&s->session, s->session.our_nonce, s->group); @@ -342,34 +340,41 @@ void frost_commit(const char *group, const char *session_id_hex, const char *mes protocol_success(resp, resp->id, result); FROST_LOGI(TAG, "Created commitment for session %.16s...", session_id_hex); + return 0; } -void frost_sign(const char *group, const char *session_id_hex, const char *commitments_hex, - rpc_response_t *resp) { - secresult_t rng_health = rng_is_healthy_secure(); - rng_health = ag_verify_condition_secure(rng_health); - if (!SECRESULT_IS_TRUE(rng_health)) { - PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_INTERNAL, - "RNG health check failed, device in safe mode"); - return; - } +void frost_commit(const char *group, const char *session_id_hex, const char *message_hex, + rpc_response_t *resp) { + KEEP_ASSERT_VOID(group != NULL); + KEEP_ASSERT_VOID(session_id_hex != NULL); + KEEP_ASSERT_VOID(message_hex != NULL); + KEEP_ASSERT_VOID(resp != NULL); + KEEP_ASSERT_VOID(group[0] != '\0'); uint8_t session_id[SESSION_ID_LEN]; - if (parse_session_id(session_id_hex, session_id, resp) != 0) { - return; - } + uint8_t message[SESSION_ID_LEN]; - signing_session_t *s = find_session(session_id); - if (!s) { - PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_SIGN, "Session not found"); + if (frost_commit_validate(session_id_hex, message_hex, session_id, message, resp) != 0) { return; } - if (ct_compare(s->group, group, strlen(group) + 1) != 0) { - PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_PARAMS, "Group mismatch"); - return; + frost_commit_generate(group, session_id_hex, session_id, message, resp); +} + +static int frost_sign_validate_rng(rpc_response_t *resp) { + secresult_t rng_health = rng_is_healthy_secure(); + rng_health = ag_verify_condition_secure(rng_health); + if (!SECRESULT_IS_TRUE(rng_health)) { + PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_INTERNAL, + "RNG health check failed, device in safe mode"); + return -1; } + return 0; +} +static int frost_sign_check_policy(signing_session_t *s, const char *session_id_hex, + const uint8_t *session_id, const char *commitments_hex, + rpc_response_t *resp) { ag_random_delay_us(100, 1000); secresult_t policy_check = verify_policy_unchanged_secure(s->has_policy, s->policy_hash); @@ -377,7 +382,7 @@ void frost_sign(const char *group, const char *session_id_hex, const char *commi if (!SECRESULT_IS_TRUE(policy_check)) { free_session(s); PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_SIGN, "Policy changed during session"); - return; + return -1; } ag_random_delay_us(100, 1000); @@ -385,13 +390,13 @@ void frost_sign(const char *group, const char *session_id_hex, const char *commi int parsed = frost_parse_commitments(commitments_hex, &s->session); if (parsed < 0) { PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_PARAMS, "Invalid commitments format"); - return; + return -1; } uint8_t total_participants = s->session.commitment_count + 1; if (total_participants < s->frost_state.threshold) { PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_SIGN, "Not enough commitments for threshold"); - return; + return -1; } s->session.participant_count = total_participants; s->session.state = SESSION_AWAITING_SHARES; @@ -411,13 +416,18 @@ void frost_sign(const char *group, const char *session_id_hex, const char *commi protocol_success(resp, resp->id, result); FROST_LOGI(TAG, "Returning cached signature share for session %.16s... (retry)", session_id_hex); - return; + return 1; } } PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_SIGN, "Session already consumed"); - return; + return -1; } + return 0; +} + +static void frost_sign_execute(signing_session_t *s, const char *session_id_hex, + const uint8_t *session_id, rpc_response_t *resp) { ag_random_delay_us(100, 1000); bool policy_snapshot = s->has_policy; @@ -472,6 +482,42 @@ void frost_sign(const char *group, const char *session_id_hex, const char *commi FROST_LOGI(TAG, "Created signature share for session %.16s...", session_id_hex); } +void frost_sign(const char *group, const char *session_id_hex, const char *commitments_hex, + rpc_response_t *resp) { + KEEP_ASSERT_VOID(group != NULL); + KEEP_ASSERT_VOID(session_id_hex != NULL); + KEEP_ASSERT_VOID(commitments_hex != NULL); + KEEP_ASSERT_VOID(resp != NULL); + KEEP_ASSERT_VOID(group[0] != '\0'); + + if (frost_sign_validate_rng(resp) != 0) { + return; + } + + uint8_t session_id[SESSION_ID_LEN]; + if (parse_session_id(session_id_hex, session_id, resp) != 0) { + return; + } + + signing_session_t *s = find_session(session_id); + if (!s) { + PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_SIGN, "Session not found"); + return; + } + + if (ct_compare(s->group, group, strlen(group) + 1) != 0) { + PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_PARAMS, "Group mismatch"); + return; + } + + int policy_ret = frost_sign_check_policy(s, session_id_hex, session_id, commitments_hex, resp); + if (policy_ret != 0) { + return; + } + + frost_sign_execute(s, session_id_hex, session_id, resp); +} + void frost_signer_cleanup_stale(void) { uint32_t now = get_time_ms(); for (int i = 0; i < MAX_SESSIONS; i++) { @@ -613,64 +659,64 @@ void frost_export_share(const char *group, rpc_response_t *resp) { frost_free(&state); } -void frost_session_resume(const char *session_id_hex, rpc_response_t *resp) { - uint8_t session_id[SESSION_ID_LEN]; - if (parse_session_id(session_id_hex, session_id, resp) != 0) { - return; - } - +static int frost_session_resume_load(const uint8_t *session_id, session_t *restored_session, + uint8_t *nonce_backup, char *restored_group, + rpc_response_t *resp) { if (find_session(session_id) != NULL) { PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_SIGN, "Session already active"); - return; + return -1; } if (is_session_consumed(session_id)) { PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_SIGN, "Session already used"); - return; + return -1; } - session_t restored_session; - uint8_t nonce_backup[SIGNATURE_LEN]; - char restored_group[STORAGE_GROUP_LEN + 1]; - if (session_checkpoint_load(session_id, &restored_session, nonce_backup, restored_group, - sizeof(restored_group)) != 0) { + if (session_checkpoint_load(session_id, restored_session, nonce_backup, restored_group, + STORAGE_GROUP_LEN + 1) != 0) { PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_SIGN, "No checkpoint found"); - return; + return -1; } if (restored_group[0] == '\0') { - session_destroy(&restored_session); - secure_memzero(nonce_backup, sizeof(nonce_backup)); + session_destroy(restored_session); + secure_memzero(nonce_backup, SIGNATURE_LEN); PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_SIGN, "Checkpoint missing group"); - return; + return -1; } uint32_t now = get_time_ms(); uint32_t extended_timeout = SESSION_TIMEOUT_MS * 10; - if (elapsed_ms(restored_session.created_at, now) > extended_timeout) { + if (elapsed_ms(restored_session->created_at, now) > extended_timeout) { session_checkpoint_clear(session_id); - session_destroy(&restored_session); - secure_memzero(nonce_backup, sizeof(nonce_backup)); + session_destroy(restored_session); + secure_memzero(nonce_backup, SIGNATURE_LEN); PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_SIGN, "Checkpoint expired"); - return; + return -1; } + return 0; +} + +static int frost_session_resume_reconstruct(const uint8_t *session_id, session_t *restored_session, + uint8_t *nonce_backup, const char *restored_group, + const char *session_id_hex, rpc_response_t *resp) { signing_session_t *s = alloc_session(session_id); if (!s) { - session_destroy(&restored_session); - secure_memzero(nonce_backup, sizeof(nonce_backup)); + session_destroy(restored_session); + secure_memzero(nonce_backup, SIGNATURE_LEN); PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_SIGN, "No free session slots"); - return; + return -1; } const share_store_t *store = share_store_default(); if (share_store_load_frost_state(store, restored_group, &s->frost_state) != 0) { free_session(s); - session_destroy(&restored_session); - secure_memzero(nonce_backup, sizeof(nonce_backup)); + session_destroy(restored_session); + secure_memzero(nonce_backup, SIGNATURE_LEN); PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_SHARE, "Share not found for group"); - return; + return -1; } strncpy(s->group, restored_group, STORAGE_GROUP_LEN); @@ -681,11 +727,11 @@ void frost_session_resume(const char *session_id_hex, rpc_response_t *resp) { secresult_t policy_ret = capture_policy_snapshot_secure(&has_policy, policy_hash); if (!SECRESULT_IS_TRUE(policy_ret)) { free_session(s); - session_destroy(&restored_session); - secure_memzero(nonce_backup, sizeof(nonce_backup)); + session_destroy(restored_session); + secure_memzero(nonce_backup, SIGNATURE_LEN); secure_memzero(policy_hash, sizeof(policy_hash)); PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_SIGN, "Policy verification failed"); - return; + return -1; } s->has_policy = has_policy; memcpy(s->policy_hash, policy_hash, 32); @@ -694,18 +740,18 @@ void frost_session_resume(const char *session_id_hex, rpc_response_t *resp) { int clear_ret = session_checkpoint_clear(session_id); if (clear_ret != 0) { free_session(s); - secure_memzero(&restored_session, sizeof(restored_session)); - secure_memzero(nonce_backup, sizeof(nonce_backup)); + secure_memzero(restored_session, sizeof(session_t)); + secure_memzero(nonce_backup, SIGNATURE_LEN); PROTOCOL_ERROR(resp, resp->id, PROTOCOL_ERR_SIGN, "Failed to clear checkpoint"); - return; + return -1; } - memcpy(&s->session, &restored_session, sizeof(session_t)); + memcpy(&s->session, restored_session, sizeof(session_t)); memcpy(s->session.our_nonce, nonce_backup, SIGNATURE_LEN); - secure_memzero(&restored_session, sizeof(restored_session)); - secure_memzero(nonce_backup, sizeof(nonce_backup)); + secure_memzero(restored_session, sizeof(session_t)); + secure_memzero(nonce_backup, SIGNATURE_LEN); - s->session.created_at = now; + s->session.created_at = get_time_ms(); char result[256]; snprintf(result, sizeof(result), @@ -714,6 +760,29 @@ void frost_session_resume(const char *session_id_hex, rpc_response_t *resp) { protocol_success(resp, resp->id, result); FROST_LOGI(TAG, "Resumed session %.16s...", session_id_hex); + return 0; +} + +void frost_session_resume(const char *session_id_hex, rpc_response_t *resp) { + KEEP_ASSERT_VOID(session_id_hex != NULL); + KEEP_ASSERT_VOID(resp != NULL); + + uint8_t session_id[SESSION_ID_LEN]; + if (parse_session_id(session_id_hex, session_id, resp) != 0) { + return; + } + + session_t restored_session; + uint8_t nonce_backup[SIGNATURE_LEN]; + char restored_group[STORAGE_GROUP_LEN + 1]; + + if (frost_session_resume_load(session_id, &restored_session, nonce_backup, restored_group, + resp) != 0) { + return; + } + + frost_session_resume_reconstruct(session_id, &restored_session, nonce_backup, restored_group, + session_id_hex, resp); } void frost_session_list(rpc_response_t *resp) { diff --git a/main/log_compat.h b/main/log_compat.h new file mode 100644 index 0000000..3203423 --- /dev/null +++ b/main/log_compat.h @@ -0,0 +1,22 @@ +// SPDX-FileCopyrightText: © 2026 PrivKey LLC +// SPDX-License-Identifier: AGPL-3.0-or-later + +#ifndef LOG_COMPAT_H +#define LOG_COMPAT_H + +#ifdef ESP_PLATFORM +#include "esp_log.h" +#else +#include +#ifndef ESP_LOGE +#define ESP_LOGE(tag, fmt, ...) fprintf(stderr, "E (%s): " fmt "\n", tag, ##__VA_ARGS__) +#endif +#ifndef ESP_LOGW +#define ESP_LOGW(tag, fmt, ...) fprintf(stderr, "W (%s): " fmt "\n", tag, ##__VA_ARGS__) +#endif +#ifndef ESP_LOGI +#define ESP_LOGI(tag, fmt, ...) printf("I (%s): " fmt "\n", tag, ##__VA_ARGS__) +#endif +#endif + +#endif diff --git a/main/main.c b/main/main.c index 1d91fcd..6d23fac 100644 --- a/main/main.c +++ b/main/main.c @@ -247,7 +247,7 @@ static void handle_bitcoin_parse(const rpc_request_t *req, rpc_response_t *resp) PROTOCOL_ERROR(resp, req->id, PROTOCOL_ERR_INTERNAL, "PSBT not initialized"); return; } - if (!req->psbt) { + if (!req->psbt[0]) { PROTOCOL_ERROR(resp, req->id, PROTOCOL_ERR_PARAMS, "Missing psbt"); return; } @@ -275,7 +275,7 @@ static void handle_bitcoin_sign(const rpc_request_t *req, rpc_response_t *resp) PROTOCOL_ERROR(resp, req->id, PROTOCOL_ERR_INTERNAL, "PSBT not initialized"); return; } - if (!req->psbt) { + if (!req->psbt[0]) { PROTOCOL_ERROR(resp, req->id, PROTOCOL_ERR_PARAMS, "Missing psbt"); return; } @@ -428,7 +428,7 @@ static void handle_request(const rpc_request_t *req, rpc_response_t *resp) { } } -void app_main(void) { +static void app_init(void) { ESP_LOGI(TAG, "================================="); ESP_LOGI(TAG, " Keep Hardware - FROST Signer"); ESP_LOGI(TAG, " Version: %s", VERSION); @@ -496,6 +496,10 @@ void app_main(void) { } else { ESP_LOGW(TAG, "UX backend or show_idle unavailable"); } +} + +void app_main(void) { + app_init(); static char line_buf[PROTOCOL_MAX_MESSAGE_LEN]; static char resp_buf[PROTOCOL_MAX_MESSAGE_LEN]; diff --git a/main/nostr_frost.h b/main/nostr_frost.h index c1ec1bc..a54fc10 100644 --- a/main/nostr_frost.h +++ b/main/nostr_frost.h @@ -25,6 +25,7 @@ #define MAX_RELAYS 4 #define RELAY_URL_LEN 128 #define DKG_CONTEXT_TAG "frost-keygen" +#define MAX_SIGN_PAYLOAD_SIZE 4096 typedef struct { uint8_t npub[32]; @@ -77,7 +78,7 @@ typedef struct { uint8_t group_id[GROUP_ID_LEN]; uint8_t request_id[32]; frost_message_type_t message_type; - uint8_t *payload; + uint8_t payload[MAX_SIGN_PAYLOAD_SIZE]; size_t payload_len; uint32_t nonce_index; uint8_t policy_hash[32]; diff --git a/main/nostr_frost_sign.c b/main/nostr_frost_sign.c index dec06b1..39b4f73 100644 --- a/main/nostr_frost_sign.c +++ b/main/nostr_frost_sign.c @@ -90,22 +90,16 @@ int frost_parse_sign_request(const char *event_json, const frost_group_t *group, cJSON *payload_hex = cJSON_GetObjectItem(inner, "payload"); if (payload_hex && cJSON_IsString(payload_hex)) { size_t hex_len = strlen(payload_hex->valuestring); - if (hex_len > SIZE_MAX - 2) { + if (hex_len / 2 + 1 > MAX_SIGN_PAYLOAD_SIZE) { cJSON_Delete(inner); free(decrypted); cJSON_Delete(root); return -7; } - request->payload = malloc(hex_len / 2 + 1); - if (request->payload) { - int decoded = hex_to_bytes(payload_hex->valuestring, request->payload, - hex_len / 2 + 1); - if (decoded > 0) { - request->payload_len = (size_t)decoded; - } else { - free(request->payload); - request->payload = NULL; - } + int decoded = hex_to_bytes(payload_hex->valuestring, request->payload, + hex_len / 2 + 1); + if (decoded > 0) { + request->payload_len = (size_t)decoded; } } cJSON *nonce_idx = cJSON_GetObjectItem(inner, "nonce_index"); @@ -187,14 +181,10 @@ int frost_create_sign_request(const frost_group_t *group, const frost_sign_reque cJSON *content_obj = cJSON_CreateObject(); cJSON_AddStringToObject(content_obj, "message_type", msg_type_str); cJSON_AddStringToObject(content_obj, "request_id", rid_hex); - if (request->payload && request->payload_len > 0) { - size_t payload_hex_size = request->payload_len * 2 + 1; - char *payload_hex = malloc(payload_hex_size); - if (payload_hex) { - bytes_to_hex(request->payload, request->payload_len, payload_hex, payload_hex_size); - cJSON_AddStringToObject(content_obj, "payload", payload_hex); - free(payload_hex); - } + if (request->payload_len > 0 && request->payload_len <= MAX_SIGN_PAYLOAD_SIZE) { + char payload_hex[MAX_SIGN_PAYLOAD_SIZE * 2 + 1]; + bytes_to_hex(request->payload, request->payload_len, payload_hex, sizeof(payload_hex)); + cJSON_AddStringToObject(content_obj, "payload", payload_hex); } cJSON_AddNumberToObject(content_obj, "nonce_index", request->nonce_index); @@ -428,10 +418,8 @@ int frost_parse_sign_response(const char *event_json, const frost_group_t *group } void frost_sign_request_free(frost_sign_request_t *request) { - if (request && request->payload) { - secure_memzero(request->payload, request->payload_len); - free(request->payload); - request->payload = NULL; + if (request) { + secure_memzero(request->payload, sizeof(request->payload)); request->payload_len = 0; } } diff --git a/main/policy.c b/main/policy.c index 25fcc29..6ec3488 100644 --- a/main/policy.c +++ b/main/policy.c @@ -50,6 +50,8 @@ int policy_init(void) { } int policy_save_bundle(const policy_bundle_t *bundle) { + KEEP_ASSERT(bundle != NULL); + if (!initialized) return POLICY_ERR_STORAGE; if (bundle->version != POLICY_VERSION) @@ -85,6 +87,8 @@ int policy_save_bundle(const policy_bundle_t *bundle) { } int policy_load_bundle(policy_bundle_t *bundle) { + KEEP_ASSERT(bundle != NULL); + if (!initialized) return POLICY_ERR_STORAGE; @@ -142,6 +146,8 @@ bool policy_has_bundle(void) { } static int policy_verify_signature(const policy_bundle_t *bundle) { + KEEP_ASSERT(bundle != NULL); + secp256k1_context *ctx = secp256k1_context_create(SECP256K1_CONTEXT_VERIFY); if (!ctx) return POLICY_ERR_INVALID_SIG; diff --git a/main/protocol.c b/main/protocol.c index a333e4f..590a39e 100644 --- a/main/protocol.c +++ b/main/protocol.c @@ -8,7 +8,6 @@ #include #include #include -#include #include static bool is_valid_base64(const char *str, size_t len) { @@ -185,11 +184,7 @@ int protocol_parse_request(const char *json, rpc_request_t *req) { cJSON_Delete(root); return ERR_PROTOCOL_PARAMS; } - req->psbt = strdup(psbt->valuestring); - if (!req->psbt) { - cJSON_Delete(root); - return ERR_PROTOCOL_INTERNAL; - } + memcpy(req->psbt, psbt->valuestring, len + 1); } cJSON *input_idx = cJSON_GetObjectItem(params, "input_idx"); if (input_idx && cJSON_IsNumber(input_idx)) { @@ -222,10 +217,7 @@ void protocol_free_request(rpc_request_t *req) { if (req) { secure_memzero(req->passphrase, sizeof(req->passphrase)); secure_memzero(req->pin, sizeof(req->pin)); - if (req->psbt) { - free(req->psbt); - req->psbt = NULL; - } + secure_memzero(req->psbt, sizeof(req->psbt)); } } diff --git a/main/protocol.h b/main/protocol.h index a033772..1586296 100644 --- a/main/protocol.h +++ b/main/protocol.h @@ -64,7 +64,7 @@ typedef struct { uint8_t our_index; uint8_t peer_index; char dkg_data[2048]; - char *psbt; + char psbt[PROTOCOL_MAX_PSBT_LEN]; size_t input_idx; char policy_bundle[5120]; char passphrase[256]; diff --git a/main/random_utils.c b/main/random_utils.c index 77a9fbc..0dcaa3a 100644 --- a/main/random_utils.c +++ b/main/random_utils.c @@ -11,17 +11,12 @@ static rng_health_stats_t g_rng_stats = {0}; -#ifdef ESP_PLATFORM -#include "esp_log.h" +#include "log_compat.h" + static const char *TAG = "rng"; #define RNG_LOG_ERROR(fmt, ...) ESP_LOGE(TAG, fmt, ##__VA_ARGS__) #define RNG_LOG_WARN(fmt, ...) ESP_LOGW(TAG, fmt, ##__VA_ARGS__) #define RNG_LOG_INFO(fmt, ...) ESP_LOGI(TAG, fmt, ##__VA_ARGS__) -#else -#define RNG_LOG_ERROR(fmt, ...) ((void)0) -#define RNG_LOG_WARN(fmt, ...) ((void)0) -#define RNG_LOG_INFO(fmt, ...) ((void)0) -#endif int rng_health_check(const uint8_t *buf, size_t len) { if (len < 8) diff --git a/main/storage.c b/main/storage.c index f711109..1986ccf 100644 --- a/main/storage.c +++ b/main/storage.c @@ -403,6 +403,9 @@ int storage_migrate_if_needed(void) { } int storage_save_share(const char *group, const char *share_hex) { + KEEP_ASSERT(group != NULL); + KEEP_ASSERT(share_hex != NULL); + if (!initialized) { return STORAGE_ERR_NOT_INIT; } @@ -496,6 +499,10 @@ int storage_save_share(const char *group, const char *share_hex) { } int storage_load_share(const char *group, char *share_hex, size_t len) { + KEEP_ASSERT(group != NULL); + KEEP_ASSERT(share_hex != NULL); + KEEP_ASSERT(len > 0); + if (!initialized) { return STORAGE_ERR_NOT_INIT; } @@ -556,6 +563,8 @@ int storage_load_share(const char *group, char *share_hex, size_t len) { } int storage_delete_share(const char *group) { + KEEP_ASSERT(group != NULL); + if (!initialized) { return STORAGE_ERR_NOT_INIT; } diff --git a/main/storage_checkpoint.c b/main/storage_checkpoint.c index 8429fd9..e4cf7a3 100644 --- a/main/storage_checkpoint.c +++ b/main/storage_checkpoint.c @@ -8,7 +8,6 @@ #include "esp_partition.h" #include "esp_log.h" #include -#include #define TAG "storage_checkpoint" @@ -35,6 +34,9 @@ typedef struct { _Static_assert(sizeof(checkpoint_header_t) == 96, "checkpoint_header_t must be 96 bytes"); +#define CHECKPOINT_MAX_DATA_SIZE (STORAGE_CHECKPOINT_MAX_SIZE - sizeof(checkpoint_header_t)) +static uint8_t checkpoint_crypto_buf[CHECKPOINT_MAX_DATA_SIZE]; + typedef struct { uint32_t magic; uint8_t session_id[STORAGE_SESSION_ID_LEN]; @@ -115,14 +117,12 @@ int storage_checkpoint_save(const char *session_id, const uint8_t *data, size_t header.counter = checkpoint_get_counter(); header.data_len = (uint16_t)len; - uint8_t *encrypted = malloc(len); - if (!encrypted) - return STORAGE_ERR_IO; + if (len > CHECKPOINT_MAX_DATA_SIZE) + return STORAGE_ERR_INVALID_DATA; int ret = storage_crypto_encrypt(data, len, header.session_id, CHECKPOINT_SESSION_ID_LEN, - header.nonce, encrypted, header.tag); + header.nonce, checkpoint_crypto_buf, header.tag); if (ret != 0) { - free(encrypted); return STORAGE_ERR_ENCRYPT; } @@ -132,21 +132,18 @@ int storage_checkpoint_save(const char *session_id, const uint8_t *data, size_t err = esp_partition_erase_range(checkpoint_partition, 0, erase_size); if (err != ESP_OK) { - secure_memzero(encrypted, len); - free(encrypted); + secure_memzero(checkpoint_crypto_buf, len); return STORAGE_ERR_IO; } err = esp_partition_write(checkpoint_partition, 0, &header, sizeof(header)); if (err != ESP_OK) { - secure_memzero(encrypted, len); - free(encrypted); + secure_memzero(checkpoint_crypto_buf, len); return STORAGE_ERR_IO; } - err = esp_partition_write(checkpoint_partition, sizeof(header), encrypted, len); - secure_memzero(encrypted, len); - free(encrypted); + err = esp_partition_write(checkpoint_partition, sizeof(header), checkpoint_crypto_buf, len); + secure_memzero(checkpoint_crypto_buf, len); if (err != ESP_OK) return STORAGE_ERR_IO; @@ -186,20 +183,18 @@ int storage_checkpoint_load(const char *session_id, uint8_t *data, size_t max_le if (header.counter != current_counter) return STORAGE_ERR_CHECKPOINT_EXPIRED; - uint8_t *encrypted = malloc(header.data_len); - if (!encrypted) - return STORAGE_ERR_IO; + if (header.data_len > CHECKPOINT_MAX_DATA_SIZE) + return STORAGE_ERR_INVALID_DATA; - err = esp_partition_read(checkpoint_partition, sizeof(header), encrypted, header.data_len); + err = esp_partition_read(checkpoint_partition, sizeof(header), checkpoint_crypto_buf, + header.data_len); if (err != ESP_OK) { - free(encrypted); return STORAGE_ERR_IO; } - int ret = storage_crypto_decrypt(encrypted, header.data_len, header.session_id, + int ret = storage_crypto_decrypt(checkpoint_crypto_buf, header.data_len, header.session_id, CHECKPOINT_SESSION_ID_LEN, header.nonce, header.tag, data); - secure_memzero(encrypted, header.data_len); - free(encrypted); + secure_memzero(checkpoint_crypto_buf, header.data_len); if (ret != 0) return STORAGE_ERR_DECRYPT; @@ -311,6 +306,10 @@ static int find_free_checkpoint_slot(void) { } int storage_save_session_checkpoint(const uint8_t *session_id, const void *data, size_t len) { + KEEP_ASSERT(session_id != NULL); + KEEP_ASSERT(data != NULL); + KEEP_ASSERT(len > 0); + if (!storage_is_initialized()) { return STORAGE_ERR_NOT_INIT; } @@ -376,6 +375,10 @@ int storage_save_session_checkpoint(const uint8_t *session_id, const void *data, } int storage_load_session_checkpoint(const uint8_t *session_id, void *data, size_t len) { + KEEP_ASSERT(session_id != NULL); + KEEP_ASSERT(data != NULL); + KEEP_ASSERT(len > 0); + if (!storage_is_initialized()) { return STORAGE_ERR_NOT_INIT; } diff --git a/main/storage_crypto.c b/main/storage_crypto.c index 925cdc7..ea94a6b 100644 --- a/main/storage_crypto.c +++ b/main/storage_crypto.c @@ -11,9 +11,10 @@ #include #include +#include "log_compat.h" + #ifdef ESP_PLATFORM #include "esp_mac.h" -#include "esp_log.h" #include "esp_timer.h" #include "nvs_flash.h" #include "nvs.h" @@ -21,16 +22,7 @@ static uint32_t get_time_ms(void) { return (uint32_t)(esp_timer_get_time() / 1000); } #else -#include #include -#define ESP_LOGW(tag, ...) \ - fprintf(stderr, "W (%s): ", tag); \ - fprintf(stderr, __VA_ARGS__); \ - fprintf(stderr, "\n") -#define ESP_LOGE(tag, ...) \ - fprintf(stderr, "E (%s): ", tag); \ - fprintf(stderr, __VA_ARGS__); \ - fprintf(stderr, "\n") static uint32_t get_time_ms(void) { struct timespec ts; clock_gettime(CLOCK_MONOTONIC, &ts); @@ -249,6 +241,12 @@ static int gcm_init_with_key(mbedtls_gcm_context *gcm) { int storage_crypto_encrypt(const uint8_t *plaintext, size_t plaintext_len, const uint8_t *aad, size_t aad_len, uint8_t nonce[STORAGE_CRYPTO_NONCE_SIZE], uint8_t *ciphertext, uint8_t tag[STORAGE_CRYPTO_TAG_SIZE]) { + KEEP_ASSERT(plaintext != NULL); + KEEP_ASSERT(nonce != NULL); + KEEP_ASSERT(ciphertext != NULL); + KEEP_ASSERT(tag != NULL); + KEEP_ASSERT(plaintext_len > 0); + if (!key_initialized || !plaintext || !nonce || !ciphertext || !tag) { return -1; } @@ -275,6 +273,12 @@ int storage_crypto_encrypt(const uint8_t *plaintext, size_t plaintext_len, const int storage_crypto_decrypt(const uint8_t *ciphertext, size_t ciphertext_len, const uint8_t *aad, size_t aad_len, const uint8_t nonce[STORAGE_CRYPTO_NONCE_SIZE], const uint8_t tag[STORAGE_CRYPTO_TAG_SIZE], uint8_t *plaintext) { + KEEP_ASSERT(ciphertext != NULL); + KEEP_ASSERT(nonce != NULL); + KEEP_ASSERT(tag != NULL); + KEEP_ASSERT(plaintext != NULL); + KEEP_ASSERT(ciphertext_len > 0); + if (!key_initialized || !ciphertext || !nonce || !tag || !plaintext) { return -1; } diff --git a/main/storage_export.c b/main/storage_export.c index 9dffa74..19e4d50 100644 --- a/main/storage_export.c +++ b/main/storage_export.c @@ -15,16 +15,15 @@ #include #include +#include "log_compat.h" + #ifdef ESP_PLATFORM -#include "esp_log.h" #include "esp_timer.h" static uint32_t get_time_ms(void) { return (uint32_t)(esp_timer_get_time() / 1000); } #else #include -#include -#define ESP_LOGW(tag, fmt, ...) fprintf(stderr, "W (%s): " fmt "\n", tag, ##__VA_ARGS__) static uint32_t get_time_ms(void) { struct timespec ts; clock_gettime(CLOCK_MONOTONIC, &ts); diff --git a/main/storage_metadata.c b/main/storage_metadata.c index fc6122e..aeaaea5 100644 --- a/main/storage_metadata.c +++ b/main/storage_metadata.c @@ -89,6 +89,9 @@ static int find_free_metadata_slot(void) { } int storage_save_metadata(const char *group, const group_metadata_t *metadata) { + KEEP_ASSERT(group != NULL); + KEEP_ASSERT(metadata != NULL); + if (!storage_is_initialized()) { return STORAGE_ERR_NOT_INIT; } @@ -186,6 +189,9 @@ int storage_save_metadata(const char *group, const group_metadata_t *metadata) { } int storage_load_metadata(const char *group, group_metadata_t *metadata) { + KEEP_ASSERT(group != NULL); + KEEP_ASSERT(metadata != NULL); + if (!storage_is_initialized()) { return STORAGE_ERR_NOT_INIT; } diff --git a/test/native/test_protocol.c b/test/native/test_protocol.c index 3f001fc..d589222 100644 --- a/test/native/test_protocol.c +++ b/test/native/test_protocol.c @@ -326,8 +326,8 @@ static int test_psbt_allocation(void) { if (protocol_parse_request( "{\"id\":1,\"method\":\"bitcoin_sign\",\"params\":{\"psbt\":\"cHNidP8B\"}}", &req) != 0) FAIL("parse failed"); - if (req.psbt == NULL) - FAIL("psbt should be allocated"); + if (req.psbt[0] == '\0') + FAIL("psbt should be populated"); if (strcmp(req.psbt, "cHNidP8B") != 0) FAIL("wrong psbt value"); protocol_free_request(&req); From a86ea0f9b4fba241335b3121ed0bd07b35522991 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kyle=20=F0=9F=90=86?= Date: Thu, 12 Feb 2026 15:36:36 -0500 Subject: [PATCH 2/6] Fix security and stack safety issues from hardening review - Heap-allocate large buffers in frost_coordinator WebSocket handler - Validate subscription IDs to prevent JSON injection - Replace portMAX_DELAY with finite 10s timeout on WS sends - Check esp_websocket_client_send_text return values - Revert frost_sign_request_t.payload to heap pointer (was 4KB on stack) - Heap-allocate payload_hex in frost_create_sign_request - Add mutex protection for checkpoint_crypto_buf shared buffer - Map all non-protocol errors to -32603 in JSON-RPC responses - Wrap error context output in #ifndef NDEBUG - Clear sensitive fields (share, dkg_data) in protocol_free_request - Secure-zero share_hex and result buffers in DKG round2 - Remove dead code and redundant NULL checks after KEEP_ASSERT - Reduce session resume timeout from 10x to 3x --- main/error_codes.c | 4 ++- main/frost_coordinator.c | 51 ++++++++++++++++++++++++++++--------- main/frost_crypto_ops.c | 18 ++----------- main/frost_dkg.c | 6 ++--- main/frost_signer.c | 2 +- main/nostr_frost.h | 2 +- main/nostr_frost_sign.c | 35 +++++++++++++++++++------ main/protocol.c | 4 +++ main/storage_checkpoint.c | 47 +++++++++++++++++++++++++--------- test/native/test_protocol.c | 4 +-- 10 files changed, 116 insertions(+), 57 deletions(-) diff --git a/main/error_codes.c b/main/error_codes.c index 3083ee8..957616c 100644 --- a/main/error_codes.c +++ b/main/error_codes.c @@ -367,6 +367,8 @@ int error_to_jsonrpc_code(int code) { case ERR_PROTOCOL_INTERNAL: return -32603; default: - return code; + if (code >= -32768 && code <= -32000) + return code; + return -32603; } } diff --git a/main/frost_coordinator.c b/main/frost_coordinator.c index ef9aafd..d781d47 100644 --- a/main/frost_coordinator.c +++ b/main/frost_coordinator.c @@ -22,6 +22,21 @@ #define TAG "frost_coord" #define MAX_WS_MESSAGE_SIZE 4096 #define MAX_PUBLISH_MSG_SIZE 4108 +#define WS_SEND_TIMEOUT_MS 10000 + +static bool is_safe_subscription_id(const char *id) { + if (!id || *id == '\0') return false; + size_t len = 0; + for (const char *p = id; *p; p++) { + char c = *p; + if (!((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || + (c >= '0' && c <= '9') || c == '-' || c == '_')) + return false; + if (++len > 64) + return false; + } + return true; +} typedef struct { char url[RELAY_URL_LEN]; @@ -71,11 +86,13 @@ static void websocket_event_handler(void *handler_args, esp_event_base_t base, i case WEBSOCKET_EVENT_DATA: if (data->op_code == 0x01 && data->data_len > 0 && (size_t)data->data_len < MAX_WS_MESSAGE_SIZE) { - char msg[MAX_WS_MESSAGE_SIZE]; + char *msg = malloc(data->data_len + 1); + if (!msg) break; memcpy(msg, data->data_ptr, data->data_len); msg[data->data_len] = '\0'; cJSON *arr = cJSON_Parse(msg); + free(msg); if (arr && cJSON_IsArray(arr) && cJSON_GetArraySize(arr) >= 1) { cJSON *type = cJSON_GetArrayItem(arr, 0); if (type && cJSON_IsString(type)) { @@ -89,14 +106,15 @@ static void websocket_event_handler(void *handler_args, esp_event_base_t base, i int k = kind->valueint; if (k == FROST_KIND_SIGN_REQUEST && g_ctx.callbacks.on_sign_request) { - frost_sign_request_t req; - if (frost_parse_sign_request(event_str, + frost_sign_request_t *req = malloc(sizeof(*req)); + if (req && frost_parse_sign_request(event_str, &g_ctx.current_group, - g_ctx.privkey, &req) == 0) { + g_ctx.privkey, req) == 0) { g_ctx.callbacks.on_sign_request( - &req, g_ctx.callbacks.user_ctx); - frost_sign_request_free(&req); + req, g_ctx.callbacks.user_ctx); + frost_sign_request_free(req); } + free(req); } else if (k == FROST_KIND_SIGN_RESPONSE && g_ctx.callbacks.on_sign_response) { frost_sign_response_t resp; @@ -348,6 +366,8 @@ int frost_coordinator_set_group(const frost_group_t *group) { int frost_coordinator_subscribe(const char *subscription_id) { if (!g_initialized || !g_ctx.has_group) return -1; + if (!is_safe_subscription_id(subscription_id)) + return -2; char pubkey_hex[65]; bytes_to_hex(g_ctx.pubkey, 32, pubkey_hex, sizeof(pubkey_hex)); @@ -362,7 +382,7 @@ int frost_coordinator_subscribe(const char *subscription_id) { for (int i = 0; i < g_ctx.relay_count; i++) { relay_connection_t *relay = &g_ctx.relays[i]; if (relay->state == COORDINATOR_STATE_CONNECTED && relay->ws_handle) { - esp_websocket_client_send_text(relay->ws_handle, filter, strlen(filter), portMAX_DELAY); + esp_websocket_client_send_text(relay->ws_handle, filter, strlen(filter), pdMS_TO_TICKS(WS_SEND_TIMEOUT_MS)); ESP_LOGI(TAG, "Subscribed on %s", relay->url); } } @@ -375,6 +395,8 @@ int frost_coordinator_subscribe(const char *subscription_id) { int frost_coordinator_unsubscribe(const char *subscription_id) { if (!g_initialized) return -1; + if (!is_safe_subscription_id(subscription_id)) + return -2; char close_msg[128]; snprintf(close_msg, sizeof(close_msg), "[\"CLOSE\",\"%s\"]", subscription_id); @@ -384,7 +406,7 @@ int frost_coordinator_unsubscribe(const char *subscription_id) { relay_connection_t *relay = &g_ctx.relays[i]; if (relay->state == COORDINATOR_STATE_CONNECTED && relay->ws_handle) { esp_websocket_client_send_text(relay->ws_handle, close_msg, strlen(close_msg), - portMAX_DELAY); + pdMS_TO_TICKS(WS_SEND_TIMEOUT_MS)); } } #endif @@ -399,20 +421,25 @@ static int publish_event(const char *event_json) { size_t msg_len = strlen(event_json) + 12; if (msg_len > MAX_PUBLISH_MSG_SIZE) return -1; - char msg[MAX_PUBLISH_MSG_SIZE]; + char *msg = malloc(msg_len + 1); + if (!msg) + return -1; - snprintf(msg, sizeof(msg), "[\"EVENT\",%s]", event_json); + snprintf(msg, msg_len + 1, "[\"EVENT\",%s]", event_json); int published = 0; #ifdef ESP_PLATFORM for (int i = 0; i < g_ctx.relay_count; i++) { relay_connection_t *relay = &g_ctx.relays[i]; if (relay->state == COORDINATOR_STATE_CONNECTED && relay->ws_handle) { - esp_websocket_client_send_text(relay->ws_handle, msg, strlen(msg), portMAX_DELAY); - published++; + int ret = esp_websocket_client_send_text(relay->ws_handle, msg, strlen(msg), + pdMS_TO_TICKS(WS_SEND_TIMEOUT_MS)); + if (ret >= 0) + published++; } } #endif + free(msg); ESP_LOGI(TAG, "Published to %d relays", published); return published; } diff --git a/main/frost_crypto_ops.c b/main/frost_crypto_ops.c index d6423ad..acf9411 100644 --- a/main/frost_crypto_ops.c +++ b/main/frost_crypto_ops.c @@ -75,7 +75,7 @@ int frost_dkg_round1_generate(const frost_group_t *group, uint8_t our_index, KEEP_ASSERT(group->threshold > 0); secp256k1_context *ctx = get_secp_ctx(); - if (!ctx || !group || !round1 || !secret_shares_out || !share_count) + if (!ctx) return -1; if (group->threshold > MAX_THRESHOLD || group->participant_count > MAX_GROUP_PARTICIPANTS) return -2; @@ -83,11 +83,6 @@ int frost_dkg_round1_generate(const frost_group_t *group, uint8_t our_index, secp256k1_frost_vss_commitments *vss = secp256k1_frost_vss_commitments_create(group->threshold); if (!vss) return -3; - - if (group->participant_count > MAX_GROUP_PARTICIPANTS) { - secp256k1_frost_vss_commitments_destroy(vss); - return -4; - } secp256k1_frost_keygen_secret_share shares[MAX_GROUP_PARTICIPANTS]; int ret = secp256k1_frost_keygen_dkg_begin( @@ -162,7 +157,7 @@ int frost_dkg_finalize(const frost_group_t *group, const frost_dkg_round1_t *all KEEP_ASSERT(share_count > 0); secp256k1_context *ctx = get_secp_ctx(); - if (!ctx || !group || !all_round1 || !received_shares || !our_share || !group_pubkey) + if (!ctx) return -1; if (round1_count != group->participant_count || share_count != group->participant_count) return -2; @@ -193,11 +188,6 @@ int frost_dkg_finalize(const frost_group_t *group, const frost_dkg_round1_t *all memcpy(commitments[i]->zkp_z, all_round1[i].zkp_z, 32); } - if (share_count > MAX_GROUP_PARTICIPANTS) { - for (size_t i = 0; i < round1_count; i++) - secp256k1_frost_vss_commitments_destroy(commitments[i]); - return -6; - } secp256k1_frost_keygen_secret_share shares[MAX_GROUP_PARTICIPANTS]; for (size_t i = 0; i < share_count; i++) { @@ -239,10 +229,6 @@ int frost_sign_partial(const frost_group_t *group, const frost_sign_request_t *r KEEP_ASSERT(our_share != NULL); KEEP_ASSERT(response != NULL); - if (!group || !request || !our_share || !response) { - return -1; - } - memset(response, 0, sizeof(*response)); memcpy(response->request_id, request->request_id, 32); response->participant_index = our_index; diff --git a/main/frost_dkg.c b/main/frost_dkg.c index fc73357..a9aee0d 100644 --- a/main/frost_dkg.c +++ b/main/frost_dkg.c @@ -235,10 +235,6 @@ void dkg_round1_peer(const rpc_request_t *req, rpc_response_t *resp) { peer->participant_index = req->peer_index; size_t dkg_data_len = strlen(req->dkg_data); - if (dkg_data_len >= sizeof(req->dkg_data)) { - PROTOCOL_ERROR(resp, req->id, -1, "dkg_data too long"); - return; - } char data[sizeof(req->dkg_data)]; memcpy(data, req->dkg_data, dkg_data_len + 1); @@ -349,10 +345,12 @@ void dkg_round2(const rpc_request_t *req, rpc_response_t *resp) { first = false; offset += snprintf(result + offset, sizeof(result) - offset, "{\"recipient_index\":%d,\"share\":\"%s\"}", i + 1, share_hex); + secure_memzero(share_hex, sizeof(share_hex)); } offset += snprintf(result + offset, sizeof(result) - offset, "]}"); protocol_success(resp, req->id, result); + secure_memzero(result, sizeof(result)); } void dkg_receive_share(const rpc_request_t *req, rpc_response_t *resp) { diff --git a/main/frost_signer.c b/main/frost_signer.c index 6b0da45..ac4ee2e 100644 --- a/main/frost_signer.c +++ b/main/frost_signer.c @@ -686,7 +686,7 @@ static int frost_session_resume_load(const uint8_t *session_id, session_t *resto } uint32_t now = get_time_ms(); - uint32_t extended_timeout = SESSION_TIMEOUT_MS * 10; + uint32_t extended_timeout = SESSION_TIMEOUT_MS * 3; if (elapsed_ms(restored_session->created_at, now) > extended_timeout) { session_checkpoint_clear(session_id); diff --git a/main/nostr_frost.h b/main/nostr_frost.h index a54fc10..ca44c9f 100644 --- a/main/nostr_frost.h +++ b/main/nostr_frost.h @@ -78,7 +78,7 @@ typedef struct { uint8_t group_id[GROUP_ID_LEN]; uint8_t request_id[32]; frost_message_type_t message_type; - uint8_t payload[MAX_SIGN_PAYLOAD_SIZE]; + uint8_t *payload; size_t payload_len; uint32_t nonce_index; uint8_t policy_hash[32]; diff --git a/main/nostr_frost_sign.c b/main/nostr_frost_sign.c index 39b4f73..8e93ecc 100644 --- a/main/nostr_frost_sign.c +++ b/main/nostr_frost_sign.c @@ -11,7 +11,6 @@ #include #include #include -#include #include int frost_parse_sign_request(const char *event_json, const frost_group_t *group, @@ -90,16 +89,27 @@ int frost_parse_sign_request(const char *event_json, const frost_group_t *group, cJSON *payload_hex = cJSON_GetObjectItem(inner, "payload"); if (payload_hex && cJSON_IsString(payload_hex)) { size_t hex_len = strlen(payload_hex->valuestring); - if (hex_len / 2 + 1 > MAX_SIGN_PAYLOAD_SIZE) { + size_t max_bytes = hex_len / 2 + 1; + if (max_bytes > MAX_SIGN_PAYLOAD_SIZE) { cJSON_Delete(inner); free(decrypted); cJSON_Delete(root); return -7; } + request->payload = malloc(max_bytes); + if (!request->payload) { + cJSON_Delete(inner); + free(decrypted); + cJSON_Delete(root); + return -8; + } int decoded = hex_to_bytes(payload_hex->valuestring, request->payload, - hex_len / 2 + 1); + max_bytes); if (decoded > 0) { request->payload_len = (size_t)decoded; + } else { + free(request->payload); + request->payload = NULL; } } cJSON *nonce_idx = cJSON_GetObjectItem(inner, "nonce_index"); @@ -181,10 +191,15 @@ int frost_create_sign_request(const frost_group_t *group, const frost_sign_reque cJSON *content_obj = cJSON_CreateObject(); cJSON_AddStringToObject(content_obj, "message_type", msg_type_str); cJSON_AddStringToObject(content_obj, "request_id", rid_hex); - if (request->payload_len > 0 && request->payload_len <= MAX_SIGN_PAYLOAD_SIZE) { - char payload_hex[MAX_SIGN_PAYLOAD_SIZE * 2 + 1]; - bytes_to_hex(request->payload, request->payload_len, payload_hex, sizeof(payload_hex)); - cJSON_AddStringToObject(content_obj, "payload", payload_hex); + if (request->payload && request->payload_len > 0 && + request->payload_len <= MAX_SIGN_PAYLOAD_SIZE) { + char *payload_hex = malloc(request->payload_len * 2 + 1); + if (payload_hex) { + bytes_to_hex(request->payload, request->payload_len, payload_hex, + request->payload_len * 2 + 1); + cJSON_AddStringToObject(content_obj, "payload", payload_hex); + free(payload_hex); + } } cJSON_AddNumberToObject(content_obj, "nonce_index", request->nonce_index); @@ -419,7 +434,11 @@ int frost_parse_sign_response(const char *event_json, const frost_group_t *group void frost_sign_request_free(frost_sign_request_t *request) { if (request) { - secure_memzero(request->payload, sizeof(request->payload)); + if (request->payload) { + secure_memzero(request->payload, request->payload_len); + free(request->payload); + request->payload = NULL; + } request->payload_len = 0; } } diff --git a/main/protocol.c b/main/protocol.c index 590a39e..95d530a 100644 --- a/main/protocol.c +++ b/main/protocol.c @@ -218,6 +218,8 @@ void protocol_free_request(rpc_request_t *req) { secure_memzero(req->passphrase, sizeof(req->passphrase)); secure_memzero(req->pin, sizeof(req->pin)); secure_memzero(req->psbt, sizeof(req->psbt)); + secure_memzero(req->share, sizeof(req->share)); + secure_memzero(req->dkg_data, sizeof(req->dkg_data)); } } @@ -255,6 +257,7 @@ int protocol_format_response(const rpc_response_t *resp, char *buf, size_t len) cJSON_AddStringToObject(error, "name", error_name(resp->error_code)); cJSON_AddStringToObject(error, "message", resp->error_msg); cJSON_AddStringToObject(error, "category", error_category_name(resp->error_code)); +#ifndef NDEBUG if (resp->error_ctx.file[0] != '\0') { cJSON *ctx = cJSON_AddObjectToObject(error, "context"); if (ctx) { @@ -263,6 +266,7 @@ int protocol_format_response(const rpc_response_t *resp, char *buf, size_t len) cJSON_AddStringToObject(ctx, "func", resp->error_ctx.func); } } +#endif } cJSON_bool ok = cJSON_PrintPreallocated(root, buf, (int)len, 0); diff --git a/main/storage_checkpoint.c b/main/storage_checkpoint.c index e4cf7a3..1572b72 100644 --- a/main/storage_checkpoint.c +++ b/main/storage_checkpoint.c @@ -7,6 +7,10 @@ #include "crypto_asm.h" #include "esp_partition.h" #include "esp_log.h" +#ifdef ESP_PLATFORM +#include +#include +#endif #include #define TAG "storage_checkpoint" @@ -37,6 +41,19 @@ _Static_assert(sizeof(checkpoint_header_t) == 96, "checkpoint_header_t must be 9 #define CHECKPOINT_MAX_DATA_SIZE (STORAGE_CHECKPOINT_MAX_SIZE - sizeof(checkpoint_header_t)) static uint8_t checkpoint_crypto_buf[CHECKPOINT_MAX_DATA_SIZE]; +#ifdef ESP_PLATFORM +static SemaphoreHandle_t checkpoint_mutex = NULL; +static void checkpoint_lock(void) { + if (checkpoint_mutex) xSemaphoreTake(checkpoint_mutex, portMAX_DELAY); +} +static void checkpoint_unlock(void) { + if (checkpoint_mutex) xSemaphoreGive(checkpoint_mutex); +} +#else +static void checkpoint_lock(void) {} +static void checkpoint_unlock(void) {} +#endif + typedef struct { uint32_t magic; uint8_t session_id[STORAGE_SESSION_ID_LEN]; @@ -60,6 +77,14 @@ static int checkpoint_init(void) { return -1; } +#ifdef ESP_PLATFORM + if (!checkpoint_mutex) { + checkpoint_mutex = xSemaphoreCreateMutex(); + if (!checkpoint_mutex) + return -1; + } +#endif + checkpoint_initialized = true; return 0; } @@ -94,7 +119,7 @@ static void pad_session_id(uint8_t padded[CHECKPOINT_SESSION_ID_LEN], const char int storage_checkpoint_save(const char *session_id, const uint8_t *data, size_t len) { if (!session_id || !data) return STORAGE_ERR_INVALID_DATA; - if (len == 0 || len > STORAGE_CHECKPOINT_MAX_SIZE - sizeof(checkpoint_header_t)) + if (len == 0 || len > CHECKPOINT_MAX_DATA_SIZE) return STORAGE_ERR_INVALID_DATA; if (!storage_crypto_is_initialized()) return STORAGE_ERR_CRYPTO_NOT_INIT; @@ -117,12 +142,12 @@ int storage_checkpoint_save(const char *session_id, const uint8_t *data, size_t header.counter = checkpoint_get_counter(); header.data_len = (uint16_t)len; - if (len > CHECKPOINT_MAX_DATA_SIZE) - return STORAGE_ERR_INVALID_DATA; + checkpoint_lock(); int ret = storage_crypto_encrypt(data, len, header.session_id, CHECKPOINT_SESSION_ID_LEN, header.nonce, checkpoint_crypto_buf, header.tag); if (ret != 0) { + checkpoint_unlock(); return STORAGE_ERR_ENCRYPT; } @@ -133,17 +158,20 @@ int storage_checkpoint_save(const char *session_id, const uint8_t *data, size_t err = esp_partition_erase_range(checkpoint_partition, 0, erase_size); if (err != ESP_OK) { secure_memzero(checkpoint_crypto_buf, len); + checkpoint_unlock(); return STORAGE_ERR_IO; } err = esp_partition_write(checkpoint_partition, 0, &header, sizeof(header)); if (err != ESP_OK) { secure_memzero(checkpoint_crypto_buf, len); + checkpoint_unlock(); return STORAGE_ERR_IO; } err = esp_partition_write(checkpoint_partition, sizeof(header), checkpoint_crypto_buf, len); secure_memzero(checkpoint_crypto_buf, len); + checkpoint_unlock(); if (err != ESP_OK) return STORAGE_ERR_IO; @@ -176,25 +204,26 @@ int storage_checkpoint_load(const char *session_id, uint8_t *data, size_t max_le if (ct_compare(header.session_id, expected_id, CHECKPOINT_SESSION_ID_LEN) != 0) return STORAGE_ERR_NOT_FOUND; - if (header.data_len > max_len || header.data_len > STORAGE_CHECKPOINT_MAX_SIZE - sizeof(header)) + if (header.data_len > max_len || header.data_len > CHECKPOINT_MAX_DATA_SIZE) return STORAGE_ERR_INVALID_DATA; uint32_t current_counter = checkpoint_get_counter(); if (header.counter != current_counter) return STORAGE_ERR_CHECKPOINT_EXPIRED; - if (header.data_len > CHECKPOINT_MAX_DATA_SIZE) - return STORAGE_ERR_INVALID_DATA; + checkpoint_lock(); err = esp_partition_read(checkpoint_partition, sizeof(header), checkpoint_crypto_buf, header.data_len); if (err != ESP_OK) { + checkpoint_unlock(); return STORAGE_ERR_IO; } int ret = storage_crypto_decrypt(checkpoint_crypto_buf, header.data_len, header.session_id, CHECKPOINT_SESSION_ID_LEN, header.nonce, header.tag, data); secure_memzero(checkpoint_crypto_buf, header.data_len); + checkpoint_unlock(); if (ret != 0) return STORAGE_ERR_DECRYPT; @@ -313,9 +342,6 @@ int storage_save_session_checkpoint(const uint8_t *session_id, const void *data, if (!storage_is_initialized()) { return STORAGE_ERR_NOT_INIT; } - if (!session_id || !data) { - return STORAGE_ERR_INVALID_DATA; - } if (!storage_crypto_is_initialized()) { return STORAGE_ERR_CRYPTO_NOT_INIT; } @@ -385,9 +411,6 @@ int storage_load_session_checkpoint(const uint8_t *session_id, void *data, size_ if (!storage_crypto_is_initialized()) { return STORAGE_ERR_CRYPTO_NOT_INIT; } - if (!session_id || !data) { - return STORAGE_ERR_INVALID_DATA; - } int slot_idx = find_checkpoint_slot(session_id); if (slot_idx < 0) { diff --git a/test/native/test_protocol.c b/test/native/test_protocol.c index d589222..b63a779 100644 --- a/test/native/test_protocol.c +++ b/test/native/test_protocol.c @@ -161,7 +161,7 @@ static int test_format_error(void) { FAIL("missing id"); if (strstr(buf, "\"error\"") == NULL) FAIL("missing error"); - if (strstr(buf, "\"code\":-1") == NULL) + if (strstr(buf, "\"code\":-32603") == NULL) FAIL("missing code"); if (strstr(buf, "Share not found") == NULL) FAIL("missing message"); @@ -183,7 +183,7 @@ static int test_format_error_with_context(void) { FAIL("missing id"); if (strstr(buf, "\"error\"") == NULL) FAIL("missing error"); - if (strstr(buf, "\"code\":-2") == NULL) + if (strstr(buf, "\"code\":-32603") == NULL) FAIL("missing code"); if (strstr(buf, "Test error") == NULL) FAIL("missing message"); From 447024bb38bad0c05381184c20527ebab5dd2bfa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kyle=20=F0=9F=90=86?= Date: Thu, 12 Feb 2026 15:50:33 -0500 Subject: [PATCH 3/6] Fix DRAM overflow, clang-format, and clang-tidy CI failures - Revert checkpoint_crypto_buf from 24KB static to heap-allocated (fixes dram0_0_seg overflow by 4280 bytes) - Keep mutex protection for thread safety - Fix clang-format violations in coordinator, sign, checkpoint - Add missing direct includes for clang-tidy misc-include-cleaner --- main/frost_coordinator.c | 20 +++++++++------ main/frost_signer_core.c | 1 + main/frost_signer_storage.c | 3 +++ main/nostr_frost_sign.c | 4 +-- main/storage_checkpoint.c | 51 +++++++++++++++++++++++++++---------- 5 files changed, 55 insertions(+), 24 deletions(-) diff --git a/main/frost_coordinator.c b/main/frost_coordinator.c index d781d47..fb8ef93 100644 --- a/main/frost_coordinator.c +++ b/main/frost_coordinator.c @@ -25,12 +25,14 @@ #define WS_SEND_TIMEOUT_MS 10000 static bool is_safe_subscription_id(const char *id) { - if (!id || *id == '\0') return false; + if (!id || *id == '\0') + return false; size_t len = 0; for (const char *p = id; *p; p++) { char c = *p; - if (!((c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || - (c >= '0' && c <= '9') || c == '-' || c == '_')) + bool valid = (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || + c == '-' || c == '_'; + if (!valid) return false; if (++len > 64) return false; @@ -87,7 +89,8 @@ static void websocket_event_handler(void *handler_args, esp_event_base_t base, i if (data->op_code == 0x01 && data->data_len > 0 && (size_t)data->data_len < MAX_WS_MESSAGE_SIZE) { char *msg = malloc(data->data_len + 1); - if (!msg) break; + if (!msg) + break; memcpy(msg, data->data_ptr, data->data_len); msg[data->data_len] = '\0'; @@ -107,9 +110,9 @@ static void websocket_event_handler(void *handler_args, esp_event_base_t base, i if (k == FROST_KIND_SIGN_REQUEST && g_ctx.callbacks.on_sign_request) { frost_sign_request_t *req = malloc(sizeof(*req)); - if (req && frost_parse_sign_request(event_str, - &g_ctx.current_group, - g_ctx.privkey, req) == 0) { + if (req && frost_parse_sign_request( + event_str, &g_ctx.current_group, + g_ctx.privkey, req) == 0) { g_ctx.callbacks.on_sign_request( req, g_ctx.callbacks.user_ctx); frost_sign_request_free(req); @@ -382,7 +385,8 @@ int frost_coordinator_subscribe(const char *subscription_id) { for (int i = 0; i < g_ctx.relay_count; i++) { relay_connection_t *relay = &g_ctx.relays[i]; if (relay->state == COORDINATOR_STATE_CONNECTED && relay->ws_handle) { - esp_websocket_client_send_text(relay->ws_handle, filter, strlen(filter), pdMS_TO_TICKS(WS_SEND_TIMEOUT_MS)); + esp_websocket_client_send_text(relay->ws_handle, filter, strlen(filter), + pdMS_TO_TICKS(WS_SEND_TIMEOUT_MS)); ESP_LOGI(TAG, "Subscribed on %s", relay->url); } } diff --git a/main/frost_signer_core.c b/main/frost_signer_core.c index 0939334..b14aab2 100644 --- a/main/frost_signer_core.c +++ b/main/frost_signer_core.c @@ -2,6 +2,7 @@ // SPDX-License-Identifier: AGPL-3.0-or-later #include "frost_signer_core.h" +#include "session.h" #include "hex_utils.h" #include "crypto_asm.h" #include diff --git a/main/frost_signer_storage.c b/main/frost_signer_storage.c index c8edef9..4da7872 100644 --- a/main/frost_signer_storage.c +++ b/main/frost_signer_storage.c @@ -2,8 +2,11 @@ // SPDX-License-Identifier: AGPL-3.0-or-later #include "frost_signer_storage.h" +#include "storage.h" +#include "frost.h" #include "hex_utils.h" #include "crypto_asm.h" +#include #include static share_store_t default_store = {.load = storage_load_share, diff --git a/main/nostr_frost_sign.c b/main/nostr_frost_sign.c index 8e93ecc..55cd63d 100644 --- a/main/nostr_frost_sign.c +++ b/main/nostr_frost_sign.c @@ -103,8 +103,8 @@ int frost_parse_sign_request(const char *event_json, const frost_group_t *group, cJSON_Delete(root); return -8; } - int decoded = hex_to_bytes(payload_hex->valuestring, request->payload, - max_bytes); + int decoded = + hex_to_bytes(payload_hex->valuestring, request->payload, max_bytes); if (decoded > 0) { request->payload_len = (size_t)decoded; } else { diff --git a/main/storage_checkpoint.c b/main/storage_checkpoint.c index 1572b72..23c4e20 100644 --- a/main/storage_checkpoint.c +++ b/main/storage_checkpoint.c @@ -11,6 +11,7 @@ #include #include #endif +#include #include #define TAG "storage_checkpoint" @@ -39,19 +40,24 @@ typedef struct { _Static_assert(sizeof(checkpoint_header_t) == 96, "checkpoint_header_t must be 96 bytes"); #define CHECKPOINT_MAX_DATA_SIZE (STORAGE_CHECKPOINT_MAX_SIZE - sizeof(checkpoint_header_t)) -static uint8_t checkpoint_crypto_buf[CHECKPOINT_MAX_DATA_SIZE]; #ifdef ESP_PLATFORM static SemaphoreHandle_t checkpoint_mutex = NULL; + static void checkpoint_lock(void) { - if (checkpoint_mutex) xSemaphoreTake(checkpoint_mutex, portMAX_DELAY); + if (checkpoint_mutex) + xSemaphoreTake(checkpoint_mutex, portMAX_DELAY); } + static void checkpoint_unlock(void) { - if (checkpoint_mutex) xSemaphoreGive(checkpoint_mutex); + if (checkpoint_mutex) + xSemaphoreGive(checkpoint_mutex); } #else -static void checkpoint_lock(void) {} -static void checkpoint_unlock(void) {} +static void checkpoint_lock(void) { +} +static void checkpoint_unlock(void) { +} #endif typedef struct { @@ -144,9 +150,16 @@ int storage_checkpoint_save(const char *session_id, const uint8_t *data, size_t checkpoint_lock(); + uint8_t *encrypted = malloc(len); + if (!encrypted) { + checkpoint_unlock(); + return STORAGE_ERR_IO; + } + int ret = storage_crypto_encrypt(data, len, header.session_id, CHECKPOINT_SESSION_ID_LEN, - header.nonce, checkpoint_crypto_buf, header.tag); + header.nonce, encrypted, header.tag); if (ret != 0) { + free(encrypted); checkpoint_unlock(); return STORAGE_ERR_ENCRYPT; } @@ -157,20 +170,23 @@ int storage_checkpoint_save(const char *session_id, const uint8_t *data, size_t err = esp_partition_erase_range(checkpoint_partition, 0, erase_size); if (err != ESP_OK) { - secure_memzero(checkpoint_crypto_buf, len); + secure_memzero(encrypted, len); + free(encrypted); checkpoint_unlock(); return STORAGE_ERR_IO; } err = esp_partition_write(checkpoint_partition, 0, &header, sizeof(header)); if (err != ESP_OK) { - secure_memzero(checkpoint_crypto_buf, len); + secure_memzero(encrypted, len); + free(encrypted); checkpoint_unlock(); return STORAGE_ERR_IO; } - err = esp_partition_write(checkpoint_partition, sizeof(header), checkpoint_crypto_buf, len); - secure_memzero(checkpoint_crypto_buf, len); + err = esp_partition_write(checkpoint_partition, sizeof(header), encrypted, len); + secure_memzero(encrypted, len); + free(encrypted); checkpoint_unlock(); if (err != ESP_OK) @@ -213,16 +229,23 @@ int storage_checkpoint_load(const char *session_id, uint8_t *data, size_t max_le checkpoint_lock(); - err = esp_partition_read(checkpoint_partition, sizeof(header), checkpoint_crypto_buf, - header.data_len); + uint8_t *encrypted = malloc(header.data_len); + if (!encrypted) { + checkpoint_unlock(); + return STORAGE_ERR_IO; + } + + err = esp_partition_read(checkpoint_partition, sizeof(header), encrypted, header.data_len); if (err != ESP_OK) { + free(encrypted); checkpoint_unlock(); return STORAGE_ERR_IO; } - int ret = storage_crypto_decrypt(checkpoint_crypto_buf, header.data_len, header.session_id, + int ret = storage_crypto_decrypt(encrypted, header.data_len, header.session_id, CHECKPOINT_SESSION_ID_LEN, header.nonce, header.tag, data); - secure_memzero(checkpoint_crypto_buf, header.data_len); + secure_memzero(encrypted, header.data_len); + free(encrypted); checkpoint_unlock(); if (ret != 0) From a5fd7845e670235cd7a12abb072d57cfcd2e9bac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kyle=20=F0=9F=90=86?= Date: Thu, 12 Feb 2026 15:53:36 -0500 Subject: [PATCH 4/6] Check WS send return in subscribe/unsubscribe, fix clang-tidy includes - Log warnings on failed subscribe/unsubscribe sends - Add direct includes to frost_signer_core.c and frost_signer_storage.c for clang-tidy misc-include-cleaner --- main/frost_coordinator.c | 15 ++++++++++----- main/frost_signer_core.c | 2 ++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/main/frost_coordinator.c b/main/frost_coordinator.c index fb8ef93..1986a6d 100644 --- a/main/frost_coordinator.c +++ b/main/frost_coordinator.c @@ -385,9 +385,12 @@ int frost_coordinator_subscribe(const char *subscription_id) { for (int i = 0; i < g_ctx.relay_count; i++) { relay_connection_t *relay = &g_ctx.relays[i]; if (relay->state == COORDINATOR_STATE_CONNECTED && relay->ws_handle) { - esp_websocket_client_send_text(relay->ws_handle, filter, strlen(filter), - pdMS_TO_TICKS(WS_SEND_TIMEOUT_MS)); - ESP_LOGI(TAG, "Subscribed on %s", relay->url); + int ret = esp_websocket_client_send_text(relay->ws_handle, filter, strlen(filter), + pdMS_TO_TICKS(WS_SEND_TIMEOUT_MS)); + if (ret < 0) + ESP_LOGW(TAG, "Subscribe send failed on %s: %d", relay->url, ret); + else + ESP_LOGI(TAG, "Subscribed on %s", relay->url); } } #endif @@ -409,8 +412,10 @@ int frost_coordinator_unsubscribe(const char *subscription_id) { for (int i = 0; i < g_ctx.relay_count; i++) { relay_connection_t *relay = &g_ctx.relays[i]; if (relay->state == COORDINATOR_STATE_CONNECTED && relay->ws_handle) { - esp_websocket_client_send_text(relay->ws_handle, close_msg, strlen(close_msg), - pdMS_TO_TICKS(WS_SEND_TIMEOUT_MS)); + int ret = esp_websocket_client_send_text(relay->ws_handle, close_msg, strlen(close_msg), + pdMS_TO_TICKS(WS_SEND_TIMEOUT_MS)); + if (ret < 0) + ESP_LOGW(TAG, "Unsubscribe send failed on %s: %d", relay->url, ret); } } #endif diff --git a/main/frost_signer_core.c b/main/frost_signer_core.c index b14aab2..7c9e15f 100644 --- a/main/frost_signer_core.c +++ b/main/frost_signer_core.c @@ -2,9 +2,11 @@ // SPDX-License-Identifier: AGPL-3.0-or-later #include "frost_signer_core.h" +#include "frost.h" #include "session.h" #include "hex_utils.h" #include "crypto_asm.h" +#include #include static const uint8_t SESSION_ID_ALL_ONES[SESSION_ID_LEN] = { From 668f777fcbf487cfada2329da64e67c4b7d32955 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kyle=20=F0=9F=90=86?= Date: Thu, 12 Feb 2026 15:59:12 -0500 Subject: [PATCH 5/6] Heap-allocate WS event structs, fix checkpoint mutex races - Heap-allocate frost_sign_response_t, frost_dkg_round1_t (~1.1KB), frost_dkg_round2_t, nip46_request_t in websocket handler - Fix TOCTOU: move checkpoint_lock() before existence check in save - Add mutex protection to storage_checkpoint_clear - Destroy mutex in storage_checkpoint_cleanup - Add direct includes for clang-tidy misc-include-cleaner --- main/frost_coordinator.c | 45 ++++++++++++++++++++++----------------- main/storage_checkpoint.c | 31 +++++++++++++++++++++------ 2 files changed, 50 insertions(+), 26 deletions(-) diff --git a/main/frost_coordinator.c b/main/frost_coordinator.c index 1986a6d..4622209 100644 --- a/main/frost_coordinator.c +++ b/main/frost_coordinator.c @@ -120,40 +120,45 @@ static void websocket_event_handler(void *handler_args, esp_event_base_t base, i free(req); } else if (k == FROST_KIND_SIGN_RESPONSE && g_ctx.callbacks.on_sign_response) { - frost_sign_response_t resp; - if (frost_parse_sign_response(event_str, - &g_ctx.current_group, - g_ctx.privkey, &resp) == 0) { + frost_sign_response_t *resp = malloc(sizeof(*resp)); + if (resp && frost_parse_sign_response( + event_str, &g_ctx.current_group, + g_ctx.privkey, resp) == 0) { g_ctx.callbacks.on_sign_response( - &resp, g_ctx.callbacks.user_ctx); + resp, g_ctx.callbacks.user_ctx); } + free(resp); } else if (k == FROST_KIND_DKG_ROUND1 && g_ctx.callbacks.on_dkg_round1) { - frost_dkg_round1_t r1; - if (frost_parse_dkg_round1_event(event_str, - &g_ctx.current_group, - g_ctx.privkey, &r1) == 0) { - g_ctx.callbacks.on_dkg_round1(&r1, + frost_dkg_round1_t *r1 = malloc(sizeof(*r1)); + if (r1 && frost_parse_dkg_round1_event( + event_str, &g_ctx.current_group, + g_ctx.privkey, r1) == 0) { + g_ctx.callbacks.on_dkg_round1(r1, g_ctx.callbacks.user_ctx); } + free(r1); } else if (k == FROST_KIND_DKG_ROUND2 && g_ctx.callbacks.on_dkg_round2) { - frost_dkg_round2_t r2; - if (frost_parse_dkg_round2_event(event_str, - &g_ctx.current_group, - g_ctx.privkey, &r2) == 0) { - g_ctx.callbacks.on_dkg_round2(&r2, + frost_dkg_round2_t *r2 = malloc(sizeof(*r2)); + if (r2 && frost_parse_dkg_round2_event( + event_str, &g_ctx.current_group, + g_ctx.privkey, r2) == 0) { + g_ctx.callbacks.on_dkg_round2(r2, g_ctx.callbacks.user_ctx); } + free(r2); } else if (k == NIP46_KIND_NOSTR_CONNECT && g_ctx.callbacks.on_nip46_request) { - nip46_request_t nip46_req; - if (frost_parse_nip46_event(event_str, g_ctx.privkey, - &nip46_req) == 0) { + nip46_request_t *nip46_req = malloc(sizeof(*nip46_req)); + if (nip46_req && + frost_parse_nip46_event(event_str, g_ctx.privkey, + nip46_req) == 0) { g_ctx.callbacks.on_nip46_request( - &nip46_req, g_ctx.callbacks.user_ctx); - frost_nip46_request_free(&nip46_req); + nip46_req, g_ctx.callbacks.user_ctx); + frost_nip46_request_free(nip46_req); } + free(nip46_req); } free(event_str); } diff --git a/main/storage_checkpoint.c b/main/storage_checkpoint.c index 23c4e20..6cceee3 100644 --- a/main/storage_checkpoint.c +++ b/main/storage_checkpoint.c @@ -133,11 +133,16 @@ int storage_checkpoint_save(const char *session_id, const uint8_t *data, size_t if (checkpoint_init() != 0) return STORAGE_ERR_NOT_INIT; + checkpoint_lock(); + checkpoint_header_t existing; esp_err_t err = esp_partition_read(checkpoint_partition, 0, &existing, sizeof(existing)); - if (err != ESP_OK) + if (err != ESP_OK) { + checkpoint_unlock(); return STORAGE_ERR_IO; + } if (existing.magic == CHECKPOINT_MAGIC) { + checkpoint_unlock(); return STORAGE_ERR_CHECKPOINT_EXISTS; } @@ -148,8 +153,6 @@ int storage_checkpoint_save(const char *session_id, const uint8_t *data, size_t header.counter = checkpoint_get_counter(); header.data_len = (uint16_t)len; - checkpoint_lock(); - uint8_t *encrypted = malloc(len); if (!encrypted) { checkpoint_unlock(); @@ -263,19 +266,27 @@ int storage_checkpoint_clear(const char *session_id) { if (checkpoint_init() != 0) return STORAGE_ERR_NOT_INIT; + checkpoint_lock(); + checkpoint_header_t header; esp_err_t err = esp_partition_read(checkpoint_partition, 0, &header, sizeof(header)); - if (err != ESP_OK) + if (err != ESP_OK) { + checkpoint_unlock(); return STORAGE_ERR_IO; + } - if (header.magic != CHECKPOINT_MAGIC) + if (header.magic != CHECKPOINT_MAGIC) { + checkpoint_unlock(); return STORAGE_ERR_NOT_FOUND; + } uint8_t expected_id[CHECKPOINT_SESSION_ID_LEN]; pad_session_id(expected_id, session_id); - if (ct_compare(header.session_id, expected_id, CHECKPOINT_SESSION_ID_LEN) != 0) + if (ct_compare(header.session_id, expected_id, CHECKPOINT_SESSION_ID_LEN) != 0) { + checkpoint_unlock(); return STORAGE_ERR_NOT_FOUND; + } checkpoint_increment_counter(); @@ -286,6 +297,8 @@ int storage_checkpoint_clear(const char *session_id) { erase_size = checkpoint_partition->size; err = esp_partition_erase_range(checkpoint_partition, 0, erase_size); + checkpoint_unlock(); + if (err != ESP_OK) return STORAGE_ERR_IO; @@ -554,6 +567,12 @@ bool storage_has_session_checkpoint(const uint8_t *session_id) { } void storage_checkpoint_cleanup(void) { +#ifdef ESP_PLATFORM + if (checkpoint_mutex) { + vSemaphoreDelete(checkpoint_mutex); + checkpoint_mutex = NULL; + } +#endif checkpoint_initialized = false; checkpoint_partition = NULL; checkpoint_counter = 0; From b7223ac18ab32aa92ee780e65d628485750014c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kyle=20=F0=9F=90=86?= Date: Thu, 12 Feb 2026 16:04:07 -0500 Subject: [PATCH 6/6] Fix clang-tidy CI: remove ESP_PLATFORM=0 that triggers esp_log.h include --- .github/workflows/static-analysis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/static-analysis.yml b/.github/workflows/static-analysis.yml index 62cb891..8bb82f6 100644 --- a/.github/workflows/static-analysis.yml +++ b/.github/workflows/static-analysis.yml @@ -102,7 +102,7 @@ jobs: -I ../secp256k1-frost/build/include \ -I components/crypto_asm/include \ -I components/secure_element/include \ - -DESP_PLATFORM=0 + -std=c11 scan-build: runs-on: ubuntu-latest