From e05e0d58b3c3a2ab4a7fa526f66e36b472a45ef5 Mon Sep 17 00:00:00 2001 From: "jabber.developer2" Date: Sat, 25 Jul 2026 15:43:59 +0300 Subject: [PATCH] security: E2EE and transport correctness (issue #147) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit T04: promote security events to warnings — SASL auth failure, TLS handshake failure, cert-failure details, see-other-host redirect; DISABLE_TLS, TRUST_TLS and LEGACY_AUTH get a log warning plus a console notice (REQ-LOG-01, REQ-LOG-02, REQ-AUTH-03) T05: warn when a session ends up unencrypted without the user having asked for it; refuse in-band registration on an unencrypted stream; warn on each HTTP transfer with certificate verification disabled (REQ-CRY-03, REQ-CFG-01) T06: pin the update check to https with peer/host verification and no redirects; strict N.N.N parser for the fetched version, which is untrusted network input (REQ-VUL-02) T09: no plaintext logging on failed MUC OMEMO sends; OTR opportunistic first message passes allow_unencrypted_message(); get_random_string() draws from a CSPRNG without modulo bias; guard the identity-key length decrement against unsigned underflow (REQ-CRY-01, REQ-CRY-02, REQ-CRY-07, REQ-MEM-05) REQ-VUL-03 and REQ-CRY-09 are already satisfied on master and are left unchanged. The console warnings use cons_show_warning() from #87, so this needs that change in master first. --- configure.ac | 6 ++ src/command/cmd_funcs.c | 15 +++ src/common.c | 120 ++++++++++++++++++--- src/event/client_events.c | 8 +- src/event/server_events.c | 7 +- src/omemo/omemo.c | 6 ++ src/otr/otr.c | 5 + src/tools/http_common.c | 11 ++ src/tools/http_common.h | 1 + src/tools/http_download.c | 3 + src/tools/http_upload.c | 2 + src/xmpp/connection.c | 44 +++++++- tests/functionaltests/functionaltests.c | 1 + tests/functionaltests/test_connect.c | 12 +++ tests/functionaltests/test_connect.h | 1 + tests/functionaltests/test_export_import.c | 2 + tests/unittests/test_common.c | 39 +++++++ tests/unittests/test_common.h | 1 + tests/unittests/unittests.c | 1 + 19 files changed, 262 insertions(+), 23 deletions(-) diff --git a/configure.ac b/configure.ac index dece25a2..e518043c 100644 --- a/configure.ac +++ b/configure.ac @@ -342,6 +342,12 @@ AS_IF([test "x$enable_gdk_pixbuf" != xno], [AC_MSG_ERROR([gdk-pixbuf-2.0 >= 2.4 is required to scale avatars before uploading])], [AC_MSG_NOTICE([gdk-pixbuf-2.0 >= 2.4 not found, GDK Pixbuf support not enabled])])])]) +dnl libgcrypt: CSPRNG for stanza ids and instance identifiers (also pulled in by OMEMO/OTR) +AC_CHECK_LIB([gcrypt], [gcry_create_nonce], + [AC_DEFINE([HAVE_LIBGCRYPT], [1], [Have libgcrypt]) + LIBS="-lgcrypt $LIBS"], + [AC_MSG_NOTICE([libgcrypt not found, identifiers will use the OS random device])]) + dnl feature: omemo AM_CONDITIONAL([BUILD_OMEMO], [false]) if test "x$enable_omemo" != xno; then diff --git a/src/command/cmd_funcs.c b/src/command/cmd_funcs.c index 77b6dd96..1180c94d 100644 --- a/src/command/cmd_funcs.c +++ b/src/command/cmd_funcs.c @@ -9558,6 +9558,11 @@ _url_aesgcm_method(ProfWin* window, const char* cmd_template, gchar* url, gchar* return; AESGCMDownload* download = g_new0(AESGCMDownload, 1); download->id = get_random_string(4); + if (!download->id) { + cons_show_error("Could not start download: no random source available."); + g_free(download); + return; + } download->url = strdup(url); download->filename = strdup(filename); if (cmd_template != NULL) { @@ -9580,6 +9585,11 @@ _download_install_plugin(ProfWin* window, gchar* url, gchar* path) return FALSE; HTTPDownload* download = g_new0(HTTPDownload, 1); download->id = get_random_string(4); + if (!download->id) { + cons_show_error("Could not start download: no random source available."); + g_free(download); + return FALSE; + } download->url = strdup(url); download->filename = strdup(filename); download->cmd_template = NULL; @@ -9599,6 +9609,11 @@ _url_http_method(ProfWin* window, const char* cmd_template, gchar* url, gchar* p return; HTTPDownload* download = g_new0(HTTPDownload, 1); download->id = get_random_string(4); + if (!download->id) { + cons_show_error("Could not start download: no random source available."); + g_free(download); + return; + } download->url = strdup(url); download->filename = strdup(filename); download->cmd_template = cmd_template ? strdup(cmd_template) : NULL; diff --git a/src/common.c b/src/common.c index 009c6f41..6d4dafe7 100644 --- a/src/common.c +++ b/src/common.c @@ -23,6 +23,9 @@ #include #include +#ifdef HAVE_LIBGCRYPT +#include +#endif #include #include #include @@ -470,9 +473,26 @@ release_get_latest(void) curl_easy_setopt(handle, CURLOPT_TIMEOUT, 2L); curl_easy_setopt(handle, CURLOPT_WRITEDATA, (void*)&output); - curl_easy_perform(handle); + curl_easy_setopt(handle, CURLOPT_SSL_VERIFYPEER, 1L); + curl_easy_setopt(handle, CURLOPT_SSL_VERIFYHOST, 2L); + curl_easy_setopt(handle, CURLOPT_FOLLOWLOCATION, 0L); +#if LIBCURL_VERSION_NUM >= 0x075500 // 7.85.0 introduced the string form + curl_easy_setopt(handle, CURLOPT_PROTOCOLS_STR, "https"); + curl_easy_setopt(handle, CURLOPT_REDIR_PROTOCOLS_STR, "https"); +#else + curl_easy_setopt(handle, CURLOPT_PROTOCOLS, CURLPROTO_HTTPS); + curl_easy_setopt(handle, CURLOPT_REDIR_PROTOCOLS, CURLPROTO_HTTPS); +#endif + + CURLcode res = curl_easy_perform(handle); curl_easy_cleanup(handle); + if (res != CURLE_OK) { + log_warning("Update check failed: %s", curl_easy_strerror(res)); + free(output.buffer); + return NULL; + } + if (output.buffer) { output.buffer[output.size++] = '\0'; return output.buffer; @@ -481,6 +501,43 @@ release_get_latest(void) } } +// strict "major.minor.patch": the found version is untrusted network input +static gboolean +_parse_version(const char* const version, int* major, int* minor, int* patch) +{ + if (!version) { + return FALSE; + } + + int* out[3] = { major, minor, patch }; + const char* curr = version; + + for (int part = 0; part < 3; part++) { + if (part > 0) { + if (*curr != '.') { + return FALSE; + } + curr++; + } + + if (!g_ascii_isdigit(*curr)) { + return FALSE; + } + + gint64 value = 0; + while (g_ascii_isdigit(*curr)) { + value = value * 10 + (*curr - '0'); + if (value > G_MAXINT) { + return FALSE; + } + curr++; + } + *out[part] = (int)value; + } + + return *curr == '\0'; +} + gboolean release_is_new(const char* const curr_version, const char* const found_version) { @@ -490,12 +547,10 @@ release_is_new(const char* const curr_version, const char* const found_version) int curr_maj, curr_min, curr_patch, found_maj, found_min, found_patch; - int parse_curr = sscanf(curr_version, "%d.%d.%d", &curr_maj, &curr_min, - &curr_patch); - int parse_found = sscanf(found_version, "%d.%d.%d", &found_maj, &found_min, - &found_patch); + gboolean parse_curr = _parse_version(curr_version, &curr_maj, &curr_min, &curr_patch); + gboolean parse_found = _parse_version(found_version, &found_maj, &found_min, &found_patch); - if (parse_found == 3 && parse_curr == 3) { + if (parse_found && parse_curr) { if (found_maj > curr_maj) { return TRUE; } else if (found_maj == curr_maj && found_min > curr_min) { @@ -699,22 +754,57 @@ get_file_paths_recursive(const char* path, GSList** contents) } } +static gboolean +_random_bytes(unsigned char* buf, size_t len) +{ +#ifdef HAVE_LIBGCRYPT + // never initialise gcrypt here: omemo_crypto_init() must set up secure memory first + if (gcry_control(GCRYCTL_INITIALIZATION_FINISHED_P)) { + gcry_create_nonce(buf, len); + return TRUE; + } +#endif + + FILE* urandom = fopen("/dev/urandom", "rb"); + if (urandom) { + size_t got = fread(buf, 1, len, urandom); + fclose(urandom); + if (got == len) { + return TRUE; + } + } + + return FALSE; +} + gchar* get_random_string(size_t length) { - GRand* prng; - gchar* rand; - gchar alphabet[] = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"; - int endrange = sizeof(alphabet) - 1; + static const gchar alphabet[] = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"; + const unsigned alphabet_len = sizeof(alphabet) - 1; + const unsigned limit = 256 - (256 % alphabet_len); // reject the uneven tail, it would bias early letters - rand = g_malloc0(length + 1); + gchar* rand = g_malloc0(length + 1); - prng = g_rand_new(); + for (size_t i = 0; i < length;) { + unsigned char block[64]; + size_t want = length - i; + if (want > sizeof(block)) { + want = sizeof(block); + } - for (size_t i = 0; i < length; i++) { - rand[i] = alphabet[g_rand_int_range(prng, 0, endrange)]; + if (!_random_bytes(block, want)) { + log_error("No cryptographic random source available for identifier generation"); + g_free(rand); + return NULL; + } + + for (size_t j = 0; j < want && i < length; j++) { + if (block[j] < limit) { + rand[i++] = alphabet[block[j] % alphabet_len]; + } + } } - g_rand_free(prng); return rand; } diff --git a/src/event/client_events.c b/src/event/client_events.c index 8ebefa6e..106697fd 100644 --- a/src/event/client_events.c +++ b/src/event/client_events.c @@ -198,9 +198,11 @@ cl_ev_send_muc_msg_corrected(ProfMucWin* mucwin, const char* const msg, const ch #ifdef HAVE_OMEMO if (mucwin->is_omemo) { auto_char char* id = omemo_on_message_send((ProfWin*)mucwin, message, FALSE, TRUE, replace_id); - groupchat_log_omemo_msg_out(mucwin->roomjid, message); - log_database_add_outgoing_muc(id, mucwin->roomjid, message, replace_id, PROF_MSG_ENC_OMEMO); - mucwin_outgoing_msg(mucwin, message, id, PROF_MSG_ENC_OMEMO, replace_id); + if (id != NULL) { // nothing was sent, so don't persist or echo the plaintext + groupchat_log_omemo_msg_out(mucwin->roomjid, message); + log_database_add_outgoing_muc(id, mucwin->roomjid, message, replace_id, PROF_MSG_ENC_OMEMO); + mucwin_outgoing_msg(mucwin, message, id, PROF_MSG_ENC_OMEMO, replace_id); + } } else #endif { diff --git a/src/event/server_events.c b/src/event/server_events.c index 71d7af35..81849082 100644 --- a/src/event/server_events.c +++ b/src/event/server_events.c @@ -205,7 +205,7 @@ void sv_ev_failed_login(void) { cons_show_error("Login failed."); - log_info("Login failed"); + log_warning("[SECURITY] Authentication failed for %s", STR_MAYBE_NULL(session_get_account_name())); tlscerts_clear_current(); } @@ -1145,6 +1145,11 @@ sv_ev_certfail(const char* const errormsg, const TLSCertificate* cert) return 1; } + log_warning("[SECURITY] TLS certificate verification failed: %s (subject: %s, fingerprint: %s)", + errormsg, + cert && cert->subjectname ? cert->subjectname : "(unknown)", + cert && cert->fingerprint ? cert->fingerprint : "(none)"); + cons_show(""); cons_show_error("TLS certificate verification failed: %s", errormsg); cons_show_tlscert(cert); diff --git a/src/omemo/omemo.c b/src/omemo/omemo.c index 38157336..261c87c0 100644 --- a/src/omemo/omemo.c +++ b/src/omemo/omemo.c @@ -1433,6 +1433,12 @@ _omemo_fingerprint(ec_public_key* identity, gboolean formatted) size_t identity_public_key_len = signal_buffer_len(identity_public_key); unsigned char* identity_public_key_data = signal_buffer_data(identity_public_key); + if (identity_public_key_len == 0) { + log_error("[OMEMO] cannot fingerprint an empty identity key"); // the decrement below would wrap + signal_buffer_free(identity_public_key); + return NULL; + } + /* Skip first byte corresponding to signal DJB_TYPE */ identity_public_key_len--; identity_public_key_data = &identity_public_key_data[1]; diff --git a/src/otr/otr.c b/src/otr/otr.c index b5c336a9..8ba6ed62 100644 --- a/src/otr/otr.c +++ b/src/otr/otr.c @@ -22,6 +22,7 @@ #include "config/files.h" #include "otr/otr.h" #include "otr/otrlib.h" +#include "event/client_events.h" #include "ui/ui.h" #include "ui/window_list.h" #include "xmpp/chat_session.h" @@ -331,6 +332,10 @@ otr_on_message_send(ProfChatWin* chatwin, const char* const message, gboolean re // tag and send for policy opportunistic if (policy == PROF_OTRPOLICY_OPPORTUNISTIC) { + // the tagged first message is still plaintext on the wire + if (!allow_unencrypted_message(chatwin, message)) { + return TRUE; + } auto_char char* otr_tagged_msg = otr_tag_message(message); id = message_send_chat_otr(chatwin->barejid, otr_tagged_msg, request_receipt, replace_id); chatwin_outgoing_msg(chatwin, message, id, PROF_MSG_ENC_NONE, request_receipt, replace_id); diff --git a/src/tools/http_common.c b/src/tools/http_common.c index 0386e08d..170118f9 100644 --- a/src/tools/http_common.c +++ b/src/tools/http_common.c @@ -14,6 +14,7 @@ #include #include +#include "log.h" #include "tools/http_common.h" #define FALLBACK_MSG "" @@ -51,3 +52,13 @@ http_print_transfer(ProfWin* window, char* id, theme_item_t theme_item, const ch g_string_free(msg, TRUE); } + +void +http_warn_insecure_transfer(ProfWin* window, char* id, const char* const url) +{ + log_warning("[SECURITY] TLS certificate verification disabled for transfer: %s", url); + http_print_transfer_update(window, id, THEME_ERROR, 0, + "Security warning: TLS certificate not verified for '%s' " + "(tls.policy=trust).", + url); +} diff --git a/src/tools/http_common.h b/src/tools/http_common.h index 8f858eac..5ee5daba 100644 --- a/src/tools/http_common.h +++ b/src/tools/http_common.h @@ -17,5 +17,6 @@ G_GNUC_PRINTF(4, 5) void http_print_transfer(ProfWin* window, char* id, theme_item_t theme_item, const char* fmt, ...); G_GNUC_PRINTF(5, 6) void http_print_transfer_update(ProfWin* window, char* id, theme_item_t theme_item, int flags, const char* fmt, ...); +void http_warn_insecure_transfer(ProfWin* window, char* id, const char* const url); #endif diff --git a/src/tools/http_download.c b/src/tools/http_download.c index 8c45c8b6..c302efdb 100644 --- a/src/tools/http_download.c +++ b/src/tools/http_download.c @@ -25,6 +25,7 @@ #include "profanity.h" #include "event/client_events.h" #include "tools/http_download.h" +#include "tools/http_common.h" #include "config/cafile.h" #include "config/preferences.h" #include "ui/ui.h" @@ -153,6 +154,8 @@ http_file_get(void* userdata) if (insecure) { curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 0L); curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 0L); + http_warn_insecure_transfer(download->window, download->id, + download->display_url ?: download->url); } if ((res = curl_easy_perform(curl)) != CURLE_OK) { diff --git a/src/tools/http_upload.c b/src/tools/http_upload.c index 9d67699a..3d7476fd 100644 --- a/src/tools/http_upload.c +++ b/src/tools/http_upload.c @@ -22,6 +22,7 @@ #include "profanity.h" #include "event/client_events.h" #include "tools/http_upload.h" +#include "tools/http_common.h" #include "config/cafile.h" #include "config/preferences.h" #include "ui/ui.h" @@ -242,6 +243,7 @@ http_file_put(void* userdata) if (insecure) { curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 0L); curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, 0L); + http_warn_insecure_transfer(upload->window, upload->put_url, upload->put_url); } curl_easy_setopt(curl, CURLOPT_READDATA, fh); diff --git a/src/xmpp/connection.c b/src/xmpp/connection.c index b0a8c7f4..f2109f09 100644 --- a/src/xmpp/connection.c +++ b/src/xmpp/connection.c @@ -46,6 +46,7 @@ typedef struct prof_conn_t GHashTable* available_resources; GHashTable* features_by_jid; GHashTable* requested_features; + gboolean tls_disabled_by_user; } ProfConnection; typedef struct @@ -212,6 +213,21 @@ _conn_apply_settings(const char* const jid, const char* const passwd, const char #undef LOG_FLAG_IF_SET } + conn.tls_disabled_by_user = (flags & XMPP_CONN_FLAG_DISABLE_TLS) ? TRUE : FALSE; + + if (flags & XMPP_CONN_FLAG_DISABLE_TLS) { + log_warning("[SECURITY] TLS is disabled for this connection: traffic and credentials are sent in the clear"); + cons_show_warning("TLS is disabled, this connection is unencrypted."); + } + if (flags & XMPP_CONN_FLAG_TRUST_TLS) { + log_warning("[SECURITY] TLS certificate verification is disabled for this connection"); + cons_show_warning("TLS certificates are not verified for this connection."); + } + if (flags & XMPP_CONN_FLAG_LEGACY_AUTH) { + log_warning("[SECURITY] Legacy (XEP-0078) authentication enabled: the password is sent without SASL"); + cons_show_warning("Legacy authentication is enabled for this connection."); + } + if (xmpp_conn_set_flags(conn.xmpp_conn, flags)) { log_error("libstrophe doesn't accept this combination of flags: 0x%lx", flags); conn.conn_status = JABBER_DISCONNECTED; @@ -364,7 +380,8 @@ _register_handle_proceedtls_default(xmpp_conn_t* xmpp_conn, xmpp_handler_delete(xmpp_conn, _register_handle_error); xmpp_conn_open_stream_default(xmpp_conn); } else { - log_debug("TLS failed."); + log_warning("[SECURITY] TLS handshake failed during registration, aborting"); + cons_show_warning("TLS handshake failed, registration aborted."); /* failed tls spoils the connection, so disconnect */ xmpp_disconnect(xmpp_conn); } @@ -408,6 +425,14 @@ _register_handle_features(xmpp_conn_t* xmpp_conn, xmpp_stanza_t* stanza, void* u return 0; } + /* Registration sends the new password in this stream: refuse to do so in the clear */ + if (!xmpp_conn_is_secured(xmpp_conn)) { + log_warning("[SECURITY] Refusing in-band registration over an unencrypted stream"); + cons_show_error("Registration aborted: the connection is not encrypted and the password would be sent in the clear."); + xmpp_disconnect(xmpp_conn); + return 0; + } + /* check whether server supports in-band registration */ child = xmpp_stanza_get_child_by_name(stanza, "register"); if (!child) { @@ -957,6 +982,12 @@ _connection_handler(xmpp_conn_t* const xmpp_conn, const xmpp_conn_event_t status conn.features_by_jid = g_hash_table_new_full(g_str_hash, g_str_equal, free, (GDestroyNotify)g_hash_table_destroy); g_hash_table_insert(conn.features_by_jid, strdup(conn.domain), g_hash_table_new_full(g_str_hash, g_str_equal, free, NULL)); + // tls.policy=allow negotiates opportunistically, so an unrequested downgrade must not pass silently + if (!connection_is_secured() && !conn.tls_disabled_by_user) { + log_warning("[SECURITY] Logged in over an unencrypted connection to %s: the server offered no usable TLS", conn.domain); + cons_show_warning("This session is NOT encrypted, the server did not provide TLS. Use '/account set tls force' to require it."); + } + session_login_success(connection_is_secured()); if (conn.queued_messages) { @@ -1023,8 +1054,9 @@ _connection_handler(xmpp_conn_t* const xmpp_conn, const xmpp_conn_event_t status int port; if (stream_error && stream_error->stanza && _get_other_host(stream_error->stanza, &host, &port)) { g_assert(port >= 0 && port <= UINT16_MAX); + log_warning("[SECURITY] Server redirected the connection (see-other-host) to \"%s\":%d", host, port); + cons_show_warning("The server redirected this connection to %s:%d.", host, port); session_reconnect(host, (unsigned short)port); - log_debug("Connection handler: Forcing a re-connect to \"%s\"", host); conn.conn_status = JABBER_RECONNECT; return; } @@ -1121,8 +1153,12 @@ _random_bytes_init(void) profanity_instance_id = g_key_file_get_string(keyfile.keyfile, "identifier", "random_bytes", NULL); } else { profanity_instance_id = get_random_string(10); - g_key_file_set_string(keyfile.keyfile, "identifier", "random_bytes", profanity_instance_id); - save_keyfile(&keyfile); + if (profanity_instance_id) { + g_key_file_set_string(keyfile.keyfile, "identifier", "random_bytes", profanity_instance_id); + save_keyfile(&keyfile); + } else { + log_error("Could not generate an instance identifier: no random source available"); + } } free_keyfile(&keyfile); diff --git a/tests/functionaltests/functionaltests.c b/tests/functionaltests/functionaltests.c index 394b31d9..f3bb0a77 100644 --- a/tests/functionaltests/functionaltests.c +++ b/tests/functionaltests/functionaltests.c @@ -117,6 +117,7 @@ main(int argc, char* argv[]) PROF_FUNC_TEST(connect_jid_requests_bookmarks), PROF_FUNC_TEST(connect_bad_password), PROF_FUNC_TEST(connect_shows_presence_updates), + PROF_FUNC_TEST(connect_warns_on_insecure_transport), /* Ping tests - XEP-0199 XMPP Ping */ PROF_FUNC_TEST(ping_server), diff --git a/tests/functionaltests/test_connect.c b/tests/functionaltests/test_connect.c index b10bb029..5aca24f6 100644 --- a/tests/functionaltests/test_connect.c +++ b/tests/functionaltests/test_connect.c @@ -92,3 +92,15 @@ connect_shows_presence_updates(void **state) ); assert_true(prof_output_exact("Buddy1 (mobile) is xa, \"Gone :(\"")); } + +void +connect_warns_on_insecure_transport(void **state) +{ + /* the harness connects with "tls disable auth legacy": both downgrades must reach the console */ + prof_connect(); + + prof_timeout(10); + assert_true(prof_output_exact("Warning: TLS is disabled, this connection is unencrypted.")); + assert_true(prof_output_exact("Warning: Legacy authentication is enabled for this connection.")); + prof_timeout_reset(); +} diff --git a/tests/functionaltests/test_connect.h b/tests/functionaltests/test_connect.h index d82d9364..e1e44a86 100644 --- a/tests/functionaltests/test_connect.h +++ b/tests/functionaltests/test_connect.h @@ -3,3 +3,4 @@ void connect_jid_sends_presence_after_receiving_roster(void **state); void connect_jid_requests_bookmarks(void **state); void connect_bad_password(void **state); void connect_shows_presence_updates(void **state); +void connect_warns_on_insecure_transport(void **state); diff --git a/tests/functionaltests/test_export_import.c b/tests/functionaltests/test_export_import.c index 5117423e..68e9e5ac 100644 --- a/tests/functionaltests/test_export_import.c +++ b/tests/functionaltests/test_export_import.c @@ -977,6 +977,8 @@ migration_incremental_accumulation(void** state) assert_true(prof_output_regex("Database backend switched to 'flatfile'\\.")); prof_input("/msg buddy1@localhost"); + /* full repaint: incremental redraw may emit only partial line text */ + prof_input("/redraw"); assert_true(prof_output_exact("incr-batch1-first")); assert_true(prof_output_exact("incr-batch1-second")); assert_true(prof_output_exact("incr-batch2-third")); diff --git a/tests/unittests/test_common.c b/tests/unittests/test_common.c index 7e876ecf..e86cb69d 100644 --- a/tests/unittests/test_common.c +++ b/tests/unittests/test_common.c @@ -1089,6 +1089,45 @@ release_is_new__tests__various(void** state) assert_false(release_is_new("0.16.0", "")); assert_false(release_is_new(NULL, "1.0.0")); assert_false(release_is_new("1.0.0", NULL)); + + // the found version is untrusted network input: nothing but strictly N.N.N may parse + assert_false(release_is_new("0.16.0", " 1.0.0")); // leading whitespace + assert_false(release_is_new("0.16.0", "+1.0.0")); // explicit sign + assert_false(release_is_new("0.16.0", "-1.0.0")); // negative major + assert_false(release_is_new("0.16.0", "1.0.0-rc1")); // trailing garbage + assert_false(release_is_new("0.16.0", "1.0.0\n")); // trailing newline + assert_false(release_is_new("0.16.0", "1..0")); // empty component + assert_false(release_is_new("0.16.0", "0x10.0.0")); // hex is not accepted + assert_false(release_is_new("0.16.0", "99999999999.0.0")); // out of int range, must not wrap + assert_false(release_is_new("0.16.0", "1.0.0 malicious")); + + // Boundary: the largest still-parsable component is accepted + assert_true(release_is_new("0.16.0", "2147483647.0.0")); +} + +void +get_random_string__generates_valid_ids(void** state) +{ + const gchar* alphabet = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"; + + for (size_t len = 1; len <= 40; len++) { + gchar* id = get_random_string(len); + assert_non_null(id); + assert_int_equal(len, strlen(id)); + for (size_t i = 0; i < len; i++) { + assert_non_null(strchr(alphabet, id[i])); + } + g_free(id); + } + + // a collision here would mean a dead random source + gchar* a = get_random_string(20); + gchar* b = get_random_string(20); + assert_non_null(a); + assert_non_null(b); + assert_string_not_equal(a, b); + g_free(a); + g_free(b); } void diff --git a/tests/unittests/test_common.h b/tests/unittests/test_common.h index 277a7cdb..0adaf04e 100644 --- a/tests/unittests/test_common.h +++ b/tests/unittests/test_common.h @@ -64,5 +64,6 @@ void valid_tls_policy_option__is__correct_for_various_inputs(void** state); void get_mentions__tests__various(void** state); void release_is_new__tests__various(void** state); void str_xml_sanitize__strips_illegal_characters(void** state); +void get_random_string__generates_valid_ids(void** state); #endif diff --git a/tests/unittests/unittests.c b/tests/unittests/unittests.c index cf01788e..7fbad2d0 100644 --- a/tests/unittests/unittests.c +++ b/tests/unittests/unittests.c @@ -687,6 +687,7 @@ main(int argc, char* argv[]) cmocka_unit_test(get_mentions__tests__various), cmocka_unit_test(release_is_new__tests__various), cmocka_unit_test(str_xml_sanitize__strips_illegal_characters), + cmocka_unit_test(get_random_string__generates_valid_ids), cmocka_unit_test_setup_teardown(plugins_get_command_names__returns__no_commands, load_preferences,