Compare commits

..

2 Commits

Author SHA1 Message Date
afd9d84723 security: harden untrusted-input handling (issue #148)
All checks were successful
CI Code / Check spelling (pull_request) Successful in 18s
CI Code / Check coding style (pull_request) Successful in 47s
CI Code / Linux (debian) (pull_request) Successful in 9m33s
CI Code / Linux (ubuntu) (pull_request) Successful in 9m41s
CI Code / Code Coverage (pull_request) Successful in 8m3s
CI Code / Linux (arch) (pull_request) Successful in 11m0s
T02: guard the receive-path handlers that dereferenced jid_create()
without a NULL check — MUC join errors, subscribed/unsubscribed
presence and, with silence.non-roster enabled, every incoming message.
A stanza with a missing or malformed 'from' crashed the client. The
XEP-0280 carbon path carried the same class twice: the forwarded
message's 'from' was handed to xmpp_jid_bare(), which dereferences the
jid it is given, and a malformed 'to' left the carbon dispatch
dereferencing a NULL jid_create() result (REQ-INP-01)

T11: restrict /url open and /url save to http, https and aesgcm, so a
received file:, javascript: or data: URL is refused (REQ-INP-06); spawn
terminal-notifier through g_spawn_async with an argv instead of
building a shell command for system() (REQ-INP-07); apply the XEP-0359
disco gate to MAM result ids, as live stanza-ids already do
(REQ-INP-05); replace control and bidi-reordering characters in
incoming message bodies with U+FFFD before they reach the terminal, the
logs and the database, keeping LRM/RLM for legitimate RTL text. That
pass now runs on every display path: the OX one, where the call had
been left commented out since the feature landed, and outgoing carbons,
whose body is forwarded by the server (REQ-INP-08); cover JID
part-length boundaries and invalid UTF-8 (REQ-INP-02)

T10: replace strcpy/strcat/alloca and sprintf with g_strdup_printf and
g_snprintf (REQ-MEM-03); allocate the OMEMO key buffers with g_malloc
so a failed allocation cannot reach the following memcpy (REQ-MEM-04);
remove the variable-length arrays and enforce -Werror=vla. Two of them
were sized from remote input: the disco#info feature count and a chat
message word length. The flag also caught a one-past-the-end write and
a leak in the plugin autocompleter bindings (REQ-MEM-09). The OX
receive path leaked every decrypted body, dropping the pointer instead
of freeing it, and a failed strdup no longer costs the message body
2026-08-06 15:35:27 +03:00
d914e42ff6 fix(xmpp): treat disco#info result without 'from' as from the server
All checks were successful
CI Code / Check coding style (pull_request) Successful in 23s
CI Code / Check spelling (pull_request) Successful in 14s
CI Code / Linux (debian) (pull_request) Successful in 4m57s
CI Code / Linux (arch) (pull_request) Successful in 6m23s
CI Code / Linux (ubuntu) (pull_request) Successful in 7m59s
CI Code / Code Coverage (pull_request) Successful in 9m40s
CI Code / Check spelling (push) Successful in 15s
CI Code / Check coding style (push) Successful in 25s
CI Code / Code Coverage (push) Successful in 3m4s
CI Code / Linux (arch) (push) Successful in 6m25s
Publish Docker image / Push Docker image to Docker Hub (push) Successful in 7m29s
CI Code / Linux (ubuntu) (push) Successful in 8m21s
CI Code / Linux (debian) (push) Successful in 8m39s
RFC 6120 §8.1.2.1: a stanza received over a c2s stream without a 'from'
attribute must be treated as coming from the server itself. The
on-connect disco#info handler passed the absent attribute as NULL into
connection_features_received(), where g_str_hash() dereferenced the NULL
key and crashed (remotely triggerable DoS on connect).

Substitute connection_get_domain() at both disco#info handler
boundaries, and make connection_features_received() and
connection_get_features() NULL-safe as defense in depth. Add a stabber
regression test answering the on-connect disco#info with a from-less
result.

Fixes #168
2026-07-28 22:12:48 +03:00
35 changed files with 605 additions and 114 deletions

View File

@@ -405,6 +405,8 @@ AM_CFLAGS="$AM_CFLAGS -Wimplicit-function-declaration"
AM_CFLAGS="$AM_CFLAGS -Wundef" AM_CFLAGS="$AM_CFLAGS -Wundef"
AM_CFLAGS="$AM_CFLAGS -Wfloat-equal -Wredundant-decls" AM_CFLAGS="$AM_CFLAGS -Wfloat-equal -Wredundant-decls"
AM_CFLAGS="$AM_CFLAGS -fstack-protector-strong -fno-common" AM_CFLAGS="$AM_CFLAGS -fstack-protector-strong -fno-common"
# stack arrays sized from peer input are a remote stack-exhaustion vector
AM_CFLAGS="$AM_CFLAGS -Werror=vla"
AM_CFLAGS="$AM_CFLAGS -std=gnu99 -ggdb3" AM_CFLAGS="$AM_CFLAGS -std=gnu99 -ggdb3"
# GCC-specific warnings (not supported by clang) — test each one # GCC-specific warnings (not supported by clang) — test each one

View File

@@ -58,6 +58,8 @@ cc = meson.get_compiler('c')
add_project_arguments([ add_project_arguments([
'-Wno-deprecated-declarations', '-Wno-deprecated-declarations',
'-Wno-unused-parameter', '-Wno-unused-parameter',
# stack arrays sized from peer input are a remote stack-exhaustion vector
'-Werror=vla',
], language: 'c') ], language: 'c')
analyzer_args = [] analyzer_args = []

View File

@@ -1955,7 +1955,10 @@ _cmd_ac_complete_params(ProfWin* window, const char* const input, gboolean previ
} }
size_t len = strlen(input); size_t len = strlen(input);
char parsed[len + 1]; auto_char char* parsed = malloc(len + 1);
if (!parsed) {
return NULL;
}
size_t i = 0; size_t i = 0;
while (i < len) { while (i < len) {
if (input[i] == ' ') { if (input[i] == ' ') {

View File

@@ -9644,6 +9644,13 @@ cmd_url_open(ProfWin* window, const char* const command, gchar** args)
return TRUE; return TRUE;
} }
// URLs come from received messages: refuse file:, javascript:, data: etc.
if (g_strcmp0(scheme, "http") != 0 && g_strcmp0(scheme, "https") != 0
&& g_strcmp0(scheme, "aesgcm") != 0) {
cons_show_error("URL scheme '%s' is not allowed, only http, https and aesgcm URLs can be opened.", scheme);
return TRUE;
}
auto_gchar gchar* cmd_template = prefs_get_string(PREF_URL_OPEN_CMD); auto_gchar gchar* cmd_template = prefs_get_string(PREF_URL_OPEN_CMD);
if (cmd_template == NULL) { if (cmd_template == NULL) {
cons_show_error("No default `url open` command found in executables preferences."); cons_show_error("No default `url open` command found in executables preferences.");
@@ -9689,6 +9696,13 @@ cmd_url_save(ProfWin* window, const char* const command, gchar** args)
return TRUE; return TRUE;
} }
// URLs come from received messages: refuse file:, javascript:, data: etc.
if (g_strcmp0(scheme, "http") != 0 && g_strcmp0(scheme, "https") != 0
&& g_strcmp0(scheme, "aesgcm") != 0) {
cons_show_error("URL scheme '%s' is not allowed, only http, https and aesgcm URLs can be saved.", scheme);
return TRUE;
}
auto_gchar gchar* cmd_template = prefs_get_string(PREF_URL_SAVE_CMD); auto_gchar gchar* cmd_template = prefs_get_string(PREF_URL_SAVE_CMD);
if (cmd_template == NULL && (g_strcmp0(scheme, "http") == 0 || g_strcmp0(scheme, "https") == 0)) { if (cmd_template == NULL && (g_strcmp0(scheme, "http") == 0 || g_strcmp0(scheme, "https") == 0)) {
_url_http_method(window, cmd_template, url, path); _url_http_method(window, cmd_template, url, path);

View File

@@ -455,6 +455,47 @@ str_xml_sanitize(const char* const str)
return g_string_free(sanitized, FALSE); return g_string_free(sanitized, FALSE);
} }
static gboolean
_is_neutralized_char(gunichar c)
{
if (c == 0x09 || c == 0x0A || c == 0x0D) { // tab and newlines are legitimate in a body
return FALSE;
}
if (c < 0x20 || (c >= 0x7F && c <= 0x9F)) { // C0 and C1, incl. ESC: terminal escape sequences
return TRUE;
}
// bidi embeddings, overrides and isolates: they reorder what the user sees.
// LRM/RLM (U+200E/U+200F) are kept, legitimate RTL text needs them.
return (c >= 0x202A && c <= 0x202E) || (c >= 0x2066 && c <= 0x2069);
}
/**
* Replaces control and bidi-reordering characters in untrusted text with U+FFFD.
*
* Replaced rather than dropped, so the substitution stays visible to the user.
*/
gchar*
str_neutralize_untrusted(const char* const str)
{
if (str == NULL) {
return NULL;
}
auto_gchar gchar* valid = g_utf8_make_valid(str, -1);
GString* out = g_string_new_len(NULL, (gssize)strlen(valid));
for (const char* curr = valid; *curr != '\0'; curr = g_utf8_next_char(curr)) {
gunichar c = g_utf8_get_char(curr);
if (_is_neutralized_char(c)) {
g_string_append_unichar(out, 0xFFFD);
} else {
g_string_append_unichar(out, c);
}
}
return g_string_free(out, FALSE);
}
char* char*
release_get_latest(void) release_get_latest(void)
{ {

View File

@@ -161,6 +161,7 @@ gboolean strtoi_range(const char* str, int* saveptr, int min, int max, char** er
gsize g_diff_to_gsize(const void* end, const void* start); gsize g_diff_to_gsize(const void* end, const void* start);
int utf8_display_len(const char* const str); int utf8_display_len(const char* const str);
gchar* str_xml_sanitize(const char* const str); gchar* str_xml_sanitize(const char* const str);
gchar* str_neutralize_untrusted(const char* const str);
gboolean string_matches_one_of(const char* what, const char* is, gboolean is_can_be_null, const char* first, ...) __attribute__((sentinel)); gboolean string_matches_one_of(const char* what, const char* is, gboolean is_can_be_null, const char* first, ...) __attribute__((sentinel));
gboolean valid_tls_policy_option(const char* is); gboolean valid_tls_policy_option(const char* is);

View File

@@ -434,16 +434,13 @@ sv_ev_outgoing_carbon(ProfMessage* message)
chat_state_active(chatwin->state); chat_state_active(chatwin->state);
if (message->enc == PROF_MSG_ENC_OMEMO) { // OMEMO and OX plaintext is already in message->plain by now
chatwin_outgoing_carbon(chatwin, message); if (message->enc != PROF_MSG_ENC_OMEMO && message->enc != PROF_MSG_ENC_OX) {
} else if (message->enc == PROF_MSG_ENC_OX) { if (message->encrypted) {
chatwin_outgoing_carbon(chatwin, message);
} else if (message->encrypted) {
#ifdef HAVE_LIBGPGME #ifdef HAVE_LIBGPGME
message->plain = p_gpg_decrypt(message->encrypted); message->plain = p_gpg_decrypt(message->encrypted);
if (message->plain) { if (message->plain) {
message->enc = PROF_MSG_ENC_PGP; message->enc = PROF_MSG_ENC_PGP;
chatwin_outgoing_carbon(chatwin, message);
} else { } else {
if (!message->body) { if (!message->body) {
log_error("Couldn't decrypt GPG message and body was empty"); log_error("Couldn't decrypt GPG message and body was empty");
@@ -451,16 +448,22 @@ sv_ev_outgoing_carbon(ProfMessage* message)
} }
message->enc = PROF_MSG_ENC_NONE; message->enc = PROF_MSG_ENC_NONE;
message->plain = strdup(message->body); message->plain = strdup(message->body);
chatwin_outgoing_carbon(chatwin, message);
} }
#endif #endif
} else { } else {
message->enc = PROF_MSG_ENC_NONE; message->enc = PROF_MSG_ENC_NONE;
message->plain = strdup(message->body); message->plain = strdup(message->body);
chatwin_outgoing_carbon(chatwin, message); }
} }
if (message->plain) { if (message->plain == NULL) {
return;
}
// the carbon is forwarded by the server, so its body is untrusted like any incoming one
_clean_incoming_message(message);
chatwin_outgoing_carbon(chatwin, message);
if (message->type == PROF_MSG_TYPE_MUCPM) { if (message->type == PROF_MSG_TYPE_MUCPM) {
// MUC PM, should have resource (nick) in filename // MUC PM, should have resource (nick) in filename
chat_log_msg_out(message->to_jid->barejid, message->plain, message->from_jid->resourcepart); chat_log_msg_out(message->to_jid->barejid, message->plain, message->from_jid->resourcepart);
@@ -469,8 +472,6 @@ sv_ev_outgoing_carbon(ProfMessage* message)
} }
log_database_add_incoming(message); log_database_add_incoming(message);
} }
return;
}
static void static void
_sv_ev_incoming_pgp(ProfChatWin* chatwin, gboolean new_win, ProfMessage* message, gboolean logit) _sv_ev_incoming_pgp(ProfChatWin* chatwin, gboolean new_win, ProfMessage* message, gboolean logit)
@@ -514,16 +515,19 @@ _sv_ev_incoming_ox(ProfChatWin* chatwin, gboolean new_win, ProfMessage* message,
return; return;
} }
message->plain = strdup(message->body); message->plain = strdup(message->body);
if (message->plain == NULL) {
return;
}
} }
//_clean_incoming_message(message); _clean_incoming_message(message);
chatwin_incoming_msg(chatwin, message, new_win); chatwin_incoming_msg(chatwin, message, new_win);
log_database_add_incoming(message); log_database_add_incoming(message);
if (logit) { if (logit) {
chat_log_pgp_msg_in(message); chat_log_pgp_msg_in(message);
} }
chatwin->pgp_recv = TRUE; chatwin->pgp_recv = TRUE;
// p_gpg_free_decrypted(message->plain); free(message->plain); // stanza text and strdup alike come from malloc, see prof_mem
message->plain = NULL; message->plain = NULL;
#endif #endif
} }
@@ -1303,4 +1307,16 @@ _clean_incoming_message(ProfMessage* message)
{ {
_cut(message, "\u200E"); _cut(message, "\u200E");
_cut(message, "\u200F"); _cut(message, "\u200F");
// runs after decryption, so OTR/PGP/OX plaintext is covered too
if (message->plain) {
auto_gchar gchar* clean = str_neutralize_untrusted(message->plain);
if (g_strcmp0(clean, message->plain) != 0) {
char* neutralized = strdup(clean); // message_free() frees this with free()
if (neutralized) { // on failure keep the original rather than lose the body
free(message->plain);
message->plain = neutralized;
}
}
}
} }

View File

@@ -104,7 +104,7 @@ omemo_hmac_sha256_final_func(void* hmac_context, signal_buffer** output, void* u
{ {
gcry_error_t res; gcry_error_t res;
size_t mac_len = 32; size_t mac_len = 32;
unsigned char out[mac_len]; unsigned char out[32];
res = gcry_mac_read(hmac_context, out, &mac_len); res = gcry_mac_read(hmac_context, out, &mac_len);
if (res != GPG_ERR_NO_ERROR) { if (res != GPG_ERR_NO_ERROR) {
@@ -473,14 +473,19 @@ aes256gcm_create_secure_fragment(unsigned char* key, unsigned char* nonce)
int key_size = OMEMO_AESGCM_KEY_LENGTH; int key_size = OMEMO_AESGCM_KEY_LENGTH;
int nonce_size = OMEMO_AESGCM_NONCE_LENGTH; int nonce_size = OMEMO_AESGCM_NONCE_LENGTH;
char* fragment = gcry_malloc_secure((nonce_size + key_size) * 2 + 1); const size_t fragment_size = (size_t)(nonce_size + key_size) * 2 + 1;
char* fragment = gcry_malloc_secure(fragment_size);
if (fragment == NULL) {
return NULL;
}
for (int i = 0; i < nonce_size; i++) { for (int i = 0; i < nonce_size; i++) {
sprintf(&(fragment[i * 2]), "%02x", nonce[i]); g_snprintf(&fragment[i * 2], fragment_size - (size_t)i * 2, "%02x", nonce[i]);
} }
for (int i = 0; i < key_size; i++) { for (int i = 0; i < key_size; i++) {
sprintf(&(fragment[(i + nonce_size) * 2]), "%02x", key[i]); const size_t offset = (size_t)(i + nonce_size) * 2;
g_snprintf(&fragment[offset], fragment_size - offset, "%02x", key[i]);
} }
return fragment; return fragment;

View File

@@ -452,8 +452,12 @@ omemo_identity_key(unsigned char** output, size_t* length)
signal_buffer* buffer = NULL; signal_buffer* buffer = NULL;
ec_public_key_serialize(&buffer, ratchet_identity_key_pair_get_public(omemo_ctx.identity_key_pair)); ec_public_key_serialize(&buffer, ratchet_identity_key_pair_get_public(omemo_ctx.identity_key_pair));
*length = signal_buffer_len(buffer); *length = signal_buffer_len(buffer);
*output = malloc(*length); // g_malloc aborts instead of returning NULL, but it returns NULL for a zero
// length, which memcpy must not be handed
*output = *length ? g_malloc(*length) : NULL;
if (*output) {
memcpy(*output, signal_buffer_data(buffer), *length); memcpy(*output, signal_buffer_data(buffer), *length);
}
signal_buffer_free(buffer); signal_buffer_free(buffer);
} }
@@ -472,8 +476,10 @@ omemo_signed_prekey(unsigned char** output, size_t* length)
ec_public_key_serialize(&buffer, ec_key_pair_get_public(session_signed_pre_key_get_key_pair(signed_pre_key))); ec_public_key_serialize(&buffer, ec_key_pair_get_public(session_signed_pre_key_get_key_pair(signed_pre_key)));
SIGNAL_UNREF(signed_pre_key); SIGNAL_UNREF(signed_pre_key);
*length = signal_buffer_len(buffer); *length = signal_buffer_len(buffer);
*output = malloc(*length); *output = *length ? g_malloc(*length) : NULL;
if (*output) {
memcpy(*output, signal_buffer_data(buffer), *length); memcpy(*output, signal_buffer_data(buffer), *length);
}
signal_buffer_free(buffer); signal_buffer_free(buffer);
} }
@@ -489,8 +495,10 @@ omemo_signed_prekey_signature(unsigned char** output, size_t* length)
} }
*length = session_signed_pre_key_get_signature_len(signed_pre_key); *length = session_signed_pre_key_get_signature_len(signed_pre_key);
*output = malloc(*length); *output = *length ? g_malloc(*length) : NULL;
if (*output) {
memcpy(*output, session_signed_pre_key_get_signature(signed_pre_key), *length); memcpy(*output, session_signed_pre_key_get_signature(signed_pre_key), *length);
}
SIGNAL_UNREF(signed_pre_key); SIGNAL_UNREF(signed_pre_key);
} }

View File

@@ -147,13 +147,8 @@ p_ox_gpg_signcrypt(const char* const sender_barejid, const char* const recipient
gpgme_set_offline(ctx, 1); gpgme_set_offline(ctx, 1);
gpgme_set_keylist_mode(ctx, GPGME_KEYLIST_MODE_LOCAL); gpgme_set_keylist_mode(ctx, GPGME_KEYLIST_MODE_LOCAL);
char* xmpp_jid_me = alloca((strlen(sender_barejid) + 6) * sizeof(char)); auto_gchar gchar* xmpp_jid_me = g_strdup_printf("xmpp:%s", sender_barejid);
char* xmpp_jid_recipient = alloca((strlen(recipient_barejid) + 6) * sizeof(char)); auto_gchar gchar* xmpp_jid_recipient = g_strdup_printf("xmpp:%s", recipient_barejid);
strcpy(xmpp_jid_me, "xmpp:");
strcpy(xmpp_jid_recipient, "xmpp:");
strcat(xmpp_jid_me, sender_barejid);
strcat(xmpp_jid_recipient, recipient_barejid);
gpgme_signers_clear(ctx); gpgme_signers_clear(ctx);

View File

@@ -122,7 +122,7 @@ python_api_register_command(PyObject* self, PyObject* args)
if (p_callback && PyCallable_Check(p_callback)) { if (p_callback && PyCallable_Check(p_callback)) {
Py_ssize_t len = PyList_Size(synopsis); Py_ssize_t len = PyList_Size(synopsis);
char* c_synopsis[len == 0 ? 0 : len + 1]; char** c_synopsis = g_malloc_n((size_t)len + 1, sizeof(*c_synopsis));
Py_ssize_t i = 0; Py_ssize_t i = 0;
for (i = 0; i < len; i++) { for (i = 0; i < len; i++) {
PyObject* item = PyList_GetItem(synopsis, i); PyObject* item = PyList_GetItem(synopsis, i);
@@ -132,11 +132,23 @@ python_api_register_command(PyObject* self, PyObject* args)
c_synopsis[len] = NULL; c_synopsis[len] = NULL;
Py_ssize_t args_len = PyList_Size(arguments); Py_ssize_t args_len = PyList_Size(arguments);
char* c_arguments[args_len + 1][2]; char*(*c_arguments)[2] = g_malloc_n((size_t)args_len + 1, sizeof(*c_arguments));
for (i = 0; i < args_len; i++) { for (i = 0; i < args_len; i++) {
PyObject* item = PyList_GetItem(arguments, i); PyObject* item = PyList_GetItem(arguments, i);
Py_ssize_t len2 = PyList_Size(item); Py_ssize_t len2 = PyList_Size(item);
if (len2 != 2) { if (len2 != 2) {
for (Py_ssize_t j = 0; j < len; j++) {
free(c_synopsis[j]);
}
for (Py_ssize_t j = 0; j < i; j++) {
free(c_arguments[j][0]);
free(c_arguments[j][1]);
}
g_free(c_synopsis);
g_free(c_arguments);
free(command_name_str);
free(description_str);
free(plugin_name);
Py_RETURN_NONE; Py_RETURN_NONE;
} }
PyObject* arg = PyList_GetItem(item, 0); PyObject* arg = PyList_GetItem(item, 0);
@@ -152,7 +164,7 @@ python_api_register_command(PyObject* self, PyObject* args)
c_arguments[args_len][1] = NULL; c_arguments[args_len][1] = NULL;
len = PyList_Size(examples); len = PyList_Size(examples);
char* c_examples[len == 0 ? 0 : len + 1]; char** c_examples = g_malloc_n((size_t)len + 1, sizeof(*c_examples));
for (i = 0; i < len; i++) { for (i = 0; i < len; i++) {
PyObject* item = PyList_GetItem(examples, i); PyObject* item = PyList_GetItem(examples, i);
char* c_item = python_str_or_unicode_to_string(item); char* c_item = python_str_or_unicode_to_string(item);
@@ -179,6 +191,9 @@ python_api_register_command(PyObject* self, PyObject* args)
while (c_examples[i] != NULL) { while (c_examples[i] != NULL) {
free(c_examples[i++]); free(c_examples[i++]);
} }
g_free(c_synopsis);
g_free(c_arguments);
g_free(c_examples);
disable_python_threads(); disable_python_threads();
} }
@@ -227,7 +242,7 @@ python_api_completer_add(PyObject* self, PyObject* args)
log_debug("Autocomplete add %s for %s", key_str, plugin_name); log_debug("Autocomplete add %s for %s", key_str, plugin_name);
Py_ssize_t len = PyList_Size(items); Py_ssize_t len = PyList_Size(items);
char* c_items[len]; char** c_items = g_malloc_n((size_t)len + 1, sizeof(*c_items)); // + 1 for the NULL terminator below
Py_ssize_t i = 0; Py_ssize_t i = 0;
for (i = 0; i < len; i++) { for (i = 0; i < len; i++) {
@@ -244,6 +259,7 @@ python_api_completer_add(PyObject* self, PyObject* args)
while (c_items[i] != NULL) { while (c_items[i] != NULL) {
free(c_items[i++]); free(c_items[i++]);
} }
g_free(c_items);
disable_python_threads(); disable_python_threads();
free(plugin_name); free(plugin_name);
@@ -267,7 +283,7 @@ python_api_completer_remove(PyObject* self, PyObject* args)
log_debug("Autocomplete remove %s for %s", key_str, plugin_name); log_debug("Autocomplete remove %s for %s", key_str, plugin_name);
Py_ssize_t len = PyList_Size(items); Py_ssize_t len = PyList_Size(items);
char* c_items[len]; char** c_items = g_malloc_n((size_t)len + 1, sizeof(*c_items)); // + 1 for the NULL terminator below
Py_ssize_t i = 0; Py_ssize_t i = 0;
for (i = 0; i < len; i++) { for (i = 0; i < len; i++) {
@@ -280,6 +296,11 @@ python_api_completer_remove(PyObject* self, PyObject* args)
allow_python_threads(); allow_python_threads();
api_completer_remove(plugin_name, key_str, c_items); api_completer_remove(plugin_name, key_str, c_items);
free(key_str); free(key_str);
i = 0;
while (c_items[i] != NULL) {
free(c_items[i++]);
}
g_free(c_items);
disable_python_threads(); disable_python_threads();
free(plugin_name); free(plugin_name);

View File

@@ -493,7 +493,7 @@ _inp_edited(const wint_t ch)
} }
// printable // printable
char bytes[MB_CUR_MAX + 1]; char bytes[PROF_MB_CUR_MAX + 1]; // compile-time bound, the locale value is checked at startup
size_t utf_len = wcrtomb(bytes, ch, &mbstate); size_t utf_len = wcrtomb(bytes, ch, &mbstate);
if (utf_len == (size_t)-1) { if (utf_len == (size_t)-1) {
return 0; return 0;

View File

@@ -454,22 +454,10 @@ _mucwin_print_triggers(ProfWin* window, const char* const message, GList* trigge
win_appendln_highlight(window, THEME_ROOMTRIGGER, "%s", message); win_appendln_highlight(window, THEME_ROOMTRIGGER, "%s", message);
} else { } else {
if (first_trigger_pos > 0) { if (first_trigger_pos > 0) {
char message_section[strlen(message) + 1]; auto_gchar gchar* message_section = g_strndup(message, (gsize)first_trigger_pos);
int i = 0;
while (i < first_trigger_pos) {
message_section[i] = message[i];
i++;
}
message_section[i] = '\0';
win_append_highlight(window, THEME_ROOMTRIGGER, "%s", message_section); win_append_highlight(window, THEME_ROOMTRIGGER, "%s", message_section);
} }
char trigger_section[first_trigger_len + 1]; auto_gchar gchar* trigger_section = g_strndup(&message[first_trigger_pos], (gsize)first_trigger_len);
int i = 0;
while (i < first_trigger_len) {
trigger_section[i] = message[first_trigger_pos + i];
i++;
}
trigger_section[i] = '\0';
if (first_trigger_pos + first_trigger_len < (int)strlen(message)) { if (first_trigger_pos + first_trigger_len < (int)strlen(message)) {
win_append_highlight(window, THEME_ROOMTRIGGER_TERM, "%s", trigger_section); win_append_highlight(window, THEME_ROOMTRIGGER_TERM, "%s", trigger_section);

View File

@@ -22,6 +22,7 @@
#include <windows.h> #include <windows.h>
#endif #endif
#include "common.h"
#include "log.h" #include "log.h"
#include "config/preferences.h" #include "config/preferences.h"
#include "ui/ui.h" #include "ui/ui.h"
@@ -122,47 +123,42 @@ _notify(const char* const message, int timeout, const char* const category)
static void static void
_notify(const char* const message, int timeout, const char* const category) _notify(const char* const message, int timeout, const char* const category)
{ {
GString* notify_command = g_string_new("terminal-notifier -title \"Profanity\" -message '"); if (message == NULL) {
return;
}
auto_char char* escaped_single = str_replace(message, "'", "'\\''"); // terminal-notifier swallows a leading <, [, ( or { — escape it for its parser
auto_gchar gchar* msg = NULL;
if (escaped_single[0] == '<') { if (message[0] == '<' || message[0] == '[' || message[0] == '(' || message[0] == '{') {
g_string_append(notify_command, "\\<"); msg = g_strdup_printf("\\%s", message);
g_string_append(notify_command, &escaped_single[1]);
} else if (escaped_single[0] == '[') {
g_string_append(notify_command, "\\[");
g_string_append(notify_command, &escaped_single[1]);
} else if (escaped_single[0] == '(') {
g_string_append(notify_command, "\\(");
g_string_append(notify_command, &escaped_single[1]);
} else if (escaped_single[0] == '{') {
g_string_append(notify_command, "\\{");
g_string_append(notify_command, &escaped_single[1]);
} else { } else {
g_string_append(notify_command, escaped_single); msg = g_strdup(message);
} }
g_string_append(notify_command, "'"); const gchar* argv[16]; // headroom: an added flag must not silently overrun
guint i = 0;
argv[i++] = "terminal-notifier";
argv[i++] = "-title";
argv[i++] = "Profanity";
argv[i++] = "-message";
argv[i++] = msg;
char* term_name = getenv("TERM_PROGRAM"); const char* term_name = getenv("TERM_PROGRAM");
char* app_id = NULL;
if (g_strcmp0(term_name, "Apple_Terminal") == 0) { if (g_strcmp0(term_name, "Apple_Terminal") == 0) {
app_id = "com.apple.Terminal"; argv[i++] = "-sender";
argv[i++] = "com.apple.Terminal";
} else if (g_strcmp0(term_name, "iTerm.app") == 0) { } else if (g_strcmp0(term_name, "iTerm.app") == 0) {
app_id = "com.googlecode.iterm2"; argv[i++] = "-sender";
argv[i++] = "com.googlecode.iterm2";
} }
argv[i] = NULL;
if (app_id) { GError* err = NULL;
g_string_append(notify_command, " -sender "); // argv-based spawn: the message is one argument, the shell never sees it
g_string_append(notify_command, app_id); if (!g_spawn_async(NULL, (gchar**)argv, NULL, G_SPAWN_SEARCH_PATH, NULL, NULL, NULL, &err)) {
log_error("Could not send desktop notification: %s", err->message);
g_error_free(err);
} }
int res = system(notify_command->str);
if (res == -1) {
log_error("Could not send desktop notification.");
}
g_string_free(notify_command, TRUE);
} }
#else #else
static void static void

View File

@@ -32,6 +32,7 @@
#include <curses.h> #include <curses.h>
#endif #endif
#include "common.h"
#include "log.h" #include "log.h"
#include "config/theme.h" #include "config/theme.h"
#include "config/preferences.h" #include "config/preferences.h"
@@ -2115,7 +2116,7 @@ _win_print_wrapped(WINDOW* win, const char* const message, int indent, int pad_i
_win_indent(win, indent + pad_indent); _win_indent(win, indent + pad_indent);
} }
gchar copy[wordi + 1]; gchar copy[PROF_MB_CUR_MAX + 1]; // one UTF-8 character, not the whole word
g_utf8_strncpy(copy, word_ch, 1); g_utf8_strncpy(copy, word_ch, 1);
waddstr(win, copy); waddstr(win, copy);

View File

@@ -247,14 +247,16 @@ caps_add_by_ver(const char* const ver, EntityCapabilities* caps)
if (caps->features) { if (caps->features) {
GSList* curr_feature = caps->features; GSList* curr_feature = caps->features;
int num = g_slist_length(caps->features); // the feature count comes from a disco#info response, keep it off the stack
const gchar* features_list[num]; guint num = g_slist_length(caps->features);
int curr = 0; const gchar** features_list = g_malloc_n(num, sizeof(*features_list));
guint curr = 0;
while (curr_feature) { while (curr_feature) {
features_list[curr++] = curr_feature->data; features_list[curr++] = curr_feature->data;
curr_feature = g_slist_next(curr_feature); curr_feature = g_slist_next(curr_feature);
} }
g_key_file_set_string_list(cache, ver, "features", features_list, num); g_key_file_set_string_list(cache, ver, "features", features_list, num);
g_free(features_list);
} }
_save_cache(); _save_cache();

View File

@@ -749,11 +749,22 @@ connection_get_user(void)
return connection_get_jid()->localpart; return connection_get_jid()->localpart;
} }
// NULL 'from' means the server (RFC 6120 §8.1.2.1)
static const char*
_get_from_via_jid(const char* const jid)
{
return jid ? jid : conn.domain;
}
void void
connection_features_received(const char* const jid) connection_features_received(const char* const jid)
{ {
log_info("[CONNECTION] connection_features_received %s", jid); const char* key = _get_from_via_jid(jid);
if (g_hash_table_remove(conn.requested_features, jid) && g_hash_table_size(conn.requested_features) == 0) { if (!key) {
return;
}
log_info("[CONNECTION] connection_features_received %s", key);
if (g_hash_table_remove(conn.requested_features, key) && g_hash_table_size(conn.requested_features) == 0) {
sv_ev_connection_features_received(); sv_ev_connection_features_received();
} }
} }
@@ -761,7 +772,11 @@ connection_features_received(const char* const jid)
GHashTable* GHashTable*
connection_get_features(const char* const jid) connection_get_features(const char* const jid)
{ {
return g_hash_table_lookup(conn.features_by_jid, jid); const char* key = _get_from_via_jid(jid);
if (!key || !conn.features_by_jid) {
return NULL;
}
return g_hash_table_lookup(conn.features_by_jid, key);
} }
GList* GList*

View File

@@ -1784,7 +1784,7 @@ _last_activity_get_handler(xmpp_stanza_t* const stanza)
if (prefs_get_boolean(PREF_LASTACTIVITY)) { if (prefs_get_boolean(PREF_LASTACTIVITY)) {
int idls_secs = (int)(ui_get_idle_time() / 1000); int idls_secs = (int)(ui_get_idle_time() / 1000);
char str[50]; char str[50];
sprintf(str, "%d", idls_secs); g_snprintf(str, sizeof(str), "%d", idls_secs);
xmpp_stanza_t* response = xmpp_iq_new(ctx, STANZA_TYPE_RESULT, xmpp_stanza_get_id(stanza)); xmpp_stanza_t* response = xmpp_iq_new(ctx, STANZA_TYPE_RESULT, xmpp_stanza_get_id(stanza));
xmpp_stanza_set_to(response, from); xmpp_stanza_set_to(response, from);
@@ -2314,6 +2314,7 @@ _disco_info_response_id_handler(xmpp_stanza_t* const stanza, void* const userdat
log_debug("Received disco#info response from: %s", from); log_debug("Received disco#info response from: %s", from);
} else { } else {
log_debug("Received disco#info response"); log_debug("Received disco#info response");
from = connection_get_domain(); // RFC 6120 §8.1.2.1: no 'from' means the server itself
} }
// handle error responses // handle error responses
@@ -2397,6 +2398,7 @@ _disco_info_response_id_handler_onconnect(xmpp_stanza_t* const stanza, void* con
log_debug("Received disco#info response from: %s", from); log_debug("Received disco#info response from: %s", from);
} else { } else {
log_debug("Received disco#info response"); log_debug("Received disco#info response");
from = connection_get_domain(); // RFC 6120 §8.1.2.1: no 'from' means the server itself
} }
// handle error responses // handle error responses

View File

@@ -1342,8 +1342,14 @@ _handle_carbons(xmpp_stanza_t* const stanza)
return NULL; return NULL;
} }
const char* const forwarded_from = xmpp_stanza_get_from(message_stanza);
if (!forwarded_from) { // xmpp_jid_bare() dereferences the jid it is handed
log_warning("Carbon received with no 'from' in the forwarded message");
return NULL;
}
// Eliminate duplicate messages in chat with oneself when another client is sending a message // Eliminate duplicate messages in chat with oneself when another client is sending a message
auto_char char* bare_from = xmpp_jid_bare(connection_get_ctx(), xmpp_stanza_get_from(message_stanza)); auto_char char* bare_from = xmpp_jid_bare(connection_get_ctx(), forwarded_from);
if (g_strcmp0(bare_from, xmpp_stanza_get_to(message_stanza)) == 0) { if (g_strcmp0(bare_from, xmpp_stanza_get_to(message_stanza)) == 0) {
return NULL; return NULL;
} }
@@ -1467,8 +1473,10 @@ _handle_chat(xmpp_stanza_t* const stanza, gboolean is_mam, gboolean is_carbon, c
if (message->plain || message->body || message->encrypted) { if (message->plain || message->body || message->encrypted) {
if (is_carbon) { if (is_carbon) {
if (message->to_jid == NULL) { // a malformed 'to' leaves no usable recipient
log_warning("Carbon received with missing or invalid 'to' attribute");
// if we are the recipient, treat as standard incoming message // if we are the recipient, treat as standard incoming message
if (equals_our_barejid(message->to_jid->barejid)) { } else if (equals_our_barejid(message->to_jid->barejid)) {
sv_ev_incoming_carbon(message); sv_ev_incoming_carbon(message);
// else treat as a sent message // else treat as a sent message
} else { } else {
@@ -1562,6 +1570,20 @@ _handle_mam(xmpp_stanza_t* const stanza)
const char* result_id = xmpp_stanza_get_id(result); const char* result_id = xmpp_stanza_get_id(result);
// XEP-0359 §6 gate, same as for live <stanza-id>: only keep the archive
// id if the archive's owner announces stable ids
if (result_id) {
const char* archive = by ? by : from;
if (archive == NULL) {
const Jid* myjid = connection_get_jid();
archive = myjid ? myjid->barejid : NULL;
}
if (!_stanza_id_by_trusted(archive)) {
log_debug("MAM result id dropped: archive %s does not announce XEP-0359 support", STR_MAYBE_NULL(archive));
result_id = NULL;
}
}
GDateTime* timestamp = stanza_get_delay_from(forwarded, NULL); GDateTime* timestamp = stanza_get_delay_from(forwarded, NULL);
xmpp_stanza_t* message_stanza = xmpp_stanza_get_child_by_ns(forwarded, "jabber:client"); xmpp_stanza_t* message_stanza = xmpp_stanza_get_child_by_ns(forwarded, "jabber:client");
@@ -1658,7 +1680,7 @@ _ox_openpgp_signcrypt(xmpp_ctx_t* ctx, const char* const to, const char* const t
// build rpad // build rpad
int randnr = (rand() % 100) + 1; int randnr = (rand() % 100) + 1;
char rpad_data[randnr]; char rpad_data[101];
for (int i = 0; i < randnr; i++) { for (int i = 0; i < randnr; i++) {
int rchar = (rand() % 52) + 65; int rchar = (rand() % 52) + 65;
rpad_data[i] = (char)rchar; rpad_data[i] = (char)rchar;
@@ -1747,6 +1769,10 @@ _should_ignore_based_on_silence(xmpp_stanza_t* const stanza)
if (prefs_get_boolean(PREF_SILENCE_NON_ROSTER)) { if (prefs_get_boolean(PREF_SILENCE_NON_ROSTER)) {
const char* const from = xmpp_stanza_get_from(stanza); const char* const from = xmpp_stanza_get_from(stanza);
auto_jid Jid* from_jid = jid_create(from); auto_jid Jid* from_jid = jid_create(from);
if (from_jid == NULL) { // no usable sender: treat as non-roster and ignore
log_debug("[Silence] Ignoring message with missing or invalid from attribute");
return TRUE;
}
PContact contact = roster_get_contact(from_jid->barejid); PContact contact = roster_get_contact(from_jid->barejid);
if (!contact) { if (!contact) {
log_debug("[Silence] Ignoring message from: %s", from); log_debug("[Silence] Ignoring message from: %s", from);

View File

@@ -130,9 +130,9 @@ omemo_bundle_publish(gboolean first)
iq_send_stanza(iq); iq_send_stanza(iq);
xmpp_stanza_release(iq); xmpp_stanza_release(iq);
free(identity_key); g_free(identity_key);
free(signed_prekey); g_free(signed_prekey);
free(signed_prekey_signature); g_free(signed_prekey_signature);
free(id); free(id);
} }

View File

@@ -395,6 +395,10 @@ _presence_error_handler(xmpp_stanza_t* const stanza)
} }
auto_jid Jid* fulljid = jid_create(from); auto_jid Jid* fulljid = jid_create(from);
if (fulljid == NULL) {
log_warning("MUC error presence received with missing or invalid from attribute");
return;
}
log_info("Error joining room: %s, reason: %s", fulljid->barejid, error_cond); log_info("Error joining room: %s, reason: %s", fulljid->barejid, error_cond);
if (muc_active(fulljid->barejid)) { if (muc_active(fulljid->barejid)) {
muc_leave(fulljid->barejid); muc_leave(fulljid->barejid);
@@ -451,6 +455,9 @@ _unsubscribed_handler(xmpp_stanza_t* const stanza)
log_debug("Unsubscribed presence handler fired for %s", from); log_debug("Unsubscribed presence handler fired for %s", from);
auto_jid Jid* from_jid = jid_create(from); auto_jid Jid* from_jid = jid_create(from);
if (from_jid == NULL) {
return;
}
sv_ev_subscription(from_jid->barejid, PRESENCE_UNSUBSCRIBED); sv_ev_subscription(from_jid->barejid, PRESENCE_UNSUBSCRIBED);
autocomplete_remove(sub_requests_ac, from_jid->barejid); autocomplete_remove(sub_requests_ac, from_jid->barejid);
} }
@@ -466,6 +473,9 @@ _subscribed_handler(xmpp_stanza_t* const stanza)
log_debug("Subscribed presence handler fired for %s", from); log_debug("Subscribed presence handler fired for %s", from);
auto_jid Jid* from_jid = jid_create(from); auto_jid Jid* from_jid = jid_create(from);
if (from_jid == NULL) {
return;
}
sv_ev_subscription(from_jid->barejid, PRESENCE_SUBSCRIBED); sv_ev_subscription(from_jid->barejid, PRESENCE_SUBSCRIBED);
autocomplete_remove(sub_requests_ac, from_jid->barejid); autocomplete_remove(sub_requests_ac, from_jid->barejid);
} }
@@ -476,6 +486,7 @@ _subscribe_handler(xmpp_stanza_t* const stanza)
const char* from = xmpp_stanza_get_from(stanza); const char* from = xmpp_stanza_get_from(stanza);
if (!from) { if (!from) {
log_warning("Subscribe presence handler received with no from attribute"); log_warning("Subscribe presence handler received with no from attribute");
return;
} }
log_debug("Subscribe presence handler fired for %s", from); log_debug("Subscribe presence handler fired for %s", from);
@@ -728,6 +739,9 @@ _muc_user_self_handler(xmpp_stanza_t* stanza)
{ {
const char* from = xmpp_stanza_get_from(stanza); const char* from = xmpp_stanza_get_from(stanza);
auto_jid Jid* from_jid = jid_create(from); auto_jid Jid* from_jid = jid_create(from);
if (from_jid == NULL) { // caller validates, but don't rely on it
return;
}
log_debug("Room self presence received from %s", from_jid->fulljid); log_debug("Room self presence received from %s", from_jid->fulljid);
@@ -810,6 +824,9 @@ _muc_user_occupant_handler(xmpp_stanza_t* stanza)
{ {
const char* from = xmpp_stanza_get_from(stanza); const char* from = xmpp_stanza_get_from(stanza);
auto_jid Jid* from_jid = jid_create(from); auto_jid Jid* from_jid = jid_create(from);
if (from_jid == NULL) { // caller validates, but don't rely on it
return;
}
log_debug("Room presence received from %s", from_jid->fulljid); log_debug("Room presence received from %s", from_jid->fulljid);

View File

@@ -173,6 +173,7 @@ main(int argc, char* argv[])
PROF_FUNC_TEST(disco_info_without_name), PROF_FUNC_TEST(disco_info_without_name),
PROF_FUNC_TEST(disco_items_without_name), PROF_FUNC_TEST(disco_items_without_name),
PROF_FUNC_TEST(disco_info_service_unavailable), PROF_FUNC_TEST(disco_info_service_unavailable),
PROF_FUNC_TEST(disco_info_result_no_from),
/* Roster management - add/remove/rename contacts */ /* Roster management - add/remove/rename contacts */
PROF_FUNC_TEST(sends_new_item), PROF_FUNC_TEST(sends_new_item),
@@ -237,6 +238,8 @@ main(int argc, char* argv[])
PROF_FUNC_TEST(presence_keeps_status), PROF_FUNC_TEST(presence_keeps_status),
PROF_FUNC_TEST(presence_received), PROF_FUNC_TEST(presence_received),
PROF_FUNC_TEST(presence_missing_resource_defaults), PROF_FUNC_TEST(presence_missing_resource_defaults),
PROF_FUNC_TEST(presence_error_invalid_from_no_crash),
PROF_FUNC_TEST(presence_subscription_invalid_from_no_crash),
/* Disconnect - clean session termination */ /* Disconnect - clean session termination */
PROF_FUNC_TEST(disconnect_ends_session), PROF_FUNC_TEST(disconnect_ends_session),
@@ -260,6 +263,7 @@ main(int argc, char* argv[])
PROF_FUNC_TEST(message_send), PROF_FUNC_TEST(message_send),
PROF_FUNC_TEST(message_receive_console), PROF_FUNC_TEST(message_receive_console),
PROF_FUNC_TEST(message_receive_chatwin), PROF_FUNC_TEST(message_receive_chatwin),
PROF_FUNC_TEST(message_invalid_from_silence_no_crash),
/* XEP-0359 disco gate for stanza-id trust */ /* XEP-0359 disco gate for stanza-id trust */
PROF_FUNC_TEST(stanza_id_dedup_fires_when_server_announces_sid0), PROF_FUNC_TEST(stanza_id_dedup_fires_when_server_announces_sid0),
@@ -319,6 +323,8 @@ main(int argc, char* argv[])
PROF_FUNC_TEST(receive_carbon), PROF_FUNC_TEST(receive_carbon),
PROF_FUNC_TEST(receive_self_carbon), PROF_FUNC_TEST(receive_self_carbon),
PROF_FUNC_TEST(receive_private_carbon), PROF_FUNC_TEST(receive_private_carbon),
PROF_FUNC_TEST(carbon_missing_forwarded_from_no_crash),
PROF_FUNC_TEST(carbon_invalid_to_no_crash),
#ifdef HAVE_SQLITE #ifdef HAVE_SQLITE
/* Migration stress — pool accumulation, LMC chains, large bodies */ /* Migration stress — pool accumulation, LMC chains, large bodies */

View File

@@ -144,3 +144,88 @@ receive_private_carbon(void **state)
assert_true(prof_output_regex("Buddy1/mobile: .+Private carbon")); assert_true(prof_output_regex("Buddy1/mobile: .+Private carbon"));
} }
/* Regression tests for issue #148 (REQ-INP-01) on the carbon path. The
* forwarded message is server-controlled: a missing 'from' reached
* xmpp_jid_bare(), which dereferences the jid it is handed, and an invalid
* 'to' left a NULL jid_create() result that the carbon dispatch
* dereferenced. Both terminated the client. */
static void
_carbons_on_with_buddy1(void)
{
prof_input("/carbons on");
prof_connect();
assert_true(stbbr_received(
"<iq id='*' type='set'><enable xmlns='urn:xmpp:carbons:2'/></iq>"
));
stbbr_send(
"<presence to='stabber@localhost' from='buddy1@localhost/mobile'>"
"<priority>10</priority>"
"<status>On my mobile</status>"
"</presence>"
);
assert_true(prof_output_exact("Buddy1 (mobile) is online, \"On my mobile\""));
prof_input("/msg Buddy1");
assert_true(prof_output_exact("unencrypted"));
}
/* a good carbon still arrives, so the client survived and carbons still work */
static void
_assert_carbons_still_work(void)
{
stbbr_send(
"<message type='chat' to='stabber@localhost/profanity' from='stabber@localhost'>"
"<sent xmlns='urn:xmpp:carbons:2'>"
"<forwarded xmlns='urn:xmpp:forward:0'>"
"<message id='60' xmlns='jabber:client' type='chat' to='buddy1@localhost/mobile' from='stabber@localhost/profanity'>"
"<body>still alive</body>"
"</message>"
"</forwarded>"
"</sent>"
"</message>"
);
assert_true(prof_output_regex("me: .+still alive"));
}
void
carbon_missing_forwarded_from_no_crash(void **state)
{
_carbons_on_with_buddy1();
stbbr_send(
"<message type='chat' to='stabber@localhost/profanity' from='stabber@localhost'>"
"<sent xmlns='urn:xmpp:carbons:2'>"
"<forwarded xmlns='urn:xmpp:forward:0'>"
"<message id='58' xmlns='jabber:client' type='chat' to='buddy1@localhost/mobile'>"
"<body>forwarded without a from</body>"
"</message>"
"</forwarded>"
"</sent>"
"</message>"
);
_assert_carbons_still_work();
}
void
carbon_invalid_to_no_crash(void **state)
{
_carbons_on_with_buddy1();
stbbr_send(
"<message type='chat' to='stabber@localhost/profanity' from='stabber@localhost'>"
"<sent xmlns='urn:xmpp:carbons:2'>"
"<forwarded xmlns='urn:xmpp:forward:0'>"
"<message id='59' xmlns='jabber:client' type='chat' to='bad@@jid' from='stabber@localhost/profanity'>"
"<body>forwarded to an invalid jid</body>"
"</message>"
"</forwarded>"
"</sent>"
"</message>"
);
_assert_carbons_still_work();
}

View File

@@ -4,3 +4,5 @@ void send_disable_carbons(void **state);
void receive_carbon(void **state); void receive_carbon(void **state);
void receive_self_carbon(void **state); void receive_self_carbon(void **state);
void receive_private_carbon(void **state); void receive_private_carbon(void **state);
void carbon_missing_forwarded_from_no_crash(void **state);
void carbon_invalid_to_no_crash(void **state);

View File

@@ -396,6 +396,35 @@ disco_items_without_name(void **state)
prof_timeout_reset(); prof_timeout_reset();
} }
void
disco_info_result_no_from(void **state)
{
/*
* Test that a disco#info result without a 'from' attribute is treated as
* coming from the server itself (RFC 6120 §8.1.2.1). The on-connect
* disco#info handler used to crash on such responses (issue #168).
*/
stbbr_for_query("http://jabber.org/protocol/disco#info",
"<iq to='stabber@localhost/profanity' type='result'>"
"<query xmlns='http://jabber.org/protocol/disco#info'>"
"<identity category='server' type='im' name='NoFromServer'/>"
"<feature var='urn:xmpp:ping'/>"
"</query>"
"</iq>"
);
/* the on-connect disco#info gets the same from-less response */
prof_connect();
prof_input("/disco info");
prof_timeout(10);
/* client survived and attributed the response to the server */
assert_true(prof_output_exact("Service discovery info for localhost"));
assert_true(prof_output_regex("NoFromServer.*im.*server"));
prof_timeout_reset();
}
void void
disco_info_service_unavailable(void **state) disco_info_service_unavailable(void **state)
{ {

View File

@@ -17,3 +17,4 @@ void disco_info_multiple_identities(void **state);
void disco_info_without_name(void **state); void disco_info_without_name(void **state);
void disco_items_without_name(void **state); void disco_items_without_name(void **state);
void disco_info_service_unavailable(void **state); void disco_info_service_unavailable(void **state);
void disco_info_result_no_from(void **state);

View File

@@ -123,3 +123,29 @@ stanza_id_not_trusted_when_server_does_not_announce_sid0(void **state)
assert_false(prof_output_exact("Got a message with duplicate (server-generated) stanza-id")); assert_false(prof_output_exact("Got a message with duplicate (server-generated) stanza-id"));
prof_timeout_reset(); prof_timeout_reset();
} }
/* Regression test for issue #148 (REQ-INP-01): with silence.non-roster
* enabled the incoming-message filter dereferenced jid_create() without a
* NULL check, so a message with an invalid 'from' crashed the client. */
void
message_invalid_from_silence_no_crash(void **state)
{
prof_connect();
prof_input("/silence on");
assert_true(prof_output_exact("Block all messages from JIDs that are not in the roster enabled."));
stbbr_send(
"<message to='stabber@localhost' from='bad@@jid' type='chat'>"
"<body>should be dropped, not crash</body>"
"</message>"
);
/* client is still alive: a roster contact's message comes through */
stbbr_send(
"<message to='stabber@localhost' from='buddy1@localhost/mobile' type='chat'>"
"<body>still alive</body>"
"</message>"
);
assert_true(prof_output_exact("<< chat message: Buddy1/mobile (win 2)"));
}

View File

@@ -3,3 +3,4 @@ void message_receive_console(void **state);
void message_receive_chatwin(void **state); void message_receive_chatwin(void **state);
void stanza_id_dedup_fires_when_server_announces_sid0(void **state); void stanza_id_dedup_fires_when_server_announces_sid0(void **state);
void stanza_id_not_trusted_when_server_does_not_announce_sid0(void **state); void stanza_id_not_trusted_when_server_does_not_announce_sid0(void **state);
void message_invalid_from_silence_no_crash(void **state);

View File

@@ -289,3 +289,53 @@ presence_missing_resource_defaults(void **state)
assert_true(prof_output_exact("__prof_default (15), online")); assert_true(prof_output_exact("__prof_default (15), online"));
} }
/* Regression tests for issue #148 (REQ-INP-01): presence handlers used to
* dereference jid_create() results without a NULL check, so a stanza with a
* missing or invalid 'from' crashed the client. */
void
presence_error_invalid_from_no_crash(void **state)
{
prof_connect();
/* MUC join error with an invalid 'from' */
stbbr_send(
"<presence to='stabber@localhost' from='bad@@room' type='error'>"
"<x xmlns='http://jabber.org/protocol/muc'/>"
"<error type='cancel'><not-allowed xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'/></error>"
"</presence>"
);
/* same, with no 'from' at all */
stbbr_send(
"<presence to='stabber@localhost' type='error'>"
"<x xmlns='http://jabber.org/protocol/muc'/>"
"<error type='cancel'><not-allowed xmlns='urn:ietf:params:xml:ns:xmpp-stanzas'/></error>"
"</presence>"
);
/* client is still alive and processing stanzas */
stbbr_send(
"<presence to='stabber@localhost' from='buddy1@localhost/mobile'>"
"<status>still alive</status>"
"</presence>"
);
assert_true(prof_output_exact("Buddy1 (mobile) is online, \"still alive\""));
}
void
presence_subscription_invalid_from_no_crash(void **state)
{
prof_connect();
stbbr_send("<presence to='stabber@localhost' from='bad@@jid' type='subscribed'/>");
stbbr_send("<presence to='stabber@localhost' from='bad@@jid' type='unsubscribed'/>");
stbbr_send("<presence to='stabber@localhost' from='bad@@jid' type='subscribe'/>");
stbbr_send(
"<presence to='stabber@localhost' from='buddy1@localhost/mobile'>"
"<status>still alive</status>"
"</presence>"
);
assert_true(prof_output_exact("Buddy1 (mobile) is online, \"still alive\""));
}

View File

@@ -13,3 +13,5 @@ void presence_includes_priority(void **state);
void presence_keeps_status(void **state); void presence_keeps_status(void **state);
void presence_received(void **state); void presence_received(void **state);
void presence_missing_resource_defaults(void **state); void presence_missing_resource_defaults(void **state);
void presence_error_invalid_from_no_crash(void **state);
void presence_subscription_invalid_from_no_crash(void **state);

View File

@@ -1384,3 +1384,43 @@ str_xml_sanitize__strips_illegal_characters(void** state)
assert_string_equal("UTF-8: üñîçøðé and more", res5); assert_string_equal("UTF-8: üñîçøðé and more", res5);
g_free(res5); g_free(res5);
} }
void
neutralize_untrusted_keeps_plain_text(void** state)
{
gchar* result = str_neutralize_untrusted("hello world\ttabbed\nnewline");
assert_string_equal("hello world\ttabbed\nnewline", result);
g_free(result);
}
void
neutralize_untrusted_replaces_escape_sequence(void** state)
{
gchar* result = str_neutralize_untrusted("safe\x1b[31mred");
assert_string_equal("safe\xef\xbf\xbd[31mred", result);
g_free(result);
}
void
neutralize_untrusted_replaces_bidi_override(void** state)
{
// U+202E RIGHT-TO-LEFT OVERRIDE
gchar* result = str_neutralize_untrusted("file\xe2\x80\xaegnp.exe");
assert_string_equal("file\xef\xbf\xbdgnp.exe", result);
g_free(result);
}
void
neutralize_untrusted_keeps_rtl_marks(void** state)
{
// U+200F RIGHT-TO-LEFT MARK is legitimate in RTL text
gchar* result = str_neutralize_untrusted("\xe2\x80\x8fשלום");
assert_string_equal("\xe2\x80\x8fשלום", result);
g_free(result);
}
void
neutralize_untrusted_handles_null(void** state)
{
assert_null(str_neutralize_untrusted(NULL));
}

View File

@@ -65,4 +65,10 @@ void get_mentions__tests__various(void** state);
void release_is_new__tests__various(void** state); void release_is_new__tests__various(void** state);
void str_xml_sanitize__strips_illegal_characters(void** state); void str_xml_sanitize__strips_illegal_characters(void** state);
void neutralize_untrusted_keeps_plain_text(void** state);
void neutralize_untrusted_replaces_escape_sequence(void** state);
void neutralize_untrusted_replaces_bidi_override(void** state);
void neutralize_untrusted_keeps_rtl_marks(void** state);
void neutralize_untrusted_handles_null(void** state);
#endif #endif

View File

@@ -183,6 +183,11 @@ main(int argc, char* argv[])
cmocka_unit_test(jid_is_valid_user_jid__is__true_for_at_in_resource), cmocka_unit_test(jid_is_valid_user_jid__is__true_for_at_in_resource),
cmocka_unit_test(jid_is_valid_user_jid__is__false_for_domain_jid), cmocka_unit_test(jid_is_valid_user_jid__is__false_for_domain_jid),
cmocka_unit_test(jid_is_valid_user_jid__is__false_for_invalid_jid), cmocka_unit_test(jid_is_valid_user_jid__is__false_for_invalid_jid),
cmocka_unit_test(jid_is_valid__boundary__localpart_length),
cmocka_unit_test(jid_is_valid__boundary__domainpart_length),
cmocka_unit_test(jid_is_valid__boundary__resourcepart_length),
cmocka_unit_test(jid_is_valid__boundary__total_length),
cmocka_unit_test(jid_is_valid__is__false_for_invalid_utf8),
cmocka_unit_test(jid_is_valid__is__true_for_valid_jid), cmocka_unit_test(jid_is_valid__is__true_for_valid_jid),
cmocka_unit_test(jid_is_valid__is__false_for_invalid_jid), cmocka_unit_test(jid_is_valid__is__false_for_invalid_jid),
cmocka_unit_test(jid_is_valid__is__false_for_null), cmocka_unit_test(jid_is_valid__is__false_for_null),
@@ -687,6 +692,11 @@ main(int argc, char* argv[])
cmocka_unit_test(get_mentions__tests__various), cmocka_unit_test(get_mentions__tests__various),
cmocka_unit_test(release_is_new__tests__various), cmocka_unit_test(release_is_new__tests__various),
cmocka_unit_test(str_xml_sanitize__strips_illegal_characters), cmocka_unit_test(str_xml_sanitize__strips_illegal_characters),
cmocka_unit_test(neutralize_untrusted_keeps_plain_text),
cmocka_unit_test(neutralize_untrusted_replaces_escape_sequence),
cmocka_unit_test(neutralize_untrusted_replaces_bidi_override),
cmocka_unit_test(neutralize_untrusted_keeps_rtl_marks),
cmocka_unit_test(neutralize_untrusted_handles_null),
cmocka_unit_test_setup_teardown(plugins_get_command_names__returns__no_commands, cmocka_unit_test_setup_teardown(plugins_get_command_names__returns__no_commands,
load_preferences, load_preferences,

View File

@@ -370,3 +370,75 @@ jid_is_valid__is__false_for_empty_string(void** state)
{ {
assert_false(jid_is_valid("")); assert_false(jid_is_valid(""));
} }
/* RFC 6122 size limits: 1023 bytes per part, 3071 for the full JID */
void
jid_is_valid__boundary__localpart_length(void** state)
{
gchar* local_ok = g_strnfill(1023, 'a');
gchar* jid_ok = g_strdup_printf("%s@domain", local_ok);
assert_true(jid_is_valid(jid_ok));
gchar* local_over = g_strnfill(1024, 'a');
gchar* jid_over = g_strdup_printf("%s@domain", local_over);
assert_false(jid_is_valid(jid_over));
g_free(local_ok);
g_free(jid_ok);
g_free(local_over);
g_free(jid_over);
}
void
jid_is_valid__boundary__domainpart_length(void** state)
{
gchar* domain_ok = g_strnfill(1023, 'd');
assert_true(jid_is_valid(domain_ok));
gchar* domain_over = g_strnfill(1024, 'd');
assert_false(jid_is_valid(domain_over));
g_free(domain_ok);
g_free(domain_over);
}
void
jid_is_valid__boundary__resourcepart_length(void** state)
{
gchar* res_ok = g_strnfill(1023, 'r');
gchar* jid_ok = g_strdup_printf("user@domain/%s", res_ok);
assert_true(jid_is_valid(jid_ok));
gchar* res_over = g_strnfill(1024, 'r');
gchar* jid_over = g_strdup_printf("user@domain/%s", res_over);
assert_false(jid_is_valid(jid_over));
g_free(res_ok);
g_free(jid_ok);
g_free(res_over);
g_free(jid_over);
}
void
jid_is_valid__boundary__total_length(void** state)
{
// 1023 + '@' + 1023 + '/' + 1023 = 3071, the largest legal JID
gchar* local = g_strnfill(1023, 'a');
gchar* domain = g_strnfill(1023, 'd');
gchar* res = g_strnfill(1023, 'r');
gchar* jid_max = g_strdup_printf("%s@%s/%s", local, domain, res);
assert_true(jid_is_valid(jid_max));
g_free(local);
g_free(domain);
g_free(res);
g_free(jid_max);
}
void
jid_is_valid__is__false_for_invalid_utf8(void** state)
{
assert_false(jid_is_valid("user\xff\xfe@domain"));
assert_false(jid_is_valid("us\xc3@domain")); // truncated multi-byte sequence
assert_false(jid_is_valid("user@domain/res\x80")); // stray continuation byte
}

View File

@@ -40,4 +40,10 @@ void jid_is_valid__is__false_for_invalid_jid(void** state);
void jid_is_valid__is__false_for_null(void** state); void jid_is_valid__is__false_for_null(void** state);
void jid_is_valid__is__false_for_empty_string(void** state); void jid_is_valid__is__false_for_empty_string(void** state);
void jid_is_valid__boundary__localpart_length(void** state);
void jid_is_valid__boundary__domainpart_length(void** state);
void jid_is_valid__boundary__resourcepart_length(void** state);
void jid_is_valid__boundary__total_length(void** state);
void jid_is_valid__is__false_for_invalid_utf8(void** state);
#endif #endif