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

fix: validate stanza-id 'by' attr and route MUC MAM correctly (XEP-0359, XEP-0313) #102

New Issue
Open
opened 2026-03-12 08:12:47 +00:00 by jabber.developer2 · 0 comments
jabber.developer2 commented 2026-03-12 08:12:47 +00:00
Collaborator
Copy Link

Issue 1 — Security: <stanza-id> by attribute not validated (XEP-0359)

Title

XEP-0359: <stanza-id> accepted without validating the by attribute

Description

The <stanza-id> element (XEP-0359) is parsed in both _handle_chat() and _handle_groupchat() without checking the by attribute against the expected entity.

XEP-0359 §5 (Security Considerations):

Clients MUST only trust stanza-ids when they are confident that the entity assigning the stanza-id is the actual entity advertised by the by attribute.

Current code

message.c, _handle_chat():

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);
    }
}

The same pattern exists in _handle_groupchat().

Impact

A malicious remote contact or a compromised MUC participant can inject a forged <stanza-id by="evil.example"> element into a message. Since the by attribute is never checked:

  • Dedup bypass/DoS: an attacker can craft a <stanza-id> matching an existing archive-id, causing the client to flag a legitimate live message as a duplicate and silently drop it.
  • LMC spoofing preparation: the attacker-supplied stanza-id becomes the key used for XEP-0308 Last Message Correction sender validation. If the attacker can predict or replay the id, they may be able to correct someone else's message.

Expected behavior

For 1:1 chat messages, the by attribute must match the bare JID of the user's own server (account domain).
For MUC messages, the by attribute must match the bare JID of the room.

Any <stanza-id> element whose by does not match the expected entity should be ignored.

Suggested fix

// 1:1 chat — only trust stanza-id from our server
if (stanzaidst) {
    const char* by = xmpp_stanza_get_attribute(stanzaidst, "by");
    if (by && equals_our_domainpart(by)) {
        stanzaid = (char*)xmpp_stanza_get_attribute(stanzaidst, STANZA_ATTR_ID);
        ...
    }
}

// MUC — only trust stanza-id from the room
if (stanzaidst) {
    const char* by = xmpp_stanza_get_attribute(stanzaidst, "by");
    if (by && g_strcmp0(by, from_jid->barejid) == 0) {
        stanzaid = (char*)xmpp_stanza_get_attribute(stanzaidst, STANZA_ATTR_ID);
        ...
    }
}

References

Issue 2 — MUC MAM messages misrouted through _handle_chat

Title

MUC MAM results handled as 1:1 chat / private MUC message instead of room message

Description

When a MUC room responds to a MAM query (XEP-0313 §7), the result messages are sent as:

<message from="room@conference.example" to="user@example/resource">
  <result xmlns="urn:xmpp:mam:2" id="archive-id-123">
    <forwarded xmlns="urn:xmpp:forward:0">
      <delay xmlns="urn:xmpp:delay" stamp="2025-01-01T00:00:00Z"/>
      <message type="groupchat" from="room@conference.example/nick" ...>
        <body>Hello</body>
      </message>
    </forwarded>
  </result>
</message>

The outer <message> has no type attribute (or type="chat"), so _message_handler() routes it to the type == NULL branch, which calls _handle_mam().

_handle_mam() unconditionally calls _handle_chat() on the inner forwarded <message type="groupchat">.

Inside _handle_chat():

if (muc_active(jid->barejid)) {
    _handle_muc_private_message(stanza);
    return;
}

If the room is active (which it normally is when MAM is requested), the MUC MAM message is misrouted to _handle_muc_private_message().

If the room is not active, it falls through and is treated as a regular 1:1 chat message with a wrong JID.

Expected behavior

_handle_mam() should detect the inner message type. If type="groupchat", it should delegate to a MUC-aware MAM handler that processes the message as room history (similar to sv_ev_room_history), not as a 1:1 chat.

Suggested approach

static gboolean
_handle_mam(xmpp_stanza_t* const stanza)
{
    // ... existing result/forwarded/result_id/timestamp parsing ...

    xmpp_stanza_t* message_stanza = xmpp_stanza_get_child_by_ns(forwarded, "jabber:client");
    
    const char* type = xmpp_stanza_get_type(message_stanza);
    if (type && g_strcmp0(type, STANZA_TYPE_GROUPCHAT) == 0) {
        _handle_muc_mam(message_stanza, result_id, timestamp);
    } else {
        _handle_chat(message_stanza, TRUE, FALSE, result_id, timestamp);
    }
    
    return TRUE;
}

References

**Issue 1 — Security: `<stanza-id>` `by` attribute not validated (XEP-0359)** --- ### Title XEP-0359: `<stanza-id>` accepted without validating the `by` attribute ### Description The `<stanza-id>` element (XEP-0359) is parsed in both `_handle_chat()` and `_handle_groupchat()` without checking the `by` attribute against the expected entity. **XEP-0359 §5 (Security Considerations):** > Clients MUST only trust stanza-ids when they are confident that the entity assigning the stanza-id is the actual entity advertised by the `by` attribute. #### Current code message.c, `_handle_chat()`: ```c 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); } } ``` The same pattern exists in `_handle_groupchat()`. #### Impact A malicious remote contact or a compromised MUC participant can inject a forged `<stanza-id by="evil.example">` element into a message. Since the `by` attribute is never checked: - **Dedup bypass/DoS**: an attacker can craft a `<stanza-id>` matching an existing archive-id, causing the client to flag a legitimate live message as a duplicate and silently drop it. - **LMC spoofing preparation**: the attacker-supplied stanza-id becomes the key used for XEP-0308 Last Message Correction sender validation. If the attacker can predict or replay the id, they may be able to correct someone else's message. #### Expected behavior For **1:1 chat** messages, the `by` attribute must match the bare JID of the user's own server (account domain). For **MUC** messages, the `by` attribute must match the bare JID of the room. Any `<stanza-id>` element whose `by` does not match the expected entity should be ignored. #### Suggested fix ```c // 1:1 chat — only trust stanza-id from our server if (stanzaidst) { const char* by = xmpp_stanza_get_attribute(stanzaidst, "by"); if (by && equals_our_domainpart(by)) { stanzaid = (char*)xmpp_stanza_get_attribute(stanzaidst, STANZA_ATTR_ID); ... } } // MUC — only trust stanza-id from the room if (stanzaidst) { const char* by = xmpp_stanza_get_attribute(stanzaidst, "by"); if (by && g_strcmp0(by, from_jid->barejid) == 0) { stanzaid = (char*)xmpp_stanza_get_attribute(stanzaidst, STANZA_ATTR_ID); ... } } ``` ### References - XEP-0359: Unique and Stable Stanza IDs — https://xmpp.org/extensions/xep-0359.html#security - XEP-0313: Message Archive Management — https://xmpp.org/extensions/xep-0313.html --- **Issue 2 — MUC MAM messages misrouted through `_handle_chat`** --- ### Title MUC MAM results handled as 1:1 chat / private MUC message instead of room message ### Description When a MUC room responds to a MAM query (XEP-0313 §7), the result messages are sent as: ```xml <message from="room@conference.example" to="user@example/resource"> <result xmlns="urn:xmpp:mam:2" id="archive-id-123"> <forwarded xmlns="urn:xmpp:forward:0"> <delay xmlns="urn:xmpp:delay" stamp="2025-01-01T00:00:00Z"/> <message type="groupchat" from="room@conference.example/nick" ...> <body>Hello</body> </message> </forwarded> </result> </message> ``` The **outer** `<message>` has no `type` attribute (or `type="chat"`), so `_message_handler()` routes it to the `type == NULL` branch, which calls `_handle_mam()`. `_handle_mam()` unconditionally calls `_handle_chat()` on the **inner** forwarded `<message type="groupchat">`. Inside `_handle_chat()`: ```c if (muc_active(jid->barejid)) { _handle_muc_private_message(stanza); return; } ``` **If the room is active** (which it normally is when MAM is requested), the MUC MAM message is misrouted to `_handle_muc_private_message()`. **If the room is not active**, it falls through and is treated as a regular 1:1 chat message with a wrong JID. #### Expected behavior `_handle_mam()` should detect the inner message type. If `type="groupchat"`, it should delegate to a MUC-aware MAM handler that processes the message as room history (similar to `sv_ev_room_history`), not as a 1:1 chat. #### Suggested approach ```c static gboolean _handle_mam(xmpp_stanza_t* const stanza) { // ... existing result/forwarded/result_id/timestamp parsing ... xmpp_stanza_t* message_stanza = xmpp_stanza_get_child_by_ns(forwarded, "jabber:client"); const char* type = xmpp_stanza_get_type(message_stanza); if (type && g_strcmp0(type, STANZA_TYPE_GROUPCHAT) == 0) { _handle_muc_mam(message_stanza, result_id, timestamp); } else { _handle_chat(message_stanza, TRUE, FALSE, result_id, timestamp); } return TRUE; } ``` ### References - XEP-0313 §7: MUC Archives — https://xmpp.org/extensions/xep-0313.html#muc
jabber.developer2 added the
Kind/Bug
 label 2026-03-12 08:12:47 +00:00
jabber.developer2 added this to the Plans (2025-2026) project 2026-03-12 08:12:47 +00:00
jabber.developer added the
Priority
High
2
 label 2026-03-12 15:38:30 +00:00
jabber.developer moved this to To Do in Plans (2025-2026) on 2026-06-24 16:45:55 +00:00
jabber.developer moved this to In Progress in Plans (2025-2026) on 2026-06-25 06:39:57 +00:00
jabber.developer2 was assigned by jabber.developer 2026-07-05 12:35:45 +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
Priority
High
2
Milestone
No items
No Milestone
Projects
Clear projects
No project Plans (2025-2026)
Assignees
Clear assignees
admin jabber.developer jabber.developer2
No Assignees jabber.developer2
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: devs/cproof#102
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?