From e39ffd981f84dfa103f24838ffa91c7878259e49 Mon Sep 17 00:00:00 2001 From: Michael Vetter Date: Mon, 9 Mar 2026 08:13:51 +0100 Subject: [PATCH] refactor: Refactor OMEMO error handling and add non-null attributes Add __attribute__((nonnull) hints to omemo_receive_message and omemo_on_message_recv declarations to catch NULL arguments at compiletime. So we can set the error state unconditionally in the functions. --- src/omemo/omemo.c | 38 +++++++++--------------------- src/omemo/omemo.h | 2 +- src/xmpp/omemo.c | 36 +++++++--------------------- src/xmpp/omemo.h | 2 +- tests/unittests/omemo/stub_omemo.c | 8 ++----- 5 files changed, 24 insertions(+), 62 deletions(-) diff --git a/src/omemo/omemo.c b/src/omemo/omemo.c index bf2c842a..17f6c836 100644 --- a/src/omemo/omemo.c +++ b/src/omemo/omemo.c @@ -1020,15 +1020,11 @@ omemo_on_message_recv(const char* const from_jid, uint32_t sid, auto_jid Jid* sender = NULL; auto_jid Jid* from = jid_create(from_jid); - if (error) { - *error = OMEMO_ERR_NONE; - } + *error = OMEMO_ERR_NONE; if (!from) { log_error("[OMEMO][RECV] Invalid jid %s", from_jid); - if (error) { - *error = OMEMO_ERR_INVALID_JID; - } + *error = OMEMO_ERR_INVALID_JID; return NULL; } @@ -1044,9 +1040,7 @@ omemo_on_message_recv(const char* const from_jid, uint32_t sid, if (!key) { log_warning("[OMEMO][RECV] received a message with no corresponding key"); - if (error) { - *error = OMEMO_ERR_NO_KEY; - } + *error = OMEMO_ERR_NO_KEY; return NULL; } @@ -1063,9 +1057,7 @@ omemo_on_message_recv(const char* const from_jid, uint32_t sid, g_list_free(roster); if (!sender) { log_warning("[OMEMO][RECV] cannot find MUC message sender fulljid"); - if (error) { - *error = OMEMO_ERR_MUC_SENDER_NOT_FOUND; - } + *error = OMEMO_ERR_MUC_SENDER_NOT_FOUND; return NULL; } } else { @@ -1083,9 +1075,7 @@ omemo_on_message_recv(const char* const from_jid, uint32_t sid, res = session_cipher_create(&cipher, omemo_ctx.store, &address, omemo_ctx.signal); if (res != 0) { log_error("[OMEMO][RECV] cannot create session cipher"); - if (error) { - *error = OMEMO_ERR_NO_SESSION; - } + *error = OMEMO_ERR_NO_SESSION; return NULL; } @@ -1145,12 +1135,10 @@ omemo_on_message_recv(const char* const from_jid, uint32_t sid, session_cipher_free(cipher); if (res != 0) { log_error("[OMEMO][RECV] cannot decrypt message key: %d", res); - if (error) { - if (!*trusted) { - *error = OMEMO_ERR_NOT_TRUSTED; - } else { - *error = OMEMO_ERR_DECRYPT_FAILED; - } + if (!*trusted) { + *error = OMEMO_ERR_NOT_TRUSTED; + } else { + *error = OMEMO_ERR_DECRYPT_FAILED; } return NULL; } @@ -1158,9 +1146,7 @@ omemo_on_message_recv(const char* const from_jid, uint32_t sid, if (signal_buffer_len(plaintext_key) != AES128_GCM_KEY_LENGTH + AES128_GCM_TAG_LENGTH) { log_error("[OMEMO][RECV] invalid key length"); signal_buffer_free(plaintext_key); - if (error) { - *error = OMEMO_ERR_DECRYPT_FAILED; - } + *error = OMEMO_ERR_DECRYPT_FAILED; return NULL; } @@ -1173,9 +1159,7 @@ omemo_on_message_recv(const char* const from_jid, uint32_t sid, if (res != 0) { log_error("[OMEMO][RECV] cannot decrypt message: %s", gcry_strerror(res)); free(plaintext); - if (error) { - *error = OMEMO_ERR_DECRYPT_FAILED; - } + *error = OMEMO_ERR_DECRYPT_FAILED; return NULL; } diff --git a/src/omemo/omemo.h b/src/omemo/omemo.h index 62d28197..bbae2856 100644 --- a/src/omemo/omemo.h +++ b/src/omemo/omemo.h @@ -100,7 +100,7 @@ void omemo_start_device_session(const char* const jid, uint32_t device_id, GList gboolean omemo_loaded(void); char* omemo_on_message_send(ProfWin* win, const char* const message, gboolean request_receipt, gboolean muc, const char* const replace_id); -char* omemo_on_message_recv(const char* const from, uint32_t sid, const unsigned char* const iv, size_t iv_len, GList* keys, const unsigned char* const payload, size_t payload_len, gboolean muc, gboolean* trusted, omemo_error_t* error); +char* omemo_on_message_recv(const char* const from, uint32_t sid, const unsigned char* const iv, size_t iv_len, GList* keys, const unsigned char* const payload, size_t payload_len, gboolean muc, gboolean* trusted, omemo_error_t* error) __attribute__((nonnull(9, 10))); char* omemo_encrypt_file(FILE* in, FILE* out, off_t file_size, int* gcry_res); gcry_error_t omemo_decrypt_file(FILE* in, FILE* out, off_t file_size, const char* fragment); diff --git a/src/xmpp/omemo.c b/src/xmpp/omemo.c index d53b83d7..4e3b52b5 100644 --- a/src/xmpp/omemo.c +++ b/src/xmpp/omemo.c @@ -360,9 +360,7 @@ omemo_receive_message(xmpp_stanza_t* const stanza, gboolean* trusted, omemo_erro char* iv_text = NULL; char* payload_text = NULL; - if (error) { - *error = OMEMO_ERR_NONE; - } + *error = OMEMO_ERR_NONE; xmpp_stanza_t* encrypted = xmpp_stanza_get_child_by_ns(stanza, STANZA_NS_OMEMO); if (!encrypted) { @@ -371,64 +369,48 @@ omemo_receive_message(xmpp_stanza_t* const stanza, gboolean* trusted, omemo_erro xmpp_stanza_t* header = xmpp_stanza_get_child_by_name(encrypted, "header"); if (!header) { - if (error) { - *error = OMEMO_ERR_OTHER; - } + *error = OMEMO_ERR_OTHER; return NULL; } const char* sid_text = xmpp_stanza_get_attribute(header, "sid"); if (!sid_text) { - if (error) { - *error = OMEMO_ERR_OTHER; - } + *error = OMEMO_ERR_OTHER; return NULL; } uint32_t sid = strtoul(sid_text, NULL, 10); xmpp_stanza_t* iv = xmpp_stanza_get_child_by_name(header, "iv"); if (!iv) { - if (error) { - *error = OMEMO_ERR_OTHER; - } + *error = OMEMO_ERR_OTHER; return NULL; } iv_text = xmpp_stanza_get_text(iv); if (!iv_text) { - if (error) { - *error = OMEMO_ERR_OTHER; - } + *error = OMEMO_ERR_OTHER; return NULL; } size_t iv_len; iv_raw = g_base64_decode(iv_text, &iv_len); if (!iv_raw) { - if (error) { - *error = OMEMO_ERR_OTHER; - } + *error = OMEMO_ERR_OTHER; return NULL; } xmpp_stanza_t* payload = xmpp_stanza_get_child_by_name(encrypted, "payload"); if (!payload) { - if (error) { - *error = OMEMO_ERR_OTHER; - } + *error = OMEMO_ERR_OTHER; goto quit; } payload_text = xmpp_stanza_get_text(payload); if (!payload_text) { - if (error) { - *error = OMEMO_ERR_OTHER; - } + *error = OMEMO_ERR_OTHER; goto quit; } size_t payload_len; payload_raw = g_base64_decode(payload_text, &payload_len); if (!payload_raw) { - if (error) { - *error = OMEMO_ERR_OTHER; - } + *error = OMEMO_ERR_OTHER; goto quit; } diff --git a/src/xmpp/omemo.h b/src/xmpp/omemo.h index e26029e1..622bae49 100644 --- a/src/xmpp/omemo.h +++ b/src/xmpp/omemo.h @@ -45,4 +45,4 @@ void omemo_bundle_publish(gboolean first); void omemo_bundle_request(const char* const jid, uint32_t device_id, ProfIqCallback func, ProfIqFreeCallback free_func, void* userdata); int omemo_start_device_session_handle_bundle(xmpp_stanza_t* const stanza, void* const userdata); -char* omemo_receive_message(xmpp_stanza_t* const stanza, gboolean* trusted, omemo_error_t* error); +char* omemo_receive_message(xmpp_stanza_t* const stanza, gboolean* trusted, omemo_error_t* error) __attribute__((nonnull(2, 3))); diff --git a/tests/unittests/omemo/stub_omemo.c b/tests/unittests/omemo/stub_omemo.c index 7d1afd5f..375257bc 100644 --- a/tests/unittests/omemo/stub_omemo.c +++ b/tests/unittests/omemo/stub_omemo.c @@ -135,18 +135,14 @@ omemo_on_message_recv(const char* const from, uint32_t sid, GList* keys, const unsigned char* const payload, size_t payload_len, gboolean muc, gboolean* trusted, omemo_error_t* error) { - if (error) { - *error = OMEMO_ERR_NONE; - } + *error = OMEMO_ERR_NONE; return NULL; } char* omemo_receive_message(xmpp_stanza_t* const stanza, gboolean* trusted, omemo_error_t* error) { - if (error) { - *error = OMEMO_ERR_NONE; - } + *error = OMEMO_ERR_NONE; return NULL; }