security: harden untrusted-input handling (issue #148)
All checks were successful
CI Code / Check spelling (pull_request) Successful in 18s
CI Code / Check coding style (pull_request) Successful in 47s
CI Code / Linux (debian) (pull_request) Successful in 9m33s
CI Code / Linux (ubuntu) (pull_request) Successful in 9m41s
CI Code / Code Coverage (pull_request) Successful in 8m3s
CI Code / Linux (arch) (pull_request) Successful in 11m0s
All checks were successful
CI Code / Check spelling (pull_request) Successful in 18s
CI Code / Check coding style (pull_request) Successful in 47s
CI Code / Linux (debian) (pull_request) Successful in 9m33s
CI Code / Linux (ubuntu) (pull_request) Successful in 9m41s
CI Code / Code Coverage (pull_request) Successful in 8m3s
CI Code / Linux (arch) (pull_request) Successful in 11m0s
T02: guard the receive-path handlers that dereferenced jid_create() without a NULL check — MUC join errors, subscribed/unsubscribed presence and, with silence.non-roster enabled, every incoming message. A stanza with a missing or malformed 'from' crashed the client. The XEP-0280 carbon path carried the same class twice: the forwarded message's 'from' was handed to xmpp_jid_bare(), which dereferences the jid it is given, and a malformed 'to' left the carbon dispatch dereferencing a NULL jid_create() result (REQ-INP-01) T11: restrict /url open and /url save to http, https and aesgcm, so a received file:, javascript: or data: URL is refused (REQ-INP-06); spawn terminal-notifier through g_spawn_async with an argv instead of building a shell command for system() (REQ-INP-07); apply the XEP-0359 disco gate to MAM result ids, as live stanza-ids already do (REQ-INP-05); replace control and bidi-reordering characters in incoming message bodies with U+FFFD before they reach the terminal, the logs and the database, keeping LRM/RLM for legitimate RTL text. That pass now runs on every display path: the OX one, where the call had been left commented out since the feature landed, and outgoing carbons, whose body is forwarded by the server (REQ-INP-08); cover JID part-length boundaries and invalid UTF-8 (REQ-INP-02) T10: replace strcpy/strcat/alloca and sprintf with g_strdup_printf and g_snprintf (REQ-MEM-03); allocate the OMEMO key buffers with g_malloc so a failed allocation cannot reach the following memcpy (REQ-MEM-04); remove the variable-length arrays and enforce -Werror=vla. Two of them were sized from remote input: the disco#info feature count and a chat message word length. The flag also caught a one-past-the-end write and a leak in the plugin autocompleter bindings (REQ-MEM-09). The OX receive path leaked every decrypted body, dropping the pointer instead of freeing it, and a failed strdup no longer costs the message body
This commit is contained in:
@@ -395,6 +395,10 @@ _presence_error_handler(xmpp_stanza_t* const stanza)
|
||||
}
|
||||
|
||||
auto_jid Jid* fulljid = jid_create(from);
|
||||
if (fulljid == NULL) {
|
||||
log_warning("MUC error presence received with missing or invalid from attribute");
|
||||
return;
|
||||
}
|
||||
log_info("Error joining room: %s, reason: %s", fulljid->barejid, error_cond);
|
||||
if (muc_active(fulljid->barejid)) {
|
||||
muc_leave(fulljid->barejid);
|
||||
@@ -451,6 +455,9 @@ _unsubscribed_handler(xmpp_stanza_t* const stanza)
|
||||
log_debug("Unsubscribed presence handler fired for %s", from);
|
||||
|
||||
auto_jid Jid* from_jid = jid_create(from);
|
||||
if (from_jid == NULL) {
|
||||
return;
|
||||
}
|
||||
sv_ev_subscription(from_jid->barejid, PRESENCE_UNSUBSCRIBED);
|
||||
autocomplete_remove(sub_requests_ac, from_jid->barejid);
|
||||
}
|
||||
@@ -466,6 +473,9 @@ _subscribed_handler(xmpp_stanza_t* const stanza)
|
||||
log_debug("Subscribed presence handler fired for %s", from);
|
||||
|
||||
auto_jid Jid* from_jid = jid_create(from);
|
||||
if (from_jid == NULL) {
|
||||
return;
|
||||
}
|
||||
sv_ev_subscription(from_jid->barejid, PRESENCE_SUBSCRIBED);
|
||||
autocomplete_remove(sub_requests_ac, from_jid->barejid);
|
||||
}
|
||||
@@ -476,6 +486,7 @@ _subscribe_handler(xmpp_stanza_t* const stanza)
|
||||
const char* from = xmpp_stanza_get_from(stanza);
|
||||
if (!from) {
|
||||
log_warning("Subscribe presence handler received with no from attribute");
|
||||
return;
|
||||
}
|
||||
log_debug("Subscribe presence handler fired for %s", from);
|
||||
|
||||
@@ -728,6 +739,9 @@ _muc_user_self_handler(xmpp_stanza_t* stanza)
|
||||
{
|
||||
const char* from = xmpp_stanza_get_from(stanza);
|
||||
auto_jid Jid* from_jid = jid_create(from);
|
||||
if (from_jid == NULL) { // caller validates, but don't rely on it
|
||||
return;
|
||||
}
|
||||
|
||||
log_debug("Room self presence received from %s", from_jid->fulljid);
|
||||
|
||||
@@ -810,6 +824,9 @@ _muc_user_occupant_handler(xmpp_stanza_t* stanza)
|
||||
{
|
||||
const char* from = xmpp_stanza_get_from(stanza);
|
||||
auto_jid Jid* from_jid = jid_create(from);
|
||||
if (from_jid == NULL) { // caller validates, but don't rely on it
|
||||
return;
|
||||
}
|
||||
|
||||
log_debug("Room presence received from %s", from_jid->fulljid);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user