Compare commits

..

1 Commits

Author SHA1 Message Date
a283c4e7b4 fix(xmpp): treat disco#info result without 'from' as from the server
All checks were successful
CI Code / Check spelling (pull_request) Successful in 14s
CI Code / Check coding style (pull_request) Successful in 24s
CI Code / Linux (ubuntu) (pull_request) Successful in 8m29s
CI Code / Linux (debian) (pull_request) Successful in 8m53s
CI Code / Code Coverage (pull_request) Successful in 9m7s
CI Code / Linux (arch) (pull_request) Successful in 12m11s
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-21 09:09:49 +03:00
6 changed files with 44 additions and 24 deletions

View File

@@ -25,8 +25,6 @@
#include "ui/ui.h"
#include "xmpp/xmpp.h"
extern char** environ;
typedef struct EditorContext
{
gchar* filename;
@@ -142,22 +140,6 @@ launch_editor(gchar* initial_content, void (*callback)(gchar* content, void* dat
GSource* sigchld_warmup = g_child_watch_source_new(getpid());
g_source_unref(sigchld_warmup);
// Build the editor's env without LINES/COLUMNS pre-fork, so the child only
// reassigns environ instead of calling unsetenv() between fork and exec.
// The editor's (n)curses then reads the live window via ioctl(TIOCGWINSZ).
gsize env_len = 0;
while (environ[env_len]) {
env_len++;
}
gchar** editor_env = g_new0(gchar*, env_len + 1);
gsize env_kept = 0;
for (gsize i = 0; i < env_len; i++) {
if (g_str_has_prefix(environ[i], "LINES=") || g_str_has_prefix(environ[i], "COLUMNS=")) {
continue;
}
editor_env[env_kept++] = environ[i];
}
pid_t pid = fork();
if (pid == -1) {
log_error("[Editor] Failed to fork: %s", strerror(errno));
@@ -166,14 +148,12 @@ launch_editor(gchar* initial_content, void (*callback)(gchar* content, void* dat
ui_resize();
cons_show_error("Failed to start editor: %s", strerror(errno));
g_strfreev(editor_argv);
g_free(editor_env);
g_free(ctx->filename);
g_free(ctx);
return TRUE;
} else if (pid == 0) {
// Child process: Inherits TTY from parent
environ = editor_env; // live TIOCGWINSZ size, not the inherited LINES/COLUMNS
// SIGTSTP=SIG_DFL lets vim's :stop / Ctrl-Z work; profanity catches
// the STOPPED state via editor_check_stopped() and drops to the shell.
signal(SIGINT, SIG_DFL);
@@ -190,7 +170,6 @@ launch_editor(gchar* initial_content, void (*callback)(gchar* content, void* dat
editor_pid = pid;
g_child_watch_add((GPid)pid, _editor_exit_cb, ctx);
g_strfreev(editor_argv);
g_free(editor_env); // array only; strings are borrowed from environ
return FALSE;
}

View File

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

View File

@@ -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);
} else {
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
@@ -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);
} else {
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

View File

@@ -173,6 +173,7 @@ main(int argc, char* argv[])
PROF_FUNC_TEST(disco_info_without_name),
PROF_FUNC_TEST(disco_items_without_name),
PROF_FUNC_TEST(disco_info_service_unavailable),
PROF_FUNC_TEST(disco_info_result_no_from),
/* Roster management - add/remove/rename contacts */
PROF_FUNC_TEST(sends_new_item),

View File

@@ -396,6 +396,35 @@ disco_items_without_name(void **state)
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
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_items_without_name(void **state);
void disco_info_service_unavailable(void **state);
void disco_info_result_no_from(void **state);