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
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
This commit is contained in:
@@ -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*
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user