From 07900e96174873489cd6feaca8931e83fa568d55 Mon Sep 17 00:00:00 2001 From: Michael Vetter Date: Thu, 20 Jun 2024 12:40:45 +0200 Subject: [PATCH] Revert "Fix potential double free of ratchet identity key" This reverts commit 3b099e9403c61d184d325c7b5c7fde30a572cf5c. This resulted in: ``` ==5285== Invalid read of size 16 ==5285== at 0x4FA80FC: ec_public_key_serialize (in /usr/lib64/libsignal-protocol-c.so.2.3.3) ==5285== by 0x4E5E76: omemo_identity_key (omemo.c:419) ==5285== by 0x4EBB7E: omemo_bundle_publish (omemo.c:129) ==5285== by 0x4E5BD9: omemo_publish_crypto_materials (omemo.c:335) ==5285== by 0x460407: sv_ev_connection_features_received (server_events.c:202) ==5285== by 0x43AA87: connection_features_received (connection.c:779) ==5285== by 0x4418C9: _disco_info_response_id_handler_onconnect (iq.c:2423) ==5285== by 0x43B9F1: _iq_handler (iq.c:241) ==5285== by 0x5163848: ??? (in /usr/lib64/libstrophe.so.0.13.1) ==5285== by 0x516A224: ??? (in /usr/lib64/libstrophe.so.0.13.1) ==5285== by 0x5E4FE43: ??? (in /usr/lib64/libxml2.so.2.12.8) ==5285== by 0x5E54927: xmlParseChunk (in /usr/lib64/libxml2.so.2.12.8) ==5285== by 0x5163450: xmpp_run_once (in /usr/lib64/libstrophe.so.0.13.1) ==5285== by 0x439797: connection_check_events (connection.c:162) ==5285== by 0x43894E: session_process_events (session.c:256) ==5285== by 0x4319FF: prof_run (profanity.c:128) ==5285== by 0x4EDAE6: main (main.c:174) ==5285== Address 0xa1cb1e0 is 16 bytes inside a block of size 72 free'd ==5285== at 0x484875B: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==5285== by 0x4395D0: _xfree (connection.c:110) ==5285== by 0x516A1A7: xmpp_stanza_release (in /usr/lib64/libstrophe.so.0.13.1) ==5285== by 0x516A16C: xmpp_stanza_release (in /usr/lib64/libstrophe.so.0.13.1) ==5285== by 0x516A16C: xmpp_stanza_release (in /usr/lib64/libstrophe.so.0.13.1) ==5285== by 0x516A230: ??? (in /usr/lib64/libstrophe.so.0.13.1) ==5285== by 0x5E4FE43: ??? (in /usr/lib64/libxml2.so.2.12.8) ==5285== by 0x5E54927: xmlParseChunk (in /usr/lib64/libxml2.so.2.12.8) ==5285== by 0x5163450: xmpp_run_once (in /usr/lib64/libstrophe.so.0.13.1) ==5285== by 0x439797: connection_check_events (connection.c:162) ==5285== by 0x43894E: session_process_events (session.c:256) ==5285== by 0x4319FF: prof_run (profanity.c:128) ==5285== by 0x4EDAE6: main (main.c:174) ==5285== Block was alloc'd at ==5285== at 0x4845794: malloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so) ==5285== by 0x43958A: _xmalloc (connection.c:102) ==5285== by 0x516A0D1: xmpp_stanza_new (in /usr/lib64/libstrophe.so.0.13.1) ==5285== by 0x516BF34: ??? (in /usr/lib64/libstrophe.so.0.13.1) ==5285== by 0x5F10A17: ??? (in /usr/lib64/libxml2.so.2.12.8) ==5285== by 0x5E5481F: xmlParseChunk (in /usr/lib64/libxml2.so.2.12.8) ==5285== by 0x5163450: xmpp_run_once (in /usr/lib64/libstrophe.so.0.13.1) ==5285== by 0x439797: connection_check_events (connection.c:162) ==5285== by 0x43894E: session_process_events (session.c:256) ==5285== by 0x4319FF: prof_run (profanity.c:128) ==5285== by 0x4EDAE6: main (main.c:174) ``` Tested via sending OMEMO messages via 1:1 and in MUC. --- src/omemo/omemo.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/omemo/omemo.c b/src/omemo/omemo.c index 5155adace..da0bfc531 100644 --- a/src/omemo/omemo.c +++ b/src/omemo/omemo.c @@ -271,7 +271,11 @@ omemo_on_disconnect(void) free_keyfile(&omemo_ctx.identity); signal_protocol_store_context_destroy(omemo_ctx.store); + ec_public_key* pub = ratchet_identity_key_pair_get_public(omemo_ctx.identity_key_pair); + ec_private_key* priv = ratchet_identity_key_pair_get_private(omemo_ctx.identity_key_pair); ratchet_identity_key_pair_destroy((signal_type_base*)omemo_ctx.identity_key_pair); + ec_private_key_destroy((signal_type_base*)priv); + ec_public_key_destroy((signal_type_base*)pub); signal_context_destroy(omemo_ctx.signal); memset(&omemo_ctx, 0, sizeof(omemo_ctx)); @@ -1466,8 +1470,6 @@ _load_identity(void) ec_private_key* private_key; curve_decode_private_point(&private_key, identity_key_private, identity_key_private_len, omemo_ctx.signal); ratchet_identity_key_pair_create(&omemo_ctx.identity_key_pair, public_key, private_key); - ec_private_key_destroy((signal_type_base*)private_key); - ec_public_key_destroy((signal_type_base*)public_key); char** keys = NULL; int i;