Skip to content

Commit

Permalink
Fix potential double free of ratchet identity key
Browse files Browse the repository at this point in the history
libsignal does this properly, so there wouldn't be a real double free, but
it will `abort()`.

Instead of destroying the identity key on disconnect, already destroy it
after it has been put into the libsignal 'ratchet identity key pair'.

In the case where the key pair is initially generated, the public
and private parts are only `ref()`'ed once in [0].
In the case where the key pair is read from the disk, the public
and private parts are `ref()`'ed twice, first when decoded in [1] resp.
[2] and a second time in [3].

When `omemo_on_disconnect()` is called we were `unref()`'ing the parts
twice, before this patch. First in [4], a second time in [5] resp. [6].

Now we do the second `unref()` already when loading.

[0] `signal_protocol_key_helper_generate_identity_key_pair()`
[1] `curve_decode_point()`
[2] `curve_decode_private_point()`
[3] `ratchet_identity_key_pair_create()`
[4] `ratchet_identity_key_pair_destroy()`
[5] `ec_private_key_destroy()`
[6] `ec_public_key_destroy()`

Signed-off-by: Steffen Jaeckel <[email protected]>
  • Loading branch information
sjaeckel committed Jun 19, 2024
1 parent 76a1846 commit 1f021e8
Showing 1 changed file with 2 additions and 4 deletions.
6 changes: 2 additions & 4 deletions src/omemo/omemo.c
Original file line number Diff line number Diff line change
Expand Up @@ -271,11 +271,7 @@ 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));
Expand Down Expand Up @@ -1470,6 +1466,8 @@ _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;
Expand Down

0 comments on commit 1f021e8

Please sign in to comment.