security: validate <delay> from and <stanza-id> by attributes
Some checks failed
CI Code / Check spelling (pull_request) Successful in 17s
CI Code / Check coding style (pull_request) Successful in 32s
CI Code / Code Coverage (pull_request) Failing after 10m1s
CI Code / Linux (debian) (pull_request) Failing after 12m32s
CI Code / Linux (ubuntu) (pull_request) Failing after 12m45s
CI Code / Linux (arch) (pull_request) Failing after 13m30s
Some checks failed
CI Code / Check spelling (pull_request) Successful in 17s
CI Code / Check coding style (pull_request) Successful in 32s
CI Code / Code Coverage (pull_request) Failing after 10m1s
CI Code / Linux (debian) (pull_request) Failing after 12m32s
CI Code / Linux (ubuntu) (pull_request) Failing after 12m45s
CI Code / Linux (arch) (pull_request) Failing after 13m30s
XEP-0203 §4 (Delayed Delivery): Remote clients could inject <delay xmlns='urn:xmpp:delay'> 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 <delay>/<x> 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 — <delay> inside <forwarded> is already from our own server's archive. XEP-0359 §5 (Unique and Stable Stanza IDs): The <stanza-id> 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 <stanza-id> when by == room bare JID. - message.c (_handle_chat): only accept <stanza-id> when by == our server domain.
This commit is contained in:
@@ -1090,12 +1090,16 @@ _handle_groupchat(xmpp_stanza_t* const stanza)
|
||||
message->id = strdup(id);
|
||||
}
|
||||
|
||||
// XEP-0359 §5: only accept <stanza-id> 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 <delay> 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 <delay> 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 <stanza-id>
|
||||
// XEP-0359 §5: only accept <stanza-id> 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 <forwarded> 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 <delay> 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();
|
||||
}
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user