From 6a969c7e8f80199f8b8bd5d76b40eb3f53ab3ca4 Mon Sep 17 00:00:00 2001
From: Art Cancro <ajc@citadel.org>
Date: Fri, 14 May 2010 15:24:26 +0000
Subject: [PATCH] * Properly escape XML output using new xmlesc() function for
 XMPP sessions.  This function should eventually get moved into libcitadel,
 but the bigger problem is that it doesn't handle UTF-8.  Right now we're just
 using it to keep Pidgin from barfing on illegal characters.

---
 citadel/modules/xmpp/serv_xmpp.c            | 78 ++++++++++++++++++---
 citadel/modules/xmpp/serv_xmpp.h            |  1 +
 citadel/modules/xmpp/xmpp_messages.c        |  9 ++-
 citadel/modules/xmpp/xmpp_presence.c        | 24 ++++---
 citadel/modules/xmpp/xmpp_query_namespace.c | 14 ++--
 citadel/modules/xmpp/xmpp_sasl_service.c    |  5 +-
 6 files changed, 100 insertions(+), 31 deletions(-)

diff --git a/citadel/modules/xmpp/serv_xmpp.c b/citadel/modules/xmpp/serv_xmpp.c
index 8c7297350..d9271e464 100644
--- a/citadel/modules/xmpp/serv_xmpp.c
+++ b/citadel/modules/xmpp/serv_xmpp.c
@@ -61,10 +61,65 @@
 
 struct xmpp_event *xmpp_queue = NULL;
 
-/* We have just received a <stream> tag from the client, so send them ours */
+/*
+ * Given a source string and a target buffer, returns the string
+ * properly escaped for insertion into an XML stream.  Returns a
+ * pointer to the target buffer for convenience.
+ *
+ * BUG: this does not properly handle UTF-8
+ */
+char *xmlesc(char *buf, char *str, int bufsiz)
+{
+	char *ptr;
+	unsigned char ch;
+	int len = 0;
+
+	if (!buf) return(NULL);
+	buf[0] = 0;
+	len = 0;
+	if (!str) {
+		return(buf);
+	}
+
+	for (ptr=str; *ptr; ptr++) {
+		ch = *ptr;
+		if (ch == '<') {
+			strcpy(&buf[len], "&lt;");
+			len += 4;
+		}
+		else if (ch == '>') {
+			strcpy(&buf[len], "&gt;");
+			len += 4;
+		}
+		else if (ch == '&') {
+			strcpy(&buf[len], "&amp;");
+			len += 5;
+		}
+		else if (ch <= 0x7F) {
+			buf[len++] = ch;
+			buf[len] = 0;
+		}
+		else if (ch > 0x7F) {
+			char oct[10];
+			sprintf(oct, "&#%o;", ch);
+			strcpy(&buf[len], oct);
+			len += strlen(oct);
+		}
+		if ((len + 6) > bufsiz) {
+			return(buf);
+		}
+	}
+	return(buf);
+}
 
+
+/*
+ * We have just received a <stream> tag from the client, so send them ours
+ */
 void xmpp_stream_start(void *data, const char *supplied_el, const char **attr)
 {
+	char xmlbuf[256];
+
 	while (*attr) {
 		if (!strcasecmp(attr[0], "to")) {
 			safestrncpy(XMPP->server_name, attr[1], sizeof XMPP->server_name);
@@ -75,7 +130,7 @@ void xmpp_stream_start(void *data, const char *supplied_el, const char **attr)
 	cprintf("<?xml version=\"1.0\" encoding=\"UTF-8\"?>");
 
    	cprintf("<stream:stream ");
-	cprintf("from=\"%s\" ", XMPP->server_name);
+	cprintf("from=\"%s\" ", xmlesc(xmlbuf, XMPP->server_name, sizeof xmlbuf));
 	cprintf("id=\"%08x\" ", CC->cs_pid);
 	cprintf("version=\"1.0\" ");
 	cprintf("xmlns:stream=\"http://etherx.jabber.org/streams\" ");
@@ -184,6 +239,7 @@ void xmpp_xml_start(void *data, const char *supplied_el, const char **attr) {
 void xmpp_xml_end(void *data, const char *supplied_el) {
 	char el[256];
 	char *sep = NULL;
+	char xmlbuf[256];
 
 	/* Axe the namespace, we don't care about it */
 	safestrncpy(el, supplied_el, sizeof el);
@@ -243,12 +299,12 @@ void xmpp_xml_end(void *data, const char *supplied_el) {
 			else if (XMPP->ping_requested) {
 				cprintf("<iq type=\"result\" ");
 				if (!IsEmptyStr(XMPP->iq_from)) {
-					cprintf("to=\"%s\" ", XMPP->iq_from);
+					cprintf("to=\"%s\" ", xmlesc(xmlbuf, XMPP->iq_from, sizeof xmlbuf));
 				}
 				if (!IsEmptyStr(XMPP->iq_to)) {
-					cprintf("from=\"%s\" ", XMPP->iq_to);
+					cprintf("from=\"%s\" ", xmlesc(xmlbuf, XMPP->iq_to, sizeof xmlbuf));
 				}
-				cprintf("id=\"%s\"/>", XMPP->iq_id);
+				cprintf("id=\"%s\"/>", xmlesc(xmlbuf, XMPP->iq_id, sizeof xmlbuf));
 			}
 
 			/*
@@ -259,7 +315,7 @@ void xmpp_xml_end(void *data, const char *supplied_el) {
 					"Unknown query <%s> - returning <service-unavailable/>\n",
 					el
 				);
-				cprintf("<iq type=\"error\" id=\"%s\">", XMPP->iq_id);
+				cprintf("<iq type=\"error\" id=\"%s\">", xmlesc(xmlbuf, XMPP->iq_id, sizeof xmlbuf));
 				cprintf("<error code=\"503\" type=\"cancel\">"
 					"<service-unavailable xmlns=\"urn:ietf:params:xml:ns:xmpp-stanzas\"/>"
 					"</error>"
@@ -304,21 +360,21 @@ void xmpp_xml_end(void *data, const char *supplied_el) {
 
 			/* Tell the client what its JID is */
 
-			cprintf("<iq type=\"result\" id=\"%s\">", XMPP->iq_id);
+			cprintf("<iq type=\"result\" id=\"%s\">", xmlesc(xmlbuf, XMPP->iq_id, sizeof xmlbuf));
 			cprintf("<bind xmlns=\"urn:ietf:params:xml:ns:xmpp-bind\">");
-			cprintf("<jid>%s</jid>", XMPP->client_jid);
+			cprintf("<jid>%s</jid>", xmlesc(xmlbuf, XMPP->client_jid, sizeof xmlbuf));
 			cprintf("</bind>");
 			cprintf("</iq>");
 		}
 
 		else if (XMPP->iq_session) {
-			cprintf("<iq type=\"result\" id=\"%s\">", XMPP->iq_id);
+			cprintf("<iq type=\"result\" id=\"%s\">", xmlesc(xmlbuf, XMPP->iq_id, sizeof xmlbuf));
 			cprintf("</iq>");
 		}
 
 		else {
-			cprintf("<iq type=\"error\" id=\"%s\">", XMPP->iq_id);
-			cprintf("<error>Don't know howto do '%s'!</error>", XMPP->iq_type);
+			cprintf("<iq type=\"error\" id=\"%s\">", xmlesc(xmlbuf, XMPP->iq_id, sizeof xmlbuf));
+			cprintf("<error>Don't know howto do '%s'!</error>", xmlesc(xmlbuf, XMPP->iq_type, sizeof xmlbuf));
 			cprintf("</iq>");
 		}
 
diff --git a/citadel/modules/xmpp/serv_xmpp.h b/citadel/modules/xmpp/serv_xmpp.h
index 0292d9716..989ec798d 100644
--- a/citadel/modules/xmpp/serv_xmpp.h
+++ b/citadel/modules/xmpp/serv_xmpp.h
@@ -84,3 +84,4 @@ void xmpp_non_sasl_authenticate(char *, char *, char *, char *);
 void xmpp_massacre_roster(void);
 void xmpp_delete_old_buddies_who_no_longer_exist_from_the_client_roster(void);
 int xmpp_is_visible(struct CitContext *from, struct CitContext *to_whom);
+char *xmlesc(char *buf, char *str, int bufsiz);
diff --git a/citadel/modules/xmpp/xmpp_messages.c b/citadel/modules/xmpp/xmpp_messages.c
index 2544867d4..d989beb68 100644
--- a/citadel/modules/xmpp/xmpp_messages.c
+++ b/citadel/modules/xmpp/xmpp_messages.c
@@ -67,6 +67,8 @@
 void xmpp_output_incoming_messages(void) {
 
 	struct ExpressMessage *ptr;
+	char xmlbuf1[4096];
+	char xmlbuf2[4096];
 
 	while (CC->FirstExpressMessage != NULL) {
 
@@ -76,11 +78,12 @@ void xmpp_output_incoming_messages(void) {
 		end_critical_section(S_SESSION_TABLE);
 
 		cprintf("<message to=\"%s\" from=\"%s\" type=\"chat\">",
-			XMPP->client_jid,
-			ptr->sender_email);
+			xmlesc(xmlbuf1, XMPP->client_jid, sizeof xmlbuf1),
+			xmlesc(xmlbuf2, ptr->sender_email, sizeof xmlbuf2)
+		);
 		if (ptr->text != NULL) {
 			striplt(ptr->text);
-			cprintf("<body>%s</body>", ptr->text);
+			cprintf("<body>%s</body>", xmlesc(xmlbuf1, ptr->text, sizeof xmlbuf1));
 			free(ptr->text);
 		}
 		cprintf("</message>");
diff --git a/citadel/modules/xmpp/xmpp_presence.c b/citadel/modules/xmpp/xmpp_presence.c
index 5dadfe38e..e1621a904 100644
--- a/citadel/modules/xmpp/xmpp_presence.c
+++ b/citadel/modules/xmpp/xmpp_presence.c
@@ -67,10 +67,10 @@
  */
 void xmpp_indicate_presence(char *presence_jid)
 {
-	cprintf("<presence from=\"%s\" to=\"%s\"></presence>",
-		presence_jid,
-		XMPP->client_jid
-	);
+	char xmlbuf[256];
+
+	cprintf("<presence from=\"%s\" ", xmlesc(xmlbuf, presence_jid, sizeof xmlbuf));
+	cprintf("to=\"%s\"></presence>", xmlesc(xmlbuf, XMPP->client_jid, sizeof xmlbuf));
 }
 
 
@@ -123,6 +123,8 @@ void xmpp_wholist_presence_dump(void)
  */
 void xmpp_destroy_buddy(char *presence_jid) {
 	static int unsolicited_id = 1;
+	char xmlbuf1[256];
+	char xmlbuf2[256];
 
 	if (!presence_jid) return;
 	if (!XMPP) return;
@@ -130,22 +132,24 @@ void xmpp_destroy_buddy(char *presence_jid) {
 
 	/* Transmit non-presence information */
 	cprintf("<presence type=\"unavailable\" from=\"%s\" to=\"%s\"></presence>",
-		presence_jid, XMPP->client_jid
+		xmlesc(xmlbuf1, presence_jid, sizeof xmlbuf1),
+		xmlesc(xmlbuf2, XMPP->client_jid, sizeof xmlbuf2)
 	);
 	cprintf("<presence type=\"unsubscribed\" from=\"%s\" to=\"%s\"></presence>",
-		presence_jid, XMPP->client_jid
+		xmlesc(xmlbuf1, presence_jid, sizeof xmlbuf1),
+		xmlesc(xmlbuf2, XMPP->client_jid, sizeof xmlbuf2)
 	);
 	// FIXME ... we should implement xmpp_indicate_nonpresence so we can use it elsewhere
 
 	/* Do an unsolicited roster update that deletes the contact. */
 	cprintf("<iq from=\"%s\" to=\"%s\" id=\"unbuddy_%x\" type=\"result\">",
-		CC->cs_inet_email,
-		XMPP->client_jid,
+		xmlesc(xmlbuf1, CC->cs_inet_email, sizeof xmlbuf1),
+		xmlesc(xmlbuf2, XMPP->client_jid, sizeof xmlbuf2),
 		++unsolicited_id
 	);
 	cprintf("<query xmlns=\"jabber:iq:roster\">");
-	cprintf("<item jid=\"%s\" subscription=\"remove\">", presence_jid);
-	cprintf("<group>%s</group>", config.c_humannode);
+	cprintf("<item jid=\"%s\" subscription=\"remove\">", xmlesc(xmlbuf1, presence_jid, sizeof xmlbuf1));
+	cprintf("<group>%s</group>", xmlesc(xmlbuf1, config.c_humannode, sizeof xmlbuf1));
 	cprintf("</item>");
 	cprintf("</query>"
 		"</iq>"
diff --git a/citadel/modules/xmpp/xmpp_query_namespace.c b/citadel/modules/xmpp/xmpp_query_namespace.c
index 720233e03..9a6826a76 100644
--- a/citadel/modules/xmpp/xmpp_query_namespace.c
+++ b/citadel/modules/xmpp/xmpp_query_namespace.c
@@ -62,11 +62,14 @@
  * Output a single roster item, for roster queries or pushes
  */
 void xmpp_roster_item(struct CitContext *cptr) {
+	char xmlbuf1[256];
+	char xmlbuf2[256];
+
 	cprintf("<item jid=\"%s\" name=\"%s\" subscription=\"both\">",
-		cptr->cs_inet_email,
-		cptr->user.fullname
+		xmlesc(xmlbuf1, cptr->cs_inet_email, sizeof xmlbuf1),
+		xmlesc(xmlbuf2, cptr->user.fullname, sizeof xmlbuf2)
 	);
-	cprintf("<group>%s</group>", config.c_humannode);
+	cprintf("<group>%s</group>", xmlesc(xmlbuf1, config.c_humannode, sizeof xmlbuf1));
 	cprintf("</item>");
 }
 
@@ -110,6 +113,7 @@ void xmpp_query_namespace(char *iq_id, char *iq_from, char *iq_to, char *query_x
 {
 	int supported_namespace = 0;
 	int roster_query = 0;
+	char xmlbuf[256];
 
 	/* We need to know before we begin the response whether this is a supported namespace, so
 	 * unfortunately all supported namespaces need to be defined here *and* down below where
@@ -134,9 +138,9 @@ void xmpp_query_namespace(char *iq_id, char *iq_from, char *iq_to, char *query_x
 		cprintf("<iq type=\"error\" ");
 	}
 	if (!IsEmptyStr(iq_from)) {
-		cprintf("to=\"%s\" ", iq_from);
+		cprintf("to=\"%s\" ", xmlesc(xmlbuf, iq_from, sizeof xmlbuf));
 	}
-	cprintf("id=\"%s\">", iq_id);
+	cprintf("id=\"%s\">", xmlesc(xmlbuf, iq_id, sizeof xmlbuf));
 
 	/*
 	 * Is this a query we know how to handle?
diff --git a/citadel/modules/xmpp/xmpp_sasl_service.c b/citadel/modules/xmpp/xmpp_sasl_service.c
index 8e952d2c8..a9d73b360 100644
--- a/citadel/modules/xmpp/xmpp_sasl_service.c
+++ b/citadel/modules/xmpp/xmpp_sasl_service.c
@@ -156,6 +156,7 @@ void xmpp_sasl_auth(char *sasl_auth_mech, char *authstring) {
  */
 void xmpp_non_sasl_authenticate(char *iq_id, char *username, char *password, char *resource) {
 	int result;
+	char xmlbuf[256];
 
         if (CC->logged_in) CtdlUserLogout();  /* Client may try to log in twice.  Handle this. */
 
@@ -163,13 +164,13 @@ void xmpp_non_sasl_authenticate(char *iq_id, char *username, char *password, cha
 	if (result == login_ok) {
 		result = CtdlTryPassword(password);
 		if (result == pass_ok) {
-			cprintf("<iq type=\"result\" id=\"%s\"></iq>", iq_id);	/* success */
+			cprintf("<iq type=\"result\" id=\"%s\"></iq>", xmlesc(xmlbuf, iq_id, sizeof xmlbuf));	/* success */
 			return;
 		}
 	}
 
 	/* failure */
-	cprintf("<iq type=\"error\" id=\"%s\">", iq_id);
+	cprintf("<iq type=\"error\" id=\"%s\">", xmlesc(xmlbuf, iq_id, sizeof xmlbuf));
 	cprintf("<error code=\"401\" type=\"auth\">"
 		"<not-authorized xmlns=\"urn:ietf:params:xml:ns:xmpp-stanzas\"/>"
 		"</error>"
-- 
2.30.2