Compare commits

...

1 Commits

Author SHA1 Message Date
250703a0bf security: harden untrusted-input handling (issue #148)
All checks were successful
CI Code / Check spelling (pull_request) Successful in 14s
CI Code / Check coding style (pull_request) Successful in 26s
CI Code / Code Coverage (pull_request) Successful in 3m29s
CI Code / Linux (debian) (pull_request) Successful in 5m13s
CI Code / Linux (ubuntu) (pull_request) Successful in 5m16s
CI Code / Linux (arch) (pull_request) Successful in 7m34s
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
(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
(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)
2026-08-01 13:39:19 +03:00
30 changed files with 415 additions and 78 deletions

View File

@@ -405,6 +405,8 @@ AM_CFLAGS="$AM_CFLAGS -Wimplicit-function-declaration"
AM_CFLAGS="$AM_CFLAGS -Wundef"
AM_CFLAGS="$AM_CFLAGS -Wfloat-equal -Wredundant-decls"
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"
# GCC-specific warnings (not supported by clang) — test each one

View File

@@ -58,6 +58,8 @@ cc = meson.get_compiler('c')
add_project_arguments([
'-Wno-deprecated-declarations',
'-Wno-unused-parameter',
# stack arrays sized from peer input are a remote stack-exhaustion vector
'-Werror=vla',
], language: 'c')
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);
char parsed[len + 1];
auto_char char* parsed = malloc(len + 1);
if (!parsed) {
return NULL;
}
size_t i = 0;
while (i < len) {
if (input[i] == ' ') {

View File

@@ -9644,6 +9644,13 @@ cmd_url_open(ProfWin* window, const char* const command, gchar** args)
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);
if (cmd_template == NULL) {
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;
}
// 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);
if (cmd_template == NULL && (g_strcmp0(scheme, "http") == 0 || g_strcmp0(scheme, "https") == 0)) {
_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);
}
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*
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);
int utf8_display_len(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 valid_tls_policy_option(const char* is);

View File

@@ -1303,4 +1303,13 @@ _clean_incoming_message(ProfMessage* message)
{
_cut(message, "\u200E");
_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) {
free(message->plain);
message->plain = strdup(clean); // message_free() frees this with free()
}
}
}

View File

@@ -104,7 +104,7 @@ omemo_hmac_sha256_final_func(void* hmac_context, signal_buffer** output, void* u
{
gcry_error_t res;
size_t mac_len = 32;
unsigned char out[mac_len];
unsigned char out[32];
res = gcry_mac_read(hmac_context, out, &mac_len);
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 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++) {
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++) {
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;

View File

@@ -452,8 +452,12 @@ omemo_identity_key(unsigned char** output, size_t* length)
signal_buffer* buffer = NULL;
ec_public_key_serialize(&buffer, ratchet_identity_key_pair_get_public(omemo_ctx.identity_key_pair));
*length = signal_buffer_len(buffer);
*output = malloc(*length);
memcpy(*output, signal_buffer_data(buffer), *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);
}
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)));
SIGNAL_UNREF(signed_pre_key);
*length = signal_buffer_len(buffer);
*output = malloc(*length);
memcpy(*output, signal_buffer_data(buffer), *length);
*output = *length ? g_malloc(*length) : NULL;
if (*output) {
memcpy(*output, signal_buffer_data(buffer), *length);
}
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);
*output = malloc(*length);
memcpy(*output, session_signed_pre_key_get_signature(signed_pre_key), *length);
*output = *length ? g_malloc(*length) : NULL;
if (*output) {
memcpy(*output, session_signed_pre_key_get_signature(signed_pre_key), *length);
}
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_keylist_mode(ctx, GPGME_KEYLIST_MODE_LOCAL);
char* xmpp_jid_me = alloca((strlen(sender_barejid) + 6) * sizeof(char));
char* xmpp_jid_recipient = alloca((strlen(recipient_barejid) + 6) * sizeof(char));
strcpy(xmpp_jid_me, "xmpp:");
strcpy(xmpp_jid_recipient, "xmpp:");
strcat(xmpp_jid_me, sender_barejid);
strcat(xmpp_jid_recipient, recipient_barejid);
auto_gchar gchar* xmpp_jid_me = g_strdup_printf("xmpp:%s", sender_barejid);
auto_gchar gchar* xmpp_jid_recipient = g_strdup_printf("xmpp:%s", recipient_barejid);
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)) {
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;
for (i = 0; i < len; i++) {
PyObject* item = PyList_GetItem(synopsis, i);
@@ -132,11 +132,23 @@ python_api_register_command(PyObject* self, PyObject* args)
c_synopsis[len] = NULL;
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++) {
PyObject* item = PyList_GetItem(arguments, i);
Py_ssize_t len2 = PyList_Size(item);
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;
}
PyObject* arg = PyList_GetItem(item, 0);
@@ -152,7 +164,7 @@ python_api_register_command(PyObject* self, PyObject* args)
c_arguments[args_len][1] = NULL;
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++) {
PyObject* item = PyList_GetItem(examples, i);
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) {
free(c_examples[i++]);
}
g_free(c_synopsis);
g_free(c_arguments);
g_free(c_examples);
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);
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;
for (i = 0; i < len; i++) {
@@ -244,6 +259,7 @@ python_api_completer_add(PyObject* self, PyObject* args)
while (c_items[i] != NULL) {
free(c_items[i++]);
}
g_free(c_items);
disable_python_threads();
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);
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;
for (i = 0; i < len; i++) {
@@ -280,6 +296,11 @@ python_api_completer_remove(PyObject* self, PyObject* args)
allow_python_threads();
api_completer_remove(plugin_name, key_str, c_items);
free(key_str);
i = 0;
while (c_items[i] != NULL) {
free(c_items[i++]);
}
g_free(c_items);
disable_python_threads();
free(plugin_name);

View File

@@ -493,7 +493,7 @@ _inp_edited(const wint_t ch)
}
// 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);
if (utf_len == (size_t)-1) {
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);
} else {
if (first_trigger_pos > 0) {
char message_section[strlen(message) + 1];
int i = 0;
while (i < first_trigger_pos) {
message_section[i] = message[i];
i++;
}
message_section[i] = '\0';
auto_gchar gchar* message_section = g_strndup(message, (gsize)first_trigger_pos);
win_append_highlight(window, THEME_ROOMTRIGGER, "%s", message_section);
}
char trigger_section[first_trigger_len + 1];
int i = 0;
while (i < first_trigger_len) {
trigger_section[i] = message[first_trigger_pos + i];
i++;
}
trigger_section[i] = '\0';
auto_gchar gchar* trigger_section = g_strndup(&message[first_trigger_pos], (gsize)first_trigger_len);
if (first_trigger_pos + first_trigger_len < (int)strlen(message)) {
win_append_highlight(window, THEME_ROOMTRIGGER_TERM, "%s", trigger_section);

View File

@@ -22,6 +22,7 @@
#include <windows.h>
#endif
#include "common.h"
#include "log.h"
#include "config/preferences.h"
#include "ui/ui.h"
@@ -122,47 +123,42 @@ _notify(const char* const message, int timeout, const char* const category)
static void
_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, "'", "'\\''");
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 if (escaped_single[0] == '{') {
g_string_append(notify_command, "\\{");
g_string_append(notify_command, &escaped_single[1]);
// terminal-notifier swallows a leading <, [, ( or { — escape it for its parser
auto_gchar gchar* msg = NULL;
if (message[0] == '<' || message[0] == '[' || message[0] == '(' || message[0] == '{') {
msg = g_strdup_printf("\\%s", message);
} 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");
char* app_id = NULL;
const char* term_name = getenv("TERM_PROGRAM");
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) {
app_id = "com.googlecode.iterm2";
argv[i++] = "-sender";
argv[i++] = "com.googlecode.iterm2";
}
argv[i] = NULL;
if (app_id) {
g_string_append(notify_command, " -sender ");
g_string_append(notify_command, app_id);
GError* err = NULL;
// argv-based spawn: the message is one argument, the shell never sees it
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
static void

View File

@@ -32,6 +32,7 @@
#include <curses.h>
#endif
#include "common.h"
#include "log.h"
#include "config/theme.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);
}
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);
waddstr(win, copy);

View File

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

View File

@@ -1784,7 +1784,7 @@ _last_activity_get_handler(xmpp_stanza_t* const stanza)
if (prefs_get_boolean(PREF_LASTACTIVITY)) {
int idls_secs = (int)(ui_get_idle_time() / 1000);
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_set_to(response, from);

View File

@@ -1562,6 +1562,20 @@ _handle_mam(xmpp_stanza_t* const stanza)
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);
xmpp_stanza_t* message_stanza = xmpp_stanza_get_child_by_ns(forwarded, "jabber:client");
@@ -1658,7 +1672,7 @@ _ox_openpgp_signcrypt(xmpp_ctx_t* ctx, const char* const to, const char* const t
// build rpad
int randnr = (rand() % 100) + 1;
char rpad_data[randnr];
char rpad_data[101];
for (int i = 0; i < randnr; i++) {
int rchar = (rand() % 52) + 65;
rpad_data[i] = (char)rchar;
@@ -1747,6 +1761,10 @@ _should_ignore_based_on_silence(xmpp_stanza_t* const stanza)
if (prefs_get_boolean(PREF_SILENCE_NON_ROSTER)) {
const char* const from = xmpp_stanza_get_from(stanza);
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);
if (!contact) {
log_debug("[Silence] Ignoring message from: %s", from);

View File

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

View File

@@ -395,6 +395,10 @@ _presence_error_handler(xmpp_stanza_t* const stanza)
}
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);
if (muc_active(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);
auto_jid Jid* from_jid = jid_create(from);
if (from_jid == NULL) {
return;
}
sv_ev_subscription(from_jid->barejid, PRESENCE_UNSUBSCRIBED);
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);
auto_jid Jid* from_jid = jid_create(from);
if (from_jid == NULL) {
return;
}
sv_ev_subscription(from_jid->barejid, PRESENCE_SUBSCRIBED);
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);
if (!from) {
log_warning("Subscribe presence handler received with no from attribute");
return;
}
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);
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);
@@ -810,6 +824,9 @@ _muc_user_occupant_handler(xmpp_stanza_t* stanza)
{
const char* from = xmpp_stanza_get_from(stanza);
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);

View File

@@ -238,6 +238,8 @@ main(int argc, char* argv[])
PROF_FUNC_TEST(presence_keeps_status),
PROF_FUNC_TEST(presence_received),
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 */
PROF_FUNC_TEST(disconnect_ends_session),
@@ -261,6 +263,7 @@ main(int argc, char* argv[])
PROF_FUNC_TEST(message_send),
PROF_FUNC_TEST(message_receive_console),
PROF_FUNC_TEST(message_receive_chatwin),
PROF_FUNC_TEST(message_invalid_from_silence_no_crash),
/* XEP-0359 disco gate for stanza-id trust */
PROF_FUNC_TEST(stanza_id_dedup_fires_when_server_announces_sid0),

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"));
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 stanza_id_dedup_fires_when_server_announces_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"));
}
/* 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_received(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);
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

@@ -66,3 +66,8 @@ void release_is_new__tests__various(void** state);
void str_xml_sanitize__strips_illegal_characters(void** state);
#endif
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);

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__false_for_domain_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__false_for_invalid_jid),
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(release_is_new__tests__various),
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,
load_preferences,

View File

@@ -370,3 +370,75 @@ jid_is_valid__is__false_for_empty_string(void** state)
{
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

@@ -41,3 +41,8 @@ void jid_is_valid__is__false_for_null(void** state);
void jid_is_valid__is__false_for_empty_string(void** state);
#endif
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);