Compare commits

..

2 Commits

Author SHA1 Message Date
c9455d27ba fix(xmpp): treat disco#info result without 'from' as from the server
Some checks failed
CI Code / Check spelling (pull_request) Successful in 14s
CI Code / Check coding style (pull_request) Successful in 22s
CI Code / Code Coverage (pull_request) Successful in 3m19s
CI Code / Linux (arch) (pull_request) Failing after 3m35s
CI Code / Linux (debian) (pull_request) Successful in 5m1s
CI Code / Linux (ubuntu) (pull_request) Successful in 5m4s
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-26 11:49:51 +00:00
964f73d0ad fix(editor): follow live terminal size in external editor
All checks were successful
CI Code / Check spelling (pull_request) Successful in 14s
CI Code / Check coding style (pull_request) Successful in 22s
CI Code / Linux (debian) (pull_request) Successful in 4m24s
CI Code / Code Coverage (pull_request) Successful in 3m8s
CI Code / Linux (ubuntu) (pull_request) Successful in 8m41s
CI Code / Linux (arch) (pull_request) Successful in 13m2s
CI Code / Check spelling (push) Successful in 15s
CI Code / Check coding style (push) Successful in 23s
CI Code / Code Coverage (push) Successful in 3m13s
Publish Docker image / Push Docker image to Docker Hub (push) Successful in 3m35s
CI Code / Linux (debian) (push) Successful in 5m0s
CI Code / Linux (ubuntu) (push) Successful in 5m5s
CI Code / Linux (arch) (push) Successful in 6m30s
The compose editor is spawned via fork+execvp and inherits profanity's
LINES/COLUMNS, which hold the size captured at startup and are never
refreshed on resize. A curses editor (nano, vim, ...) honors them over
the real window, so it renders at the launch-time size after a resize.

Drop LINES/COLUMNS from the child's environment before forking so its
curses falls back to ioctl(TIOCGWINSZ). Assembling the env in the parent
lets the child only reassign environ instead of calling unsetenv()
between fork and exec, keeping it clear of unsetenv()'s allocator work in
the multithreaded fork->exec window. profanity itself is unaffected.
2026-07-25 15:19:22 +00:00
7 changed files with 67 additions and 17 deletions

View File

@@ -25,6 +25,8 @@
#include "ui/ui.h" #include "ui/ui.h"
#include "xmpp/xmpp.h" #include "xmpp/xmpp.h"
extern char** environ;
typedef struct EditorContext typedef struct EditorContext
{ {
gchar* filename; gchar* filename;
@@ -140,6 +142,22 @@ launch_editor(gchar* initial_content, void (*callback)(gchar* content, void* dat
GSource* sigchld_warmup = g_child_watch_source_new(getpid()); GSource* sigchld_warmup = g_child_watch_source_new(getpid());
g_source_unref(sigchld_warmup); 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(); pid_t pid = fork();
if (pid == -1) { if (pid == -1) {
log_error("[Editor] Failed to fork: %s", strerror(errno)); log_error("[Editor] Failed to fork: %s", strerror(errno));
@@ -148,12 +166,14 @@ launch_editor(gchar* initial_content, void (*callback)(gchar* content, void* dat
ui_resize(); ui_resize();
cons_show_error("Failed to start editor: %s", strerror(errno)); cons_show_error("Failed to start editor: %s", strerror(errno));
g_strfreev(editor_argv); g_strfreev(editor_argv);
g_free(ctx->filename); g_free(editor_env);
g_free(ctx); g_free(ctx);
return TRUE; return TRUE;
} else if (pid == 0) { } else if (pid == 0) {
// Child process: Inherits TTY from parent // 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 // SIGTSTP=SIG_DFL lets vim's :stop / Ctrl-Z work; profanity catches
// the STOPPED state via editor_check_stopped() and drops to the shell. // the STOPPED state via editor_check_stopped() and drops to the shell.
signal(SIGINT, SIG_DFL); signal(SIGINT, SIG_DFL);
@@ -170,6 +190,7 @@ launch_editor(gchar* initial_content, void (*callback)(gchar* content, void* dat
editor_pid = pid; editor_pid = pid;
g_child_watch_add((GPid)pid, _editor_exit_cb, ctx); g_child_watch_add((GPid)pid, _editor_exit_cb, ctx);
g_strfreev(editor_argv); g_strfreev(editor_argv);
g_free(editor_env); // array only; strings are borrowed from environ
return FALSE; return FALSE;
} }

View File

@@ -658,15 +658,6 @@ connection_request_features(void)
/* We don't record it as a requested feature to avoid triggering th /* We don't record it as a requested feature to avoid triggering th
* sv_ev_connection_features_received too soon */ * sv_ev_connection_features_received too soon */
iq_disco_info_request_onconnect(conn.domain); iq_disco_info_request_onconnect(conn.domain);
const char* barejid = connection_get_barejid();
if (barejid && g_strcmp0(barejid, conn.domain) != 0) {
if (!g_hash_table_contains(conn.features_by_jid, barejid)) {
g_hash_table_insert(conn.features_by_jid, strdup(barejid),
g_hash_table_new_full(g_str_hash, g_str_equal, free, NULL));
}
iq_disco_info_request_onconnect(barejid); // XEP-0163: PEP services announce features on the account's bare JID
}
} }
void void
@@ -762,7 +753,11 @@ void
connection_features_received(const char* const jid) connection_features_received(const char* const jid)
{ {
log_info("[CONNECTION] connection_features_received %s", 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(); sv_ev_connection_features_received();
} }
} }
@@ -770,7 +765,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 = jid ? jid : conn.domain;
if (!key || !conn.features_by_jid) {
return NULL;
}
return g_hash_table_lookup(conn.features_by_jid, key);
} }
GList* 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); 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

@@ -446,21 +446,18 @@ _omemo_receive_devicelist(xmpp_stanza_t* const stanza, void* const userdata)
GList* device_list = NULL; GList* device_list = NULL;
if (g_strcmp0(type, STANZA_TYPE_ERROR) == 0) { if (g_strcmp0(type, STANZA_TYPE_ERROR) == 0) {
log_error("[OMEMO] can't get OMEMO device list");
xmpp_stanza_t* error = xmpp_stanza_get_child_by_name(stanza, "error"); xmpp_stanza_t* error = xmpp_stanza_get_child_by_name(stanza, "error");
if (!error) { if (!error) {
log_error("[OMEMO] missing error element in device list response"); log_error("[OMEMO] missing error element in device list response");
return 1; return 1;
} }
// a missing node is signalled via legacy code='404' or the RFC 6120 <item-not-found/> condition
const char* code = xmpp_stanza_get_attribute(error, "code"); const char* code = xmpp_stanza_get_attribute(error, "code");
if (g_strcmp0(code, "404") == 0 if (g_strcmp0(code, "404") == 0) {
|| xmpp_stanza_get_child_by_name_and_ns(error, STANZA_NAME_ITEM_NOT_FOUND, STANZA_NS_STANZAS)) {
log_debug("[OMEMO] no devicelist node for %s, bootstrapping an empty one", from);
omemo_set_device_list(from, NULL); omemo_set_device_list(from, NULL);
return 1; return 1;
} }
log_error("[OMEMO] can't get OMEMO device list");
} }
xmpp_stanza_t* root = NULL; xmpp_stanza_t* root = NULL;

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),

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);