M-11 wrapper function memory leak fix: refactored 27 wrapper functions across 8 files to use goto cleanup pattern ensuring temporary nostr_signer_t is always freed on error paths. Includes special handling for ownership transfer (nip046, core_relay_pool) and conditional signer creation (blossom_client). All 39/39 tests pass.
This commit is contained in:
@@ -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;
|
||||
}
|
||||
|
||||
|
||||
@@ -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) {
|
||||
|
||||
+3
-1
@@ -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;
|
||||
}
|
||||
|
||||
+15
-9
@@ -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;
|
||||
}
|
||||
|
||||
+5
-3
@@ -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;
|
||||
}
|
||||
|
||||
+30
-19
@@ -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,
|
||||
|
||||
+15
-9
@@ -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;
|
||||
}
|
||||
|
||||
+35
-21
@@ -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;
|
||||
}
|
||||
|
||||
+17
-11
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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.
|
||||
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Reference in New Issue
Block a user