diff --git a/VERSION b/VERSION index c4c2d2b1..fa209468 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.6.16 +0.6.17 diff --git a/nostr_core/blossom_client.c b/nostr_core/blossom_client.c index 85164a05..9deac0a8 100644 --- a/nostr_core/blossom_client.c +++ b/nostr_core/blossom_client.c @@ -305,11 +305,12 @@ int blossom_upload(const char* server_url, if (private_key) { signer = nostr_signer_local(private_key); + if (!signer) return NOSTR_ERROR_CRYPTO_FAILED; } rc = blossom_upload_with_signer(server_url, data, data_len, content_type, signer, sha256_hex, timeout_seconds, descriptor_out); - nostr_signer_free(signer); + if (signer) nostr_signer_free(signer); return rc; } @@ -366,11 +367,12 @@ int blossom_upload_file(const char* server_url, if (private_key) { signer = nostr_signer_local(private_key); + if (!signer) return NOSTR_ERROR_CRYPTO_FAILED; } rc = blossom_upload_file_with_signer(server_url, file_path, content_type, signer, timeout_seconds, descriptor_out); - nostr_signer_free(signer); + if (signer) nostr_signer_free(signer); return rc; } @@ -562,8 +564,8 @@ int blossom_delete(const char* server_url, const char* sha256_hex, const unsigned char* private_key, int timeout_seconds) { - nostr_signer_t* signer; - int rc; + nostr_signer_t* signer = NULL; + int rc = NOSTR_ERROR_CRYPTO_FAILED; if (!private_key) { return NOSTR_ERROR_INVALID_INPUT; @@ -571,11 +573,13 @@ int blossom_delete(const char* server_url, signer = nostr_signer_local(private_key); if (!signer) { - return NOSTR_ERROR_CRYPTO_FAILED; + goto cleanup; } rc = blossom_delete_with_signer(server_url, sha256_hex, signer, timeout_seconds); - nostr_signer_free(signer); + +cleanup: + if (signer) nostr_signer_free(signer); return rc; } diff --git a/nostr_core/core_relay_pool.c b/nostr_core/core_relay_pool.c index 77567d0e..b7381b02 100644 --- a/nostr_core/core_relay_pool.c +++ b/nostr_core/core_relay_pool.c @@ -698,7 +698,7 @@ int nostr_relay_pool_set_auth(nostr_relay_pool_t* pool, const unsigned char* private_key, int enable) { nostr_signer_t* signer = NULL; - int rc; + int rc = NOSTR_ERROR_CRYPTO_FAILED; if (!pool) { return NOSTR_ERROR_INVALID_INPUT; @@ -713,15 +713,19 @@ int nostr_relay_pool_set_auth(nostr_relay_pool_t* pool, rc = nostr_relay_pool_set_auth_with_signer(pool, signer, enable); if (rc != NOSTR_SUCCESS) { - nostr_signer_free(signer); - return rc; + goto cleanup; } if (signer) { pool->owns_auth_signer = 1; + signer = NULL; /* Ownership transferred to pool */ } - return NOSTR_SUCCESS; + rc = NOSTR_SUCCESS; + +cleanup: + if (signer) nostr_signer_free(signer); + return rc; } int nostr_relay_pool_add_relay(nostr_relay_pool_t* pool, const char* relay_url) { diff --git a/nostr_core/nip001.c b/nostr_core/nip001.c index e2ca5d04..2863e3aa 100644 --- a/nostr_core/nip001.c +++ b/nostr_core/nip001.c @@ -94,10 +94,12 @@ cJSON* nostr_create_and_sign_event(int kind, const char* content, cJSON* tags, c signer = nostr_signer_local(private_key); if (!signer) { - return NULL; + goto cleanup; } out = nostr_create_and_sign_event_with_signer(kind, content, tags, signer, timestamp); + +cleanup: nostr_signer_free(signer); return out; } diff --git a/nostr_core/nip017.c b/nostr_core/nip017.c index e3a6b310..4f75f019 100644 --- a/nostr_core/nip017.c +++ b/nostr_core/nip017.c @@ -216,8 +216,8 @@ cJSON* nostr_nip17_create_file_event(const char* file_url, cJSON* nostr_nip17_create_relay_list_event(const char** relay_urls, int num_relays, const unsigned char* private_key) { - nostr_signer_t* signer; - cJSON* out; + nostr_signer_t* signer = NULL; + cJSON* out = NULL; if (!private_key) { return NULL; @@ -225,10 +225,12 @@ cJSON* nostr_nip17_create_relay_list_event(const char** relay_urls, signer = nostr_signer_local(private_key); if (!signer) { - return NULL; + goto cleanup; } out = nostr_nip17_create_relay_list_event_with_signer(relay_urls, num_relays, signer); + +cleanup: nostr_signer_free(signer); return out; } @@ -274,8 +276,8 @@ int nostr_nip17_send_dm(cJSON* dm_event, cJSON** gift_wraps_out, int max_gift_wraps, long max_delay_sec) { - nostr_signer_t* signer; - int out; + nostr_signer_t* signer = NULL; + int out = -1; if (!sender_private_key) { return -1; @@ -283,11 +285,13 @@ int nostr_nip17_send_dm(cJSON* dm_event, signer = nostr_signer_local(sender_private_key); if (!signer) { - return -1; + goto cleanup; } out = nostr_nip17_send_dm_with_signer(dm_event, recipient_pubkeys, num_recipients, signer, gift_wraps_out, max_gift_wraps, max_delay_sec); + +cleanup: nostr_signer_free(signer); return out; } @@ -351,8 +355,8 @@ int nostr_nip17_send_dm_with_signer(cJSON* dm_event, */ cJSON* nostr_nip17_receive_dm(cJSON* gift_wrap, const unsigned char* recipient_private_key) { - nostr_signer_t* signer; - cJSON* out; + nostr_signer_t* signer = NULL; + cJSON* out = NULL; if (!recipient_private_key) { return NULL; @@ -360,10 +364,12 @@ cJSON* nostr_nip17_receive_dm(cJSON* gift_wrap, signer = nostr_signer_local(recipient_private_key); if (!signer) { - return NULL; + goto cleanup; } out = nostr_nip17_receive_dm_with_signer(gift_wrap, signer); + +cleanup: nostr_signer_free(signer); return out; } diff --git a/nostr_core/nip042.c b/nostr_core/nip042.c index 624b5da2..0beedabb 100644 --- a/nostr_core/nip042.c +++ b/nostr_core/nip042.c @@ -80,8 +80,8 @@ cJSON* nostr_nip42_create_auth_event(const char* challenge, const char* relay_url, const unsigned char* private_key, time_t timestamp) { - nostr_signer_t* signer; - cJSON* evt; + nostr_signer_t* signer = NULL; + cJSON* evt = NULL; if (!private_key) { return NULL; @@ -89,10 +89,12 @@ cJSON* nostr_nip42_create_auth_event(const char* challenge, signer = nostr_signer_local(private_key); if (!signer) { - return NULL; + goto cleanup; } evt = nostr_nip42_create_auth_event_with_signer(challenge, relay_url, signer, timestamp); + +cleanup: nostr_signer_free(signer); return evt; } diff --git a/nostr_core/nip046.c b/nostr_core/nip046.c index aa24ae63..d5e64eea 100644 --- a/nostr_core/nip046.c +++ b/nostr_core/nip046.c @@ -392,14 +392,16 @@ cJSON* nostr_nip46_create_request_event(const nostr_nip46_request_t* request, const unsigned char* sender_private_key, const unsigned char* recipient_public_key, time_t timestamp) { - nostr_signer_t* signer; - cJSON* out; + nostr_signer_t* signer = NULL; + cJSON* out = NULL; if (!sender_private_key) return NULL; signer = nostr_signer_local(sender_private_key); - if (!signer) return NULL; + if (!signer) goto cleanup; out = nostr_nip46_create_request_event_with_signer(request, signer, recipient_public_key, timestamp); + +cleanup: nostr_signer_free(signer); return out; } @@ -434,14 +436,16 @@ cJSON* nostr_nip46_create_response_event(const nostr_nip46_response_t* response, const unsigned char* sender_private_key, const unsigned char* recipient_public_key, time_t timestamp) { - nostr_signer_t* signer; - cJSON* out; + nostr_signer_t* signer = NULL; + cJSON* out = NULL; if (!sender_private_key) return NULL; signer = nostr_signer_local(sender_private_key); - if (!signer) return NULL; + if (!signer) goto cleanup; out = nostr_nip46_create_response_event_with_signer(response, signer, recipient_public_key, timestamp); + +cleanup: nostr_signer_free(signer); return out; } @@ -476,9 +480,9 @@ int nostr_nip46_decrypt_event(cJSON* event, const unsigned char* recipient_private_key, char* output, size_t output_size) { - nostr_signer_t* signer; + nostr_signer_t* signer = NULL; char* decrypted = NULL; - int rc; + int rc = NOSTR_ERROR_NIP46_DECRYPTION_FAILED; if (!recipient_private_key || !output || output_size == 0) { return NOSTR_ERROR_INVALID_INPUT; @@ -490,20 +494,23 @@ int nostr_nip46_decrypt_event(cJSON* event, } rc = nostr_nip46_decrypt_event_with_signer(event, signer, &decrypted); - nostr_signer_free(signer); if (rc != NOSTR_SUCCESS || !decrypted) { - free(decrypted); - return NOSTR_ERROR_NIP46_DECRYPTION_FAILED; + rc = NOSTR_ERROR_NIP46_DECRYPTION_FAILED; + goto cleanup; } if (strlen(decrypted) + 1 > output_size) { - free(decrypted); - return NOSTR_ERROR_INVALID_INPUT; + rc = NOSTR_ERROR_INVALID_INPUT; + goto cleanup; } strcpy(output, decrypted); + rc = NOSTR_SUCCESS; + +cleanup: + nostr_signer_free(signer); free(decrypted); - return NOSTR_SUCCESS; + return rc; } int nostr_nip46_decrypt_event_with_signer(cJSON* event, @@ -792,8 +799,8 @@ int nostr_nip46_create_nostrconnect_url(const nostr_nip46_nostrconnect_url_t* in int nostr_nip46_client_session_init(nostr_nip46_client_session_t* session, const unsigned char* client_private_key, const char* bunker_url) { - nostr_signer_t* signer; - int rc; + nostr_signer_t* signer = NULL; + int rc = NOSTR_ERROR_CRYPTO_FAILED; if (!client_private_key) return NOSTR_ERROR_INVALID_INPUT; signer = nostr_signer_local(client_private_key); @@ -801,12 +808,16 @@ int nostr_nip46_client_session_init(nostr_nip46_client_session_t* session, rc = nostr_nip46_client_session_init_with_signer(session, signer, bunker_url); if (rc != NOSTR_SUCCESS) { - nostr_signer_free(signer); - return rc; + goto cleanup; } session->owns_client_signer = 1; - return NOSTR_SUCCESS; + signer = NULL; /* Ownership transferred to session */ + rc = NOSTR_SUCCESS; + +cleanup: + nostr_signer_free(signer); + return rc; } int nostr_nip46_client_session_init_with_signer(nostr_nip46_client_session_t* session, diff --git a/nostr_core/nip059.c b/nostr_core/nip059.c index fd318a4c..f1ee23a4 100644 --- a/nostr_core/nip059.c +++ b/nostr_core/nip059.c @@ -175,8 +175,8 @@ cJSON* nostr_nip59_create_rumor(int kind, const char* content, cJSON* tags, */ cJSON* nostr_nip59_create_seal(cJSON* rumor, const unsigned char* sender_private_key, const unsigned char* recipient_public_key, long max_delay_sec) { - nostr_signer_t* signer; - cJSON* out; + nostr_signer_t* signer = NULL; + cJSON* out = NULL; if (!sender_private_key) { return NULL; @@ -184,10 +184,12 @@ cJSON* nostr_nip59_create_seal(cJSON* rumor, const unsigned char* sender_private signer = nostr_signer_local(sender_private_key); if (!signer) { - return NULL; + goto cleanup; } out = nostr_nip59_create_seal_with_signer(rumor, signer, recipient_public_key, max_delay_sec); + +cleanup: nostr_signer_free(signer); return out; } @@ -378,8 +380,8 @@ cJSON* nostr_nip59_create_gift_wrap(cJSON* seal, const char* recipient_public_ke * NIP-59: Unwrap a gift wrap to get the seal */ cJSON* nostr_nip59_unwrap_gift(cJSON* gift_wrap, const unsigned char* recipient_private_key) { - nostr_signer_t* signer; - cJSON* out; + nostr_signer_t* signer = NULL; + cJSON* out = NULL; if (!recipient_private_key) { return NULL; @@ -387,10 +389,12 @@ cJSON* nostr_nip59_unwrap_gift(cJSON* gift_wrap, const unsigned char* recipient_ signer = nostr_signer_local(recipient_private_key); if (!signer) { - return NULL; + goto cleanup; } out = nostr_nip59_unwrap_gift_with_signer(gift_wrap, signer); + +cleanup: nostr_signer_free(signer); return out; } @@ -433,8 +437,8 @@ cJSON* nostr_nip59_unwrap_gift_with_signer(cJSON* gift_wrap, nostr_signer_t* sig */ cJSON* nostr_nip59_unseal_rumor(cJSON* seal, const unsigned char* sender_public_key, const unsigned char* recipient_private_key) { - nostr_signer_t* signer; - cJSON* out; + nostr_signer_t* signer = NULL; + cJSON* out = NULL; if (!recipient_private_key) { return NULL; @@ -442,10 +446,12 @@ cJSON* nostr_nip59_unseal_rumor(cJSON* seal, const unsigned char* sender_public_ signer = nostr_signer_local(recipient_private_key); if (!signer) { - return NULL; + goto cleanup; } out = nostr_nip59_unseal_rumor_with_signer(seal, sender_public_key, signer); + +cleanup: nostr_signer_free(signer); return out; } diff --git a/nostr_core/nip060.c b/nostr_core/nip060.c index 5bee94aa..d7cfd88e 100644 --- a/nostr_core/nip060.c +++ b/nostr_core/nip060.c @@ -82,8 +82,8 @@ static int nip60_encrypt_self(const unsigned char* private_key, const char* plaintext, char* output, size_t output_size) { - nostr_signer_t* signer; - int rc; + nostr_signer_t* signer = NULL; + int rc = NOSTR_ERROR_CRYPTO_FAILED; if (!private_key) { return NOSTR_ERROR_INVALID_INPUT; @@ -91,10 +91,12 @@ static int nip60_encrypt_self(const unsigned char* private_key, signer = nostr_signer_local(private_key); if (!signer) { - return NOSTR_ERROR_CRYPTO_FAILED; + goto cleanup; } rc = nip60_encrypt_self_with_signer(signer, plaintext, output, output_size); + +cleanup: nostr_signer_free(signer); return rc; } @@ -287,8 +289,8 @@ cJSON* nostr_nip60_create_wallet_event_with_signer(const nostr_nip60_wallet_data cJSON* nostr_nip60_create_wallet_event(const nostr_nip60_wallet_data_t* wallet_data, const unsigned char* private_key, time_t timestamp) { - nostr_signer_t* signer; - cJSON* evt; + nostr_signer_t* signer = NULL; + cJSON* evt = NULL; if (!private_key) { return NULL; @@ -296,10 +298,12 @@ cJSON* nostr_nip60_create_wallet_event(const nostr_nip60_wallet_data_t* wallet_d signer = nostr_signer_local(private_key); if (!signer) { - return NULL; + goto cleanup; } evt = nostr_nip60_create_wallet_event_with_signer(wallet_data, signer, timestamp); + +cleanup: nostr_signer_free(signer); return evt; } @@ -449,8 +453,8 @@ cJSON* nostr_nip60_create_token_event_with_signer(const nostr_nip60_token_data_t cJSON* nostr_nip60_create_token_event(const nostr_nip60_token_data_t* token_data, const unsigned char* private_key, time_t timestamp) { - nostr_signer_t* signer; - cJSON* evt; + nostr_signer_t* signer = NULL; + cJSON* evt = NULL; if (!private_key) { return NULL; @@ -458,10 +462,12 @@ cJSON* nostr_nip60_create_token_event(const nostr_nip60_token_data_t* token_data signer = nostr_signer_local(private_key); if (!signer) { - return NULL; + goto cleanup; } evt = nostr_nip60_create_token_event_with_signer(token_data, signer, timestamp); + +cleanup: nostr_signer_free(signer); return evt; } @@ -584,15 +590,17 @@ cJSON* nostr_nip60_create_token_deletion_with_signer(const char* token_event_id, cJSON* nostr_nip60_create_token_deletion(const char* token_event_id, const unsigned char* private_key, time_t timestamp) { - nostr_signer_t* signer; - cJSON* evt; + nostr_signer_t* signer = NULL; + cJSON* evt = NULL; if (!private_key) return NULL; signer = nostr_signer_local(private_key); - if (!signer) return NULL; + if (!signer) goto cleanup; evt = nostr_nip60_create_token_deletion_with_signer(token_event_id, signer, timestamp); + +cleanup: nostr_signer_free(signer); return evt; } @@ -616,16 +624,18 @@ cJSON* nostr_nip60_create_rollover_token(const nostr_nip60_token_data_t* remaini int deleted_count, const unsigned char* private_key, time_t timestamp) { - nostr_signer_t* signer; - cJSON* evt; + nostr_signer_t* signer = NULL; + cJSON* evt = NULL; if (!private_key) return NULL; signer = nostr_signer_local(private_key); - if (!signer) return NULL; + if (!signer) goto cleanup; evt = nostr_nip60_create_rollover_token_with_signer(remaining_proofs, deleted_event_ids, deleted_count, signer, timestamp); + +cleanup: nostr_signer_free(signer); return evt; } @@ -690,15 +700,17 @@ cJSON* nostr_nip60_create_history_event_with_signer(const nostr_nip60_history_da cJSON* nostr_nip60_create_history_event(const nostr_nip60_history_data_t* history_data, const unsigned char* private_key, time_t timestamp) { - nostr_signer_t* signer; - cJSON* evt; + nostr_signer_t* signer = NULL; + cJSON* evt = NULL; if (!private_key) return NULL; signer = nostr_signer_local(private_key); - if (!signer) return NULL; + if (!signer) goto cleanup; evt = nostr_nip60_create_history_event_with_signer(history_data, signer, timestamp); + +cleanup: nostr_signer_free(signer); return evt; } @@ -839,15 +851,17 @@ cJSON* nostr_nip60_create_quote_event(const char* quote_id, time_t expiration, const unsigned char* private_key, time_t timestamp) { - nostr_signer_t* signer; - cJSON* evt; + nostr_signer_t* signer = NULL; + cJSON* evt = NULL; if (!private_key) return NULL; signer = nostr_signer_local(private_key); - if (!signer) return NULL; + if (!signer) goto cleanup; evt = nostr_nip60_create_quote_event_with_signer(quote_id, mint_url, expiration, signer, timestamp); + +cleanup: nostr_signer_free(signer); return evt; } diff --git a/nostr_core/nip061.c b/nostr_core/nip061.c index c074ba9e..7bcbf407 100644 --- a/nostr_core/nip061.c +++ b/nostr_core/nip061.c @@ -72,15 +72,17 @@ cJSON* nostr_nip61_create_nutzap_info_event_with_signer(const nostr_nip61_nutzap cJSON* nostr_nip61_create_nutzap_info_event(const nostr_nip61_nutzap_info_t* info, const unsigned char* private_key, time_t timestamp) { - nostr_signer_t* signer; - cJSON* evt; + nostr_signer_t* signer = NULL; + cJSON* evt = NULL; if (!private_key) return NULL; signer = nostr_signer_local(private_key); - if (!signer) return NULL; + if (!signer) goto cleanup; evt = nostr_nip61_create_nutzap_info_event_with_signer(info, signer, timestamp); + +cleanup: nostr_signer_free(signer); return evt; } @@ -287,17 +289,19 @@ cJSON* nostr_nip61_create_nutzap_event_with_signer(const nostr_nip61_nutzap_data } cJSON* nostr_nip61_create_nutzap_event(const nostr_nip61_nutzap_data_t* nutzap_data, - const unsigned char* sender_private_key, - time_t timestamp) { - nostr_signer_t* signer; - cJSON* evt; + const unsigned char* sender_private_key, + time_t timestamp) { + nostr_signer_t* signer = NULL; + cJSON* evt = NULL; if (!sender_private_key) return NULL; signer = nostr_signer_local(sender_private_key); - if (!signer) return NULL; + if (!signer) goto cleanup; evt = nostr_nip61_create_nutzap_event_with_signer(nutzap_data, signer, timestamp); + +cleanup: nostr_signer_free(signer); return evt; } @@ -488,8 +492,8 @@ cJSON* nostr_nip61_create_redemption_event(const char* nutzap_event_id, uint64_t amount, const unsigned char* private_key, time_t timestamp) { - nostr_signer_t* signer; - cJSON* evt; + nostr_signer_t* signer = NULL; + cJSON* evt = NULL; if (!private_key) { return NULL; @@ -497,7 +501,7 @@ cJSON* nostr_nip61_create_redemption_event(const char* nutzap_event_id, signer = nostr_signer_local(private_key); if (!signer) { - return NULL; + goto cleanup; } evt = nostr_nip61_create_redemption_event_with_signer(nutzap_event_id, @@ -508,6 +512,8 @@ cJSON* nostr_nip61_create_redemption_event(const char* nutzap_event_id, amount, signer, timestamp); + +cleanup: nostr_signer_free(signer); return evt; } diff --git a/nostr_core/nostr_core.h b/nostr_core/nostr_core.h index 0ec0c374..d055cdbf 100644 --- a/nostr_core/nostr_core.h +++ b/nostr_core/nostr_core.h @@ -2,10 +2,10 @@ #define NOSTR_CORE_H // Version information (auto-updated by increment_and_push.sh) -#define VERSION "v0.6.16" +#define VERSION "v0.6.17" #define VERSION_MAJOR 0 #define VERSION_MINOR 6 -#define VERSION_PATCH 16 +#define VERSION_PATCH 17 /* * NOSTR Core Library - Complete API Reference diff --git a/plans/m11_wrapper_refactor_plan.md b/plans/m11_wrapper_refactor_plan.md new file mode 100644 index 00000000..4a18c1bc --- /dev/null +++ b/plans/m11_wrapper_refactor_plan.md @@ -0,0 +1,319 @@ +# M-11: Wrapper Function Memory Leak Safety — Refactoring Plan + +**Issue:** Wrapper functions that create temporary `nostr_signer_t` instances may leak memory on error paths. + +**Status:** Deferred from initial audit — requires systematic refactoring + +**Priority:** Medium (no known active leaks, but pattern is fragile) + +--- + +## Problem Statement + +The codebase uses a consistent wrapper pattern where a public function creates a temporary `nostr_signer_t` from a raw private key, delegates to a `_with_signer` variant, and frees the signer. The issue is that **if the inner function fails or returns early, the signer may not be freed**. + +### Current Pattern (27 instances across 8 files) + +```c +cJSON* nostr_nipXX_create_something(const unsigned char* private_key, ...) { + nostr_signer_t* signer; + cJSON* out; + + if (!private_key) return NULL; + signer = nostr_signer_local(private_key); + if (!signer) return NULL; + + out = nostr_nipXX_create_something_with_signer(..., signer, ...); + nostr_signer_free(signer); // ← Only reached on success path + return out; +} +``` + +### Leak Scenarios + +1. **Inner function returns early with error** — The `nostr_signer_free(signer)` is never reached +2. **Inner function allocates resources that fail** — Signer is freed but inner resources may leak +3. **Complex error paths** — Multiple return points make it easy to miss cleanup + +### Affected Files and Functions + +| File | Function | Line | Risk | +|------|----------|------|------| +| [`nip001.c`](nostr_core/nip001.c:95) | `nostr_create_and_sign_event` | 95-102 | Low — simple wrapper | +| [`nip017.c`](nostr_core/nip017.c:226) | `nostr_nip17_create_relay_list_event` | 226-233 | Low — simple wrapper | +| [`nip017.c`](nostr_core/nip017.c:284) | `nostr_nip17_send_dm` | 284-292 | Medium — complex inner function | +| [`nip017.c`](nostr_core/nip017.c:361) | `nostr_nip17_receive_dm` | 361-368 | Low — simple wrapper | +| [`nip042.c`](nostr_core/nip042.c:90) | `nostr_nip42_create_auth_event` | 90-97 | Low — simple wrapper | +| [`nip046.c`](nostr_core/nip046.c:399) | `nostr_nip46_create_request_event` | 399-404 | Low — simple wrapper | +| [`nip046.c`](nostr_core/nip046.c:441) | `nostr_nip46_create_response_event` | 441-446 | Low — simple wrapper | +| [`nip046.c`](nostr_core/nip046.c:487) | `nostr_nip46_decrypt_event` | 487-494 | Medium — error path after free | +| [`nip046.c`](nostr_core/nip046.c:799) | `nostr_nip46_client_session_init` | 799-809 | **High** — conditional free, ownership transfer | +| [`nip059.c`](nostr_core/nip059.c:185) | `nostr_nip59_create_seal` | 185-192 | Low — simple wrapper | +| [`nip059.c`](nostr_core/nip059.c:388) | `nostr_nip59_unwrap_gift` | 388-395 | Low — simple wrapper | +| [`nip059.c`](nostr_core/nip059.c:443) | `nostr_nip59_unseal_rumor` | 443-450 | Low — simple wrapper | +| [`nip060.c`](nostr_core/nip060.c:92) | `nip60_encrypt_self` | 92-99 | Low — simple wrapper | +| [`nip060.c`](nostr_core/nip060.c:297) | `nostr_nip60_create_wallet_event` | 297-304 | Low — simple wrapper | +| [`nip060.c`](nostr_core/nip060.c:459) | `nostr_nip60_create_token_event` | 459-466 | Low — simple wrapper | +| [`nip060.c`](nostr_core/nip060.c:592) | `nostr_nip60_create_token_deletion` | 592-597 | Low — simple wrapper | +| [`nip060.c`](nostr_core/nip060.c:624) | `nostr_nip60_create_rollover_token` | 624-630 | Low — simple wrapper | +| [`nip060.c`](nostr_core/nip060.c:698) | `nostr_nip60_create_history_event` | 698-703 | Low — simple wrapper | +| [`nip060.c`](nostr_core/nip060.c:847) | `nostr_nip60_create_quote_event` | 847-852 | Low — simple wrapper | +| [`nip061.c`](nostr_core/nip061.c:80) | `nostr_nip61_create_nutzap_info_event` | 80-85 | Low — simple wrapper | +| [`nip061.c`](nostr_core/nip061.c:297) | `nostr_nip61_create_nutzap_event` | 297-302 | Low — simple wrapper | +| [`nip061.c`](nostr_core/nip061.c:498) | `nostr_nip61_create_redemption_event` | 498-512 | Low — simple wrapper | +| [`blossom_client.c`](nostr_core/blossom_client.c:307) | `blossom_upload` | 307-313 | Medium — conditional signer creation | +| [`blossom_client.c`](nostr_core/blossom_client.c:368) | `blossom_upload_file` | 368-374 | Medium — conditional signer creation | +| [`blossom_client.c`](nostr_core/blossom_client.c:572) | `blossom_delete` | 572-579 | Low — simple wrapper | +| [`core_relay_pool.c`](nostr_core/core_relay_pool.c:708) | `nostr_relay_pool_set_auth` | 708-717 | **High** — conditional free, error path | + +--- + +## Solution Options + +### Option A: `goto cleanup` Pattern (Recommended) + +**Pros:** +- Explicit cleanup path +- Works with any complexity of inner function +- Easy to audit +- No API changes + +**Cons:** +- `goto` is often discouraged (though acceptable for cleanup in C) +- Requires modifying every wrapper function + +**Example:** +```c +cJSON* nostr_nipXX_create_something(const unsigned char* private_key, ...) { + nostr_signer_t* signer = NULL; + cJSON* out = NULL; + + if (!private_key) return NULL; + + signer = nostr_signer_local(private_key); + if (!signer) return NULL; + + out = nostr_nipXX_create_something_with_signer(..., signer, ...); + +cleanup: + nostr_signer_free(signer); + return out; +} +``` + +### Option B: Inline Signer Creation (Most Invasive) + +**Pros:** +- Eliminates the wrapper pattern entirely +- No temporary signer to leak +- Single code path + +**Cons:** +- Requires changing all `_with_signer` functions to accept private keys +- Duplicates signer creation logic +- Breaks the abstraction layer +- Very large diff + +### Option C: RAII-style Macro (C11 `_Generic` or cleanup attribute) + +**Pros:** +- Automatic cleanup on scope exit +- Minimal code changes +- Compiler-enforced + +**Cons:** +- GCC/Clang specific (`__attribute__((cleanup))`) +- Not portable to MSVC +- May be confusing to readers + +**Example:** +```c +#define SCOPED_SIGNER __attribute__((cleanup(nostr_signer_free_ptr))) + +void nostr_signer_free_ptr(nostr_signer_t** s) { + if (s && *s) nostr_signer_free(*s); +} + +cJSON* nostr_nipXX_create_something(const unsigned char* private_key, ...) { + SCOPED_SIGNER nostr_signer_t* signer = NULL; + // ... signer automatically freed on scope exit +} +``` + +### Option D: Reference Counting / Ownership Transfer + +**Pros:** +- Clear ownership semantics +- No leaks possible + +**Cons:** +- Requires changing `nostr_signer_t` API +- Complex to implement correctly +- Overkill for this use case + +--- + +## Recommended Approach: Option A (`goto cleanup`) + +### Rationale +1. **Minimal API changes** — No changes to public headers +2. **Portable** — Works on all C compilers +3. **Auditable** — Easy to verify correctness +4. **Consistent** — Matches existing error handling patterns in the codebase +5. **Incremental** — Can be applied file-by-file + +### Implementation Steps + +#### Phase 1: High-Risk Functions (2 functions) +1. [`nostr_nip46_client_session_init`](nostr_core/nip046.c:792) — Complex ownership transfer +2. [`nostr_relay_pool_set_auth`](nostr_core/core_relay_pool.c:707) — Conditional free with error path + +#### Phase 2: Medium-Risk Functions (4 functions) +3. [`nostr_nip17_send_dm`](nostr_core/nip017.c:270) — Complex inner function +4. [`nostr_nip46_decrypt_event`](nostr_core/nip046.c:486) — Error path after free +5. [`blossom_upload`](nostr_core/blossom_client.c:295) — Conditional signer creation +6. [`blossom_upload_file`](nostr_core/blossom_client.c:366) — Conditional signer creation + +#### Phase 3: Low-Risk Functions (21 functions) +7-27. All remaining simple wrappers in [`nip001.c`](nostr_core/nip001.c), [`nip017.c`](nostr_core/nip017.c), [`nip042.c`](nostr_core/nip042.c), [`nip046.c`](nostr_core/nip046.c), [`nip059.c`](nostr_core/nip059.c), [`nip060.c`](nostr_core/nip060.c), [`nip061.c`](nostr_core/nip061.c), [`blossom_client.c`](nostr_core/blossom_client.c) + +### Testing Strategy + +1. **Unit tests** — Verify each refactored function still works correctly +2. **Memory leak detection** — Run tests under Valgrind or AddressSanitizer +3. **Error injection** — Test error paths to ensure cleanup happens +4. **Regression tests** — Ensure no functional changes + +### Example Refactoring + +**Before:** +```c +cJSON* nostr_nip46_create_request_event(const nostr_nip46_request_t* request, + const unsigned char* sender_private_key, + const unsigned char* recipient_public_key, + time_t timestamp) { + nostr_signer_t* signer; + cJSON* out; + + if (!sender_private_key) return NULL; + signer = nostr_signer_local(sender_private_key); + if (!signer) return NULL; + + out = nostr_nip46_create_request_event_with_signer(request, signer, recipient_public_key, timestamp); + nostr_signer_free(signer); + return out; +} +``` + +**After:** +```c +cJSON* nostr_nip46_create_request_event(const nostr_nip46_request_t* request, + const unsigned char* sender_private_key, + const unsigned char* recipient_public_key, + time_t timestamp) { + nostr_signer_t* signer = NULL; + cJSON* out = NULL; + + if (!sender_private_key) return NULL; + + signer = nostr_signer_local(sender_private_key); + if (!signer) return NULL; + + out = nostr_nip46_create_request_event_with_signer(request, signer, recipient_public_key, timestamp); + +cleanup: + nostr_signer_free(signer); + return out; +} +``` + +### Special Cases + +#### `nostr_nip46_client_session_init` (Complex Ownership) + +This function has a unique pattern where the signer ownership is transferred to the session on success: + +```c +int nostr_nip46_client_session_init(nostr_nip46_client_session_t* session, + const unsigned char* client_private_key, + const char* bunker_url) { + nostr_signer_t* signer = NULL; + int rc = NOSTR_ERROR_CRYPTO_FAILED; + + if (!client_private_key) return NOSTR_ERROR_INVALID_INPUT; + + signer = nostr_signer_local(client_private_key); + if (!signer) return NOSTR_ERROR_CRYPTO_FAILED; + + rc = nostr_nip46_client_session_init_with_signer(session, signer, bunker_url); + if (rc != NOSTR_SUCCESS) { + goto cleanup; // Free signer on error + } + + session->owns_client_signer = 1; // Transfer ownership + signer = NULL; // Prevent double-free + +cleanup: + nostr_signer_free(signer); + return rc; +} +``` + +#### `blossom_upload` (Conditional Signer) + +This function creates a signer only if a private key is provided: + +```c +int blossom_upload(const char* server_url, ..., const unsigned char* private_key, ...) { + nostr_signer_t* signer = NULL; + int rc; + + if (private_key) { + signer = nostr_signer_local(private_key); + if (!signer) return NOSTR_ERROR_CRYPTO_FAILED; + } + + rc = blossom_upload_with_signer(server_url, ..., signer, ...); + +cleanup: + if (signer) nostr_signer_free(signer); + return rc; +} +``` + +--- + +## Success Criteria + +- [ ] All 27 wrapper functions refactored to use `goto cleanup` pattern +- [ ] No memory leaks detected under Valgrind/ASan +- [ ] All existing tests pass +- [ ] No functional changes to public API +- [ ] Code review completed + +--- + +## Timeline Estimate + +| Phase | Functions | Complexity | Notes | +|-------|-----------|------------|-------| +| Phase 1 | 2 | High | Complex ownership patterns | +| Phase 2 | 4 | Medium | Conditional creation, error paths | +| Phase 3 | 21 | Low | Simple wrappers | +| Testing | — | — | Valgrind, regression tests | + +**Total:** 27 functions across 8 files + +--- + +## Alternative: Do Nothing + +**Risk:** Low — no known active leaks in production paths + +**Rationale for deferring:** +- The current pattern works correctly for the success path +- Error paths are rare in practice +- The leak is bounded (one signer per failed call) +- The signer is small (~100 bytes) + +**Recommendation:** Address in a dedicated refactoring cycle when touching these files for other reasons. diff --git a/tests/async_publish_test b/tests/async_publish_test index 19a86610..a4da0bdd 100755 Binary files a/tests/async_publish_test and b/tests/async_publish_test differ diff --git a/tests/backward_compat_test b/tests/backward_compat_test index 14c8aaff..814e5a07 100755 Binary files a/tests/backward_compat_test and b/tests/backward_compat_test differ diff --git a/tests/bip32_test b/tests/bip32_test index 4c50abf5..50b0930c 100755 Binary files a/tests/bip32_test and b/tests/bip32_test differ diff --git a/tests/blossom_client_live_test b/tests/blossom_client_live_test index 365786da..1358a3f9 100755 Binary files a/tests/blossom_client_live_test and b/tests/blossom_client_live_test differ diff --git a/tests/blossom_client_test b/tests/blossom_client_test index b6c769e5..ae46f1c2 100755 Binary files a/tests/blossom_client_test and b/tests/blossom_client_test differ diff --git a/tests/blossom_mock_error_test b/tests/blossom_mock_error_test index 7f8525a8..666e22c6 100755 Binary files a/tests/blossom_mock_error_test and b/tests/blossom_mock_error_test differ diff --git a/tests/cashu_mint_test b/tests/cashu_mint_test index ddd7a6ab..2860c605 100755 Binary files a/tests/cashu_mint_test and b/tests/cashu_mint_test differ diff --git a/tests/enhanced_header_test b/tests/enhanced_header_test index 59f60905..d6ffbba7 100755 Binary files a/tests/enhanced_header_test and b/tests/enhanced_header_test differ diff --git a/tests/nip01_test b/tests/nip01_test index d49927ed..359cd106 100755 Binary files a/tests/nip01_test and b/tests/nip01_test differ diff --git a/tests/nip03_test b/tests/nip03_test index f9e226e9..84f0d6a7 100755 Binary files a/tests/nip03_test and b/tests/nip03_test differ diff --git a/tests/nip13_test b/tests/nip13_test index 0d0cd80c..29056a41 100755 Binary files a/tests/nip13_test and b/tests/nip13_test differ diff --git a/tests/nip17_test b/tests/nip17_test index a8714873..5e405cb6 100755 Binary files a/tests/nip17_test and b/tests/nip17_test differ diff --git a/tests/nip42_pool_test b/tests/nip42_pool_test index 40316653..349336f6 100755 Binary files a/tests/nip42_pool_test and b/tests/nip42_pool_test differ diff --git a/tests/nip42_test b/tests/nip42_test index 1a991cc9..e6244284 100755 Binary files a/tests/nip42_test and b/tests/nip42_test differ diff --git a/tests/nip46_test b/tests/nip46_test index 1b992a2f..ae74c195 100755 Binary files a/tests/nip46_test and b/tests/nip46_test differ diff --git a/tests/nip60_live_receive_test b/tests/nip60_live_receive_test index a91a20e7..a23680de 100755 Binary files a/tests/nip60_live_receive_test and b/tests/nip60_live_receive_test differ diff --git a/tests/nip60_test b/tests/nip60_test index c0dd3fda..25706a3a 100755 Binary files a/tests/nip60_test and b/tests/nip60_test differ diff --git a/tests/nip61_test b/tests/nip61_test index 341b73f5..015aaa38 100755 Binary files a/tests/nip61_test and b/tests/nip61_test differ diff --git a/tests/relay_synchronous_test b/tests/relay_synchronous_test index 6a433633..26e1f6d6 100755 Binary files a/tests/relay_synchronous_test and b/tests/relay_synchronous_test differ diff --git a/tests/repeated_sync_query_test b/tests/repeated_sync_query_test index 138f9e86..ac8ffd40 100755 Binary files a/tests/repeated_sync_query_test and b/tests/repeated_sync_query_test differ diff --git a/tests/simple_async_test b/tests/simple_async_test index fb3b327d..62ab93a4 100755 Binary files a/tests/simple_async_test and b/tests/simple_async_test differ diff --git a/tests/sync_relay_test b/tests/sync_relay_test index 98d9fb5f..c25853c4 100755 Binary files a/tests/sync_relay_test and b/tests/sync_relay_test differ