Opened 11 years ago

Closed 11 years ago

Last modified 9 years ago

#6031 closed defect (invalid)

ASCII control characters cause problems with libpurple clients

Reported by: dhawes Owned by: deryni
Milestone: Component: XMPP
Version: 2.4.2 Keywords:
Cc:

Description

When ASCII control characters are sent to a group chat (Jabber), libpurple clients stop displaying new messages and the server connection fails after a while. When they reconnect, the logs from the group chat only display up to the point that the control character was entered, and the connection to the server will fail after another period of time.

This is the case with Pidgin 2.4.2, Adium 1.2.5, and an older gaim client. I have tried the control characters CTRL-A (SOH), CTRL-B (STX), CTRL-C (ETX), and CTRL-D (EOT), all of which cause the above behavior.

I tried this with a Java Jabber client, and all of the messages after the control character displayed without a problem. This seems to show that it isn't a server problem, but I won't rule that out yet.

To reproduce, start a group chat and paste a control character into the message box and send it. Type another message after this and see if it shows up in the chat window, which it should not. The debug window shows the message as received, but it does not show up in the chat window.

I have only tried this with Jabber group chats.

Change History (28)

comment:1 Changed 11 years ago by dhawes

Upon further investigation, it looks like the problem may be with the libxml call to xmlParseChunk in jabber_parser_process (parser.c).

When I replace the "" string that comes from the server with something else, I am able to see the messages I send (with the replacement string, of course), and there is no disconnect.

comment:2 Changed 11 years ago by dhawes

In xmlParseChunk, the control characters cause the xmlParserError XML_ERR_INVALID_CHAR /* 9 */, and the xmlParserCtxt (js->context) has its errNo set to 9 and disableSAX is set to 1.

On subsequent calls to xmlParseChunk, no action will be taken since there is an errNo and disableSAX is set to 1. It seems that after a period of time the connection will time out since no more XML messages from the server can be parsed.

I'm not quite sure what the proper thing to do in this case is, but it seems that the errNo in js->context should be checked and some action should be taken (create new context, reset context, shutdown?).

The following patch, though I do not think it is the proper solution, will allow the control characters to be discarded and the client will still be able to send and receive messages:

diff -ur pidgin-2.4.2/libpurple/protocols/jabber/parser.c pidgin-2.4.2-patch/libpurple/protocols/jabber/parser.c
--- pidgin-2.4.2/libpurple/protocols/jabber/parser.c    2008-05-12 15:07:44.000000000 -0400
+++ pidgin-2.4.2-patch/libpurple/protocols/jabber/parser.c  2008-06-11 10:57:29.000000000 -0400
@@ -197,5 +197,12 @@
            PURPLE_CONNECTION_ERROR_NETWORK_ERROR,
            _("XML Parse error"));
    }
+    if(js->context != NULL && js->context->errNo != XML_ERR_OK) {
+        do {
+            js->context->errNo = 0;
+            js->context->disableSAX = 0;
+        }
+        while(xmlParseChunk(js->context, "", 0, 0) > 0);
+    }
 }

comment:3 Changed 11 years ago by deryni

  • Owner changed from nwalp to deryni

comment:4 Changed 11 years ago by evands

Your patch would work around an XML_ERR_INVALID_CHAR error by parsing to the end and resetting the error info... but what if it were an uncoverable or systemic error, like XML_ERR_NO_MEMORY or perhaps XML_ERR_INTERNAL_ERROR? I'm afraid we could end up with an infinite loop.

comment:5 Changed 11 years ago by evands

I can't reproduce this because every server I've tried disconnects me as soon as I attempt to send a control character. What server and client are you using in which you can do this?

In any case, I believe d879d22351a06d6edbf90dada7a8c3e289f28653 should fix this. Please test and let us know.

comment:6 Changed 11 years ago by evands

  • Resolution set to fixed
  • Status changed from new to closed

Using the fact that Adium 1.2.6 does send control characters with its version string, I have verified that this fix works for me. I believe it's right for the case described here with the group chat, too, then; please reopen if not.

comment:7 Changed 11 years ago by dhawes

Pardon my ignorance, but what/where is d879d22351a06d6edbf90dada7a8c3e289f28653?

I'll try the changes in http://trac.adiumx.com/changeset/24245 and test.

comment:8 Changed 11 years ago by evands

It's an im.pidgin.pidgin monotone revision; same changes as the patch in Adium's revision 24245. Adium revision 24247 or later has the compiled libpurple.framework with the fix.

comment:9 Changed 11 years ago by dhawes

Applying the patch at http://trac.adiumx.com/browser/branches/adium-1.2/Utilities/dep-build-scripts/libpurple_jabber_parser_error_handler.diff?rev=24245&format=txt to Pidgin 2.4.3 does not fix the issue for me.

After I send a control character to the chat room, I am still unable to see any messages, although I can see them in the Send and Recv debug messages. Also, after the control character has been sent, the second message received from the server will result in an XML_ERR_DOCUMENT_END, and the account will immediately get disconnected.

Correct me if I'm wrong, but the desired action here is to ignore the control characters and not to disconnect, right?

I modified the patch to effectively "eat" the invalid chars like in my patch above, and it seems to work like I think it should (and errors are checked, so hopefully no possibility of infinite loops). Again, I'm not sure how proper that is, but it certainly takes care of the problem.

I have not tested the patch with the Adium version string issue yet.

comment:10 Changed 11 years ago by deryni

How are you sending a control character to the chat room? What server/muc component are you using that is not immediately disconnecting you? I don't believe it should be possible to make pidgin send such data to the server (that it likely is currently allowed is something I would consider a bug that should be fixed).

comment:11 Changed 11 years ago by deryni

Evan: Your code makes pidgin ignore the invalid characters (invalid entity references, etc.) and continue, correct? If so I believe, according to the draft versions of the XMPP RFCs (RFC3920bis-06 Section 5.8.3.17 and RFC3920bis-06 Section 12.1), that we should in fact be triggering an unrecoverable stream error and disconnecting the account.

comment:12 Changed 11 years ago by dhawes

I simply paste the control character into the Pidgin window. From the debug window it seems that the control character is sent unmodified to the server, and it comes back from the server as a character reference.

The server is Openfire 3.5.2.

comment:13 Changed 11 years ago by deryni

I should have guessed this was Openfire. When I attempt to paste a control character into the conversation window it pastes as a control character followed by "x#;" is that what you are seeing? It then appears to send that way and comes back as "&#x#;x#;"?

I stand by the fact that pidgin should not be able to send those sort of characters at all as they are forbidden by the XMPP RFCs, I also believe that Openfire is incorrect to convert them and pass them on. Partly because it should not be able to send restriced XML entities and partly because it should be dropping the connection when it sees them (or at least dropping those characters depending on whether they are trying to follow the bis RFC drafts).

comment:14 Changed 11 years ago by dhawes

That is not what I'm seeing. When I paste the control character into the conversation window they display as unicode replacement characters (hex value of the character inside of a little square).

I do this by entering the control character in vim (CTRL-V then CTRL-1), opening the file created by vim with kwrite, and then pasting the character into the window.

I completely agree that Pidgin should not send these characters. If that had been the case, I would probably not have come across this bug (as everyone in my group uses libpurple-based clients). I also agree that Openfire should not convert them and pass them on. I would prefer the characters be ignored rather than dropping the connection for the simple reason that it's annoying to have the reconnect issues when something goes wrong. Then again, I'd rather see RFC compliant clients/servers across the board.

comment:15 Changed 11 years ago by evands

Adium Trac #10382 discusses this issue, too. iChat apparently allows resource names with invalid characters, such as RIGHT SINGLE QUOTATION MARK (U+2019, 0xe2 0x80 0x99).

Besides fixing Pidgin not to attempt to send such things, we need to be able to handle receiving them as gracefully as possible. My change in d879d22351a06d6edbf90dada7a8c3e289f28653 made us actually throw the "XML Parse Error" disconnection error when receiving them. I thought that ignoring invalid characters would be okay, but it turns out the the 'error: document ended' error is what occurs. Previously, we silently entered a failed state.

The new behavior, in which we disconnect, means that any non-compliant client, such as iChat, can perform a DOS on any libpurple client if the server doesn't do proper checking, as Openfire apparently doesn't.

comment:16 Changed 11 years ago by evands

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:17 follow-up: Changed 11 years ago by deryni

I agree that the fact that we can be essentially forced to disconnect by being sent invalid data is not a good thing, but it *is* what is mandated by XMPP RFC (as I indicated in one of my previous comments). The fact that Openfire does not disconnect the sending client is a server bug and should be filed with them immediately (if it is not already known to them). Similarly the fact that iChat allows invalid characters to be sent should be filed as a bug with Apple (though I have significantly less hope of them fixing their bug than I do about the Openfire people fixing theirs).

I don't know that it is cleanly possible to ignore characters and recover, since the XML document will not be restarted by the server and I don't know whether libxml2 will handle that. We would probably have to fake that in order to even attempt to get this to work (something I strongly dislike even thinking about doing).

Disconnecting cleanly is a much better solution than being in an inconsistent internal state which causes message loss.

comment:18 Changed 11 years ago by evands

Indeed; according to the XML spec:

Once a fatal error is detected, however, the processor MUST NOT continue normal processing (i.e., it MUST NOT continue to pass character data and information about the document's logical structure to the application in the normal way).

My change, it turns out, doesn't actually allow any recovery. Ultimately, here's what happens:

15:08:42: (Libpurple: jabber) Recv (ssl)(213): <iq type="result" to="raeburn@mit.edu/Adiumi985" id="purple50a00886" from="somebody@mit.edu/Adium"><query xmlns="jabber:iq:version"><name>Adium</name><version>&#16;BÔøΩ&#16; (libpurple 2.4.3)</version></query></iq>
15:08:42: (Libpurple: jabber) XML parser error for JabberStream 0x0: Domain 1, code 9, level 3: xmlParseCharRef: invalid xmlChar value 16
15:08:42: (Libpurple: jabber) xmlParseChunk returned error 9

15:08:42: (Libpurple: jabber) Recv (ssl)(391): <presence from="somebody1@gmail.com/gmail.E541E193" to="raeburn@mit.edu/Adiumi985"><show>away</show><priority>0</priority><caps:c xmlns:caps="http://jabber.org/protocol/caps" node="http://mail.google.com/xmpp/client/caps" ver="1.1" ext="pmuc-v1"></caps:c><status>Wow, embedded chat!</status><x xmlns="vcard-temp:x:update"><photo>57c7617952cf2ddc707153466a23da3976f2da10</photo></x></presence>
15:08:42: (Libpurple: jabber) xmlParseChunk returned error 5
15:08:42: (Libpurple: jabber) Recv (ssl)(203): <iq type="result" to="raeburn@mit.edu/Adiumi985" id="purple50a00879" from="somebody2@mit.edu/Adium"><query xmlns="jabber:iq:version"><name>Adium</name><version> (libpurple 2.5.0devel)</version></query></iq>
15:08:42: (Libpurple: jabber) xmlParseChunk returned error 5
15:08:42: (Libpurple: jabber) Recv (ssl)(344): <presence from="somebody1@gmail.com/somebody1/fE092FC9F" to="raeburn@mit.edu/Adiumi985"><priority>1</priority><c xmlns="http://jabber.org/protocol/caps" node="http://pidgin.im/caps" ver="2.4.2devel" ext="moodn nickn tunen buzz avatar adiumcmd"/><x xmlns="vcard-temp:x:update"><photo>57c7617952cf2ddc707153466a23da3976f2da10</photo></x></presence>
15:08:42: (Libpurple: jabber) xmlParseChunk returned error 5
15:08:42: (Libpurple: jabber) Recv (ssl)(145): <iq type="result" id="purple50a0087a" to="raeburn@mit.edu/Adiumi985" from="somebody2@mit.edu/Adium"><query xmlns="jabber:iq:last" seconds="0"/></iq>
15:08:42: (Libpurple: jabber) xmlParseChunk returned error 5
15:08:42: (Libpurple: jabber) Recv (ssl)(405): <presence from="somebody1@gmail.com/abs.mit.ed1F423CED" to="raeburn@mit.edu/Adiumi985"><show>away</show><status>My desktop computer is online.  I am not.  Please try again later...</status><c xmlns="http://jabber.org/protocol/caps" node="http://pidgin.im/caps" ver="2.4.1" ext="moodn nickn tunen avatar"/><x xmlns="vcard-temp:x:update"><photo>57c7617952cf2ddc707153466a23da3976f2da10</photo></x></presence>
15:08:42: (Libpurple: jabber) xmlParseChunk returned error 5

15:08:42: (Libpurple: jabber) Recv (ssl)(206): <iq type="result" to="raeburn@mit.edu/Adiumi985" id="purple50a00882" from="somebody3@mit.edu/Pidgin"><query xmlns="jabber:iq:version"><name>Pidgin</name><version>2.4.1 (libpurple 2.4.1)</version></query></iq>
15:08:42: (Libpurple: jabber) XML parser error for JabberStream 0x0: Domain 1, code 5, level 3: Extra content at the end of the document

15:08:42: (Libpurple: jabber) xmlParseChunk returned error 5
15:08:42: Connection Disconnected: gc=afece70 (XML Parse error)

We handle the error 9 (invalid character)... but the document is then in an ended state. We continue to fail from there.

I guess we may just need to depend upon servers updating to the latest version of OpenFire, as this is a serverside problem which exposes all XML-compliant clients to DOS.

comment:19 in reply to: ↑ 17 Changed 11 years ago by irabinovitch

Earlier in this ticket, it was suggested that bugs reports be filed with Apple (re: iChat) and Jive/IgniteRealTime? (re: Openfire). Does anyone know if these bugs were ever filed?

-Ilan

Replying to deryni:

I agree that the fact that we can be essentially forced to disconnect by being sent invalid data is not a good thing, but it *is* what is mandated by XMPP RFC (as I indicated in one of my previous comments). The fact that Openfire does not disconnect the sending client is a server bug and should be filed with them immediately (if it is not already known to them). Similarly the fact that iChat allows invalid characters to be sent should be filed as a bug with Apple (though I have significantly less hope of them fixing their bug than I do about the Openfire people fixing theirs).

I don't know that it is cleanly possible to ignore characters and recover, since the XML document will not be restarted by the server and I don't know whether libxml2 will handle that. We would probably have to fake that in order to even attempt to get this to work (something I strongly dislike even thinking about doing).

Disconnecting cleanly is a much better solution than being in an inconsistent internal state which causes message loss.

comment:20 follow-up: Changed 11 years ago by deryni

I don't know if either bug was ever filed but I do know that the ChangeLog for Openfire 3.5.2 includes an entry for ticket 1388 which may be related, though it seems not to contain any actual information about the problem or about the fix so it is hard to determine.

comment:21 in reply to: ↑ 20 Changed 11 years ago by dhawes

http://trac.adiumx.com/ticket/10353#comment:15

It seems that the fix to prevent clients from sending control characters was not in 3.5.2.

comment:22 Changed 11 years ago by deryni

  • Resolution set to invalid
  • Status changed from new to closed

I'm going to close this as it isn't a libpurple bug and Openfire at least is going to be fixed (hopefully sooner rather than later).

comment:23 Changed 11 years ago by himdel

Still doesn't work with openfire 3.6.0.

comment:24 Changed 11 years ago by deryni

There is still nothing we can do about this, OpenFire needs to fix it on their end.

comment:25 Changed 11 years ago by himdel

Agreed. But openfire bug JM-1388 is referenced here as related and that bug is marked as fixed in openfire-3.5.2, so I wanted to make it known that openfire-3.6.0 still suffers the same problem.

comment:26 Changed 11 years ago by deryni

Understood, thank you.

comment:27 Changed 9 years ago by sergem

You look at this bug wrong. This IS a bug in libpurple, since it allows a DoS-attack against it.

Steps to reproduce a DoS attack:

  1. Connect to Jabber conference with i.e. psi.
  2. Send a message, that contains ASCII char 1
  3. See all libpurple clients disconnected.

After that none of libpurple clients would be able to connect to this conference, because after reconnection they would receive last messages, including the one that makes them disconnect.

And I suppose the attack can be done in many other way (i.e. by setting a status message with such a character, or even just directly sending a message).

Here's the patch to fix this problem

  • libpurple/protocols/jabber/parser.c

    diff -urN pidgin-2.7.0.orig/libpurple/protocols/jabber/parser.c pidgin-2.7.0/libpurple/protocols/jabber/parser.c
    old new  
    285285                                break;
    286286                        case XML_ERR_FATAL:
    287287                                purple_debug_error("jabber", "xmlParseChunk returned fatal %i\n", ret);
    288                                 purple_connection_error_reason (js->gc,
    289                                                                 PURPLE_CONNECTION_ERROR_NETWORK_ERROR,
    290                                                                 _("XML Parse error"));
     288                                if ((ret >= XML_ERR_INVALID_HEX_CHARREF) && (ret <= XML_ERR_INVALID_CHAR)) {
     289                                        char *open_stream = g_strdup_printf("<stream:stream "
     290                                                "xmlns='" NS_XMPP_CLIENT "' "
     291                                                "xmlns:stream='" NS_XMPP_STREAMS "' "
     292                                                "id='%s' "
     293                                                "version='%d.%d'>",
     294                                                js->stream_id,
     295                                                js->protocol_version.major, js->protocol_version.minor);
     296                                        jabber_parser_free(js);
     297                                        js->context = xmlCreatePushParserCtxt(&jabber_parser_libxml,
     298                                                js, open_stream, strlen(open_stream), NULL);
     299                                        g_free(open_stream);
     300                                } else {
     301                                        purple_connection_error_reason (js->gc,
     302                                                                        PURPLE_CONNECTION_ERROR_NETWORK_ERROR,
     303                                                                        _("XML Parse error"));
     304                                }
    291305                                break;
    292306                }
    293307        }

This patch ignores all "incorrect-char" messages. IMO it would be better if pidgin displayed them, after all UTF8 allows to see them, but losing some messages is better than not being able to login at all.

PS: I cannot reopen this bug but I hope some pidgin developers will look through it.

PPS: I personally got into this problem and had to find a fast solution, because I could not connect to jabber any more. Here is my solution. :)

comment:28 Changed 9 years ago by darkrain42

Ticket #12170 has been marked as a duplicate of this ticket.

Note: See TracTickets for help on using tickets.
All information, including names and email addresses, entered onto this website or sent to mailing lists affiliated with this website will be public. Do not post confidential information, especially passwords!