From 7fb926c78af44ec9826564862381d3e9803f7bbe Mon Sep 17 00:00:00 2001 From: "jabber.developer2" Date: Sat, 21 Feb 2026 18:24:53 +0300 Subject: [PATCH] security: validate from and by attributes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit XEP-0203 §4 (Delayed Delivery): Remote clients could inject elements with arbitrary timestamps, causing messages to appear at incorrect positions in chat history. The spec mandates checking the 'from' attribute to verify the delay was added by a trusted source. - stanza.c: add stanza_get_oldest_delay_from(stanza, from) that filters / children by the 'from' attribute; refactor stanza_get_oldest_delay() to delegate with from=NULL. Add NULL-safety for tmp timestamp in comparison logic. - stanza.h: declare stanza_get_oldest_delay_from(). - message.c (_handle_chat): replace unfiltered stanza_get_delay() with trust-ordered fallback: our domain → sender domain → now. - message.c (_handle_groupchat): replace stanza_get_oldest_delay() with fallback: room bare JID → room domain → our domain → now. - message.c (_handle_muc_private_message): same pattern. MAM path deliberately unchanged — inside is already from our own server's archive. XEP-0359 §5 (Unique and Stable Stanza IDs): The element was accepted without checking the 'by' attribute, allowing a remote client to inject forged archive IDs. - stanza.h: add STANZA_ATTR_BY define. - message.c (_handle_groupchat): only accept when by == room bare JID. - message.c (_handle_chat): only accept when by == our server domain. --- src/xmpp/message.c | 58 +++++++++++++++++++++++++++++++++------------- src/xmpp/stanza.c | 24 +++++++++++++++---- src/xmpp/stanza.h | 2 ++ 3 files changed, 64 insertions(+), 20 deletions(-) diff --git a/src/xmpp/message.c b/src/xmpp/message.c index a8051c61..14776136 100644 --- a/src/xmpp/message.c +++ b/src/xmpp/message.c @@ -1090,12 +1090,16 @@ _handle_groupchat(xmpp_stanza_t* const stanza) message->id = strdup(id); } + // XEP-0359 §5: only accept whose 'by' matches the MUC room char* stanzaid = NULL; xmpp_stanza_t* stanzaidst = xmpp_stanza_get_child_by_name_and_ns(stanza, STANZA_NAME_STANZA_ID, STANZA_NS_STABLE_ID); if (stanzaidst) { - stanzaid = (char*)xmpp_stanza_get_attribute(stanzaidst, STANZA_ATTR_ID); - if (stanzaid) { - message->stanzaid = strdup(stanzaid); + const char* by = xmpp_stanza_get_attribute(stanzaidst, STANZA_ATTR_BY); + if (by && g_strcmp0(by, from_jid->barejid) == 0) { + stanzaid = (char*)xmpp_stanza_get_attribute(stanzaidst, STANZA_ATTR_ID); + if (stanzaid) { + message->stanzaid = strdup(stanzaid); + } } } @@ -1138,12 +1142,18 @@ _handle_groupchat(xmpp_stanza_t* const stanza) message->timestamp = NULL; } - // we want to display the oldest delay - message->timestamp = stanza_get_oldest_delay(stanza); - - // now this has nothing to do with MUC history - // it's just setting the time to the received time so upon displaying we can use this time - // for example in win_println_incoming_muc_msg() + // XEP-0203 §4: only accept stamped by the room (MUC server) + // or our own server — reject client-forged timestamps. + message->timestamp = stanza_get_oldest_delay_from(stanza, from_jid->barejid); + if (!message->timestamp && from_jid->domainpart) { + message->timestamp = stanza_get_oldest_delay_from(stanza, from_jid->domainpart); + } + if (!message->timestamp) { + const char* my_domain = connection_get_domain(); + if (my_domain) { + message->timestamp = stanza_get_oldest_delay_from(stanza, my_domain); + } + } if (!message->timestamp) { message->timestamp = g_date_time_new_now_local(); } @@ -1277,7 +1287,14 @@ _handle_muc_private_message(xmpp_stanza_t* const stanza) } #endif - message->timestamp = stanza_get_delay(stanza); + // XEP-0203 §4: only trust from a server domain. + { + const char* my_domain = connection_get_domain(); + message->timestamp = stanza_get_delay_from(stanza, (gchar*)my_domain); + if (!message->timestamp && message->from_jid->domainpart) { + message->timestamp = stanza_get_delay_from(stanza, message->from_jid->domainpart); + } + } message->body = xmpp_message_get_body(stanza); if (!message->plain && !message->body) { @@ -1401,13 +1418,17 @@ _handle_chat(xmpp_stanza_t* const stanza, gboolean is_mam, gboolean is_carbon, c log_warning("MAM received with no result id"); } } else { - // live messages use XEP-0359 + // XEP-0359 §5: only accept whose 'by' matches our server char* stanzaid = NULL; xmpp_stanza_t* stanzaidst = xmpp_stanza_get_child_by_name_and_ns(stanza, STANZA_NAME_STANZA_ID, STANZA_NS_STABLE_ID); if (stanzaidst) { - stanzaid = (char*)xmpp_stanza_get_attribute(stanzaidst, STANZA_ATTR_ID); - if (stanzaid) { - message->stanzaid = strdup(stanzaid); + const char* by = xmpp_stanza_get_attribute(stanzaidst, STANZA_ATTR_BY); + const char* my_domain = connection_get_domain(); + if (by && my_domain && g_strcmp0(by, my_domain) == 0) { + stanzaid = (char*)xmpp_stanza_get_attribute(stanzaidst, STANZA_ATTR_ID); + if (stanzaid) { + message->stanzaid = strdup(stanzaid); + } } } } @@ -1425,8 +1446,13 @@ _handle_chat(xmpp_stanza_t* const stanza, gboolean is_mam, gboolean is_carbon, c // timestamp provided outside like in a by MAM message->timestamp = timestamp; } else { - // timestamp in the message stanza or use time of receival (now) - message->timestamp = stanza_get_delay(stanza); + // XEP-0203 §4: only trust from a server domain, not from + // the remote client which could forge an arbitrary timestamp. + const char* my_domain = connection_get_domain(); + message->timestamp = stanza_get_delay_from(stanza, (gchar*)my_domain); + if (!message->timestamp && jid->domainpart) { + message->timestamp = stanza_get_delay_from(stanza, jid->domainpart); + } if (!message->timestamp) { message->timestamp = g_date_time_new_now_local(); } diff --git a/src/xmpp/stanza.c b/src/xmpp/stanza.c index 593b4ece..8a07cc75 100644 --- a/src/xmpp/stanza.c +++ b/src/xmpp/stanza.c @@ -1215,6 +1215,12 @@ stanza_get_delay_from(xmpp_stanza_t* const stanza, gchar* from) GDateTime* stanza_get_oldest_delay(xmpp_stanza_t* const stanza) +{ + return stanza_get_oldest_delay_from(stanza, NULL); +} + +GDateTime* +stanza_get_oldest_delay_from(xmpp_stanza_t* const stanza, const char* const from) { xmpp_stanza_t* child; const char* child_name; @@ -1225,27 +1231,37 @@ stanza_get_oldest_delay(xmpp_stanza_t* const stanza) child_name = xmpp_stanza_get_name(child); if (child_name && g_strcmp0(child_name, STANZA_NAME_DELAY) == 0) { + if (from) { + const char* child_from = xmpp_stanza_get_attribute(child, STANZA_ATTR_FROM); + if (!child_from || g_strcmp0(child_from, from) != 0) + continue; + } GDateTime* tmp = _stanza_get_delay_timestamp_xep0203(child); if (oldest == NULL) { oldest = tmp; - } else if (g_date_time_compare(oldest, tmp) == 1) { + } else if (tmp && g_date_time_compare(oldest, tmp) == 1) { g_date_time_unref(oldest); oldest = tmp; - } else { + } else if (tmp) { g_date_time_unref(tmp); } } if (child_name && g_strcmp0(child_name, STANZA_NAME_X) == 0) { + if (from) { + const char* child_from = xmpp_stanza_get_attribute(child, STANZA_ATTR_FROM); + if (!child_from || g_strcmp0(child_from, from) != 0) + continue; + } GDateTime* tmp = _stanza_get_delay_timestamp_xep0091(child); if (oldest == NULL) { oldest = tmp; - } else if (g_date_time_compare(oldest, tmp) == 1) { + } else if (tmp && g_date_time_compare(oldest, tmp) == 1) { g_date_time_unref(oldest); oldest = tmp; - } else { + } else if (tmp) { g_date_time_unref(tmp); } } diff --git a/src/xmpp/stanza.h b/src/xmpp/stanza.h index db3b0fea..f20a7733 100644 --- a/src/xmpp/stanza.h +++ b/src/xmpp/stanza.h @@ -196,6 +196,7 @@ #define STANZA_ATTR_FILENAME "filename" #define STANZA_ATTR_SIZE "size" #define STANZA_ATTR_CONTENTTYPE "content-type" +#define STANZA_ATTR_BY "by" #define STANZA_ATTR_LABEL "label" #define STANZA_TEXT_AWAY "away" @@ -331,6 +332,7 @@ gboolean stanza_contains_chat_state(xmpp_stanza_t* stanza); GDateTime* stanza_get_delay(xmpp_stanza_t* const stanza); GDateTime* stanza_get_delay_from(xmpp_stanza_t* const stanza, gchar* from); GDateTime* stanza_get_oldest_delay(xmpp_stanza_t* const stanza); +GDateTime* stanza_get_oldest_delay_from(xmpp_stanza_t* const stanza, const char* const from); gboolean stanza_is_muc_presence(xmpp_stanza_t* const stanza); gboolean stanza_is_muc_self_presence(xmpp_stanza_t* const stanza,