devs/cproof
1
2
Fork 1
Code Issues 43 Pull Requests 6 Actions Packages Projects 1 Releases Wiki Activity

Segfault on disco#info result without a from attribute #168

New Issue
Open
opened 2026-07-16 09:48:34 +00:00 by jabber.developer2 · 0 comments
jabber.developer2 commented 2026-07-16 09:48:34 +00:00
Collaborator
Copy Link

Summary

The client crashes with SIGSEGV immediately after login when the server answers the on-connect disco#info request with an iq result that has no from attribute, e.g. <iq type='result' id='...'/>. RFC 6120 §8.1.2.1 explicitly allows this: when from is omitted on a stanza received over a client-to-server stream, the client MUST treat it as coming from the server itself. So this is a crash on protocol-legal input, remotely triggerable by any server (or MITM on a plaintext/legacy connection).

Crash path

  1. _disco_info_response_id_handler() (src/xmpp/iq.c, ~line 2441) reads from = xmpp_stanza_get_from(stanza) — NULL when the attribute is absent — and passes it straight to connection_features_received(from).
  2. connection_features_received() (src/xmpp/connection.c, ~line 753) calls g_hash_table_remove(conn.requested_features, NULL).
  3. conn.requested_features is a g_str_hash table; g_str_hash(NULL) dereferences the NULL key → SIGSEGV.

Last log lines before the crash:

prof: DBG: Received disco#info response
prof: INF: [CONNECTION] connection_features_received (null)

Reproduction

Reproducible deterministically with a minimal fake server that completes legacy (XEP-0078) auth and replies <iq type='result' id='...'/> (no from, no <query/>) to every iq:

  1. Run a plain TCP server on port 5322 that (a) answers the stream header with a <stream:stream> open tag without a version attribute (forces legacy auth), (b) replies <iq type='result' id='X'/> to every iq, echoing the id.
  2. profanity -l DEBUG, then /connect test@localhost server 127.0.0.1 port 5322 tls disable auth legacy, any password.
  3. Login succeeds, the client sends its disco#info request, receives the bare result, and segfaults.

Observed on feat/ai-api-type @ f7a818dc1 (2026-07-16); the affected code is identical on current master.

Suggested fix

Apply the RFC 6120 "no from = from the server" rule at the boundary: in _disco_info_response_id_handler() (and any sibling result handlers that key on from), substitute connection_get_domain() when from is NULL before touching the feature tables. Defense in depth: make connection_features_received() and connection_get_features() NULL-safe (early return / domain fallback) so no other caller can reach g_str_hash(NULL).

Note the same handler also calls connection_get_features(from) and logs an error for an unknown from; with the domain substitution both paths become consistent with how the request was registered in conn.requested_features.

Impact

  • Crash (denial of service) on connect against non-conforming-looking but protocol-legal servers; no memory corruption beyond the NULL dereference.
  • Low likelihood with mainstream servers (ejabberd/Prosody set from), but trivially triggerable by a hostile or minimal server implementation.
## Summary The client crashes with SIGSEGV immediately after login when the server answers the on-connect disco#info request with an iq result that has no `from` attribute, e.g. `<iq type='result' id='...'/>`. RFC 6120 §8.1.2.1 explicitly allows this: when `from` is omitted on a stanza received over a client-to-server stream, the client MUST treat it as coming from the server itself. So this is a crash on protocol-legal input, remotely triggerable by any server (or MITM on a plaintext/legacy connection). ## Crash path 1. `_disco_info_response_id_handler()` (`src/xmpp/iq.c`, ~line 2441) reads `from = xmpp_stanza_get_from(stanza)` — `NULL` when the attribute is absent — and passes it straight to `connection_features_received(from)`. 2. `connection_features_received()` (`src/xmpp/connection.c`, ~line 753) calls `g_hash_table_remove(conn.requested_features, NULL)`. 3. `conn.requested_features` is a `g_str_hash` table; `g_str_hash(NULL)` dereferences the NULL key → SIGSEGV. Last log lines before the crash: ``` prof: DBG: Received disco#info response prof: INF: [CONNECTION] connection_features_received (null) ``` ## Reproduction Reproducible deterministically with a minimal fake server that completes legacy (XEP-0078) auth and replies `<iq type='result' id='...'/>` (no `from`, no `<query/>`) to every iq: 1. Run a plain TCP server on port 5322 that (a) answers the stream header with a `<stream:stream>` open tag without a `version` attribute (forces legacy auth), (b) replies `<iq type='result' id='X'/>` to every iq, echoing the id. 2. `profanity -l DEBUG`, then `/connect test@localhost server 127.0.0.1 port 5322 tls disable auth legacy`, any password. 3. Login succeeds, the client sends its disco#info request, receives the bare result, and segfaults. Observed on `feat/ai-api-type` @ `f7a818dc1` (2026-07-16); the affected code is identical on current master. ## Suggested fix Apply the RFC 6120 "no from = from the server" rule at the boundary: in `_disco_info_response_id_handler()` (and any sibling result handlers that key on `from`), substitute `connection_get_domain()` when `from` is `NULL` before touching the feature tables. Defense in depth: make `connection_features_received()` and `connection_get_features()` NULL-safe (early return / domain fallback) so no other caller can reach `g_str_hash(NULL)`. Note the same handler also calls `connection_get_features(from)` and logs an error for an unknown `from`; with the domain substitution both paths become consistent with how the request was registered in `conn.requested_features`. ## Impact - Crash (denial of service) on connect against non-conforming-looking but protocol-legal servers; no memory corruption beyond the NULL dereference. - Low likelihood with mainstream servers (ejabberd/Prosody set `from`), but trivially triggerable by a hostile or minimal server implementation.
jabber.developer2 added the
Kind/Bug
 label 2026-07-16 09:48:34 +00:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
Compat/Breaking
Breaking change that won't be backward compatible
Kind/Bug
Something is not working
Kind/Documentation
Documentation changes
Kind/Enhancement
Improve existing functionality
Kind/Feature
New functionality
Kind/Security
This is security issue
Kind/Testing
Issue or pull request related to testing
Priority
Critical
1
The priority is critical
Priority
High
2
The priority is high
Priority
Low
4
The priority is low
Priority
Medium
3
The priority is medium
Reviewed
Confirmed
1
Issue has been confirmed
Reviewed
Duplicate
2
This issue or pull request already exists
Reviewed
Invalid
3
Invalid issue
Reviewed
Won't Fix
3
This issue won't be fixed
Status
Abandoned
3
Somebody has started to work on this but abandoned work
Status
Blocked
1
Something is blocking this issue or pull request
Status
Need More Info
2
Feedback is required to reproduce issue or to continue work
No Label
Kind/Bug
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
admin jabber.developer jabber.developer2
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: devs/cproof#168
No description provided.
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?