Vulnerability: An incorrect implementation of "XEP-0280: Message Carbons" in multiple XMPP clients allows a remote attacker to impersonate any user, including contacts, in the vulnerable application's display. This allows for various kinds of social engineering attacks. This CVE is for Psi+ (0.16.563.580 - 0.16.571.627).
Vulnerability ID: CVE-2017-5593
CWE: CWE-20
Severity score: 2.9
Project: https://github.com/psi-im/iris
Published: 2017-02-09
File: src/xmpp/xmpp-im/xmpp_tasks.cpp
Commit message: Try to fix XMPP Client User Impersonation Vulnerability
Commit author: Vitaly Tonkacheyev

Vulnerable version (before fix):
 bool JT_PushMessage::take(const QDomElement &e) 	QDomElement forward; 	Message::CarbonDir cd = Message::NoCarbon;  	// Check for Carbon 	QDomNodeList list = e1.childNodes(); 	for (int i = 0; i < list.size(); ++i) { 		QDomElement el = list.at(i).toElement();  		if (el.attribute("xmlns") == QLatin1String("urn:xmpp:carbons:2") && (el.tagName() == QLatin1String("received") || el.tagName() == QLatin1String("sent"))) { 			QDomElement el1 = el.firstChildElement(); 			if (el1.tagName() == QLatin1String("forwarded") && el1.attribute(QLatin1String("xmlns")) == QLatin1String("urn:xmpp:forward:0")) { 				QDomElement el2 = el1.firstChildElement(QLatin1String("message")); 				if (!el2.isNull()) { 					forward = el2; bool JT_PushMessage::take(const QDomElement &e) 				} 			} 		} 		else if (el.tagName() == QLatin1String("forwarded") && el.attribute(QLatin1String("xmlns")) == QLatin1String("urn:xmpp:forward:0")) { 			forward = el.firstChildElement(QLatin1String("message")); // currently only messages are supportted 			// TODO <delay> element support 			if (!forward.isNull()) { bool JT_PushMessage::take(const QDomElement &e) 		} 	}  	QString from = e1.attribute(QLatin1String("from")); 	Stanza s = client()->stream().createStanza(addCorrectNS(forward.isNull()? e1 : forward)); 	if(s.isNull()) { 		//printf("take: bad stanza??\n"); bool JT_PushMessage::take(const QDomElement &e) 		return false; 	} 	if (!forward.isNull()) { 		m.setForwardedFrom(Jid(from)); 		m.setCarbonDirection(cd); 	} 

Fixed version (after fix):
 bool JT_PushMessage::take(const QDomElement &e) 	QDomElement forward; 	Message::CarbonDir cd = Message::NoCarbon;  	Jid fromJid = Jid(e1.attribute(QLatin1String("from"))); 	// Check for Carbon 	QDomNodeList list = e1.childNodes(); 	for (int i = 0; i < list.size(); ++i) { 		QDomElement el = list.at(i).toElement();  		if (el.attribute("xmlns") == QLatin1String("urn:xmpp:carbons:2") 		    && (el.tagName() == QLatin1String("received") || el.tagName() == QLatin1String("sent")) 		    && fromJid.compare(Jid(e1.attribute(QLatin1String("to"))), false)) { 			QDomElement el1 = el.firstChildElement(); 			if (el1.tagName() == QLatin1String("forwarded") 			    && el1.attribute(QLatin1String("xmlns")) == QLatin1String("urn:xmpp:forward:0")) { 				QDomElement el2 = el1.firstChildElement(QLatin1String("message")); 				if (!el2.isNull()) { 					forward = el2; bool JT_PushMessage::take(const QDomElement &e) 				} 			} 		} 		else if (el.tagName() == QLatin1String("forwarded") 			 && el.attribute(QLatin1String("xmlns")) == QLatin1String("urn:xmpp:forward:0")) { 			forward = el.firstChildElement(QLatin1String("message")); // currently only messages are supportted 			// TODO <delay> element support 			if (!forward.isNull()) { bool JT_PushMessage::take(const QDomElement &e) 		} 	}  	Stanza s = client()->stream().createStanza(addCorrectNS(forward.isNull()? e1 : forward)); 	if(s.isNull()) { 		//printf("take: bad stanza??\n"); bool JT_PushMessage::take(const QDomElement &e) 		return false; 	} 	if (!forward.isNull()) { 		m.setForwardedFrom(fromJid); 		m.setCarbonDirection(cd); 	} 

Patch:
patch: @@ -888,14 +888,18 @@ bool JT_PushMessage::take(const QDomElement &e)
 	QDomElement forward;
 	Message::CarbonDir cd = Message::NoCarbon;
 
+	Jid fromJid = Jid(e1.attribute(QLatin1String("from")));
 	// Check for Carbon
 	QDomNodeList list = e1.childNodes();
 	for (int i = 0; i < list.size(); ++i) {
 		QDomElement el = list.at(i).toElement();
 
-		if (el.attribute("xmlns") == QLatin1String("urn:xmpp:carbons:2") && (el.tagName() == QLatin1String("received") || el.tagName() == QLatin1String("sent"))) {
+		if (el.attribute("xmlns") == QLatin1String("urn:xmpp:carbons:2")
+		    && (el.tagName() == QLatin1String("received") || el.tagName() == QLatin1String("sent"))
+		    && fromJid.compare(Jid(e1.attribute(QLatin1String("to"))), false)) {
 			QDomElement el1 = el.firstChildElement();
-			if (el1.tagName() == QLatin1String("forwarded") && el1.attribute(QLatin1String("xmlns")) == QLatin1String("urn:xmpp:forward:0")) {
+			if (el1.tagName() == QLatin1String("forwarded")
+			    && el1.attribute(QLatin1String("xmlns")) == QLatin1String("urn:xmpp:forward:0")) {
 				QDomElement el2 = el1.firstChildElement(QLatin1String("message"));
 				if (!el2.isNull()) {
 					forward = el2;
@@ -904,7 +908,8 @@ bool JT_PushMessage::take(const QDomElement &e)
 				}
 			}
 		}
-		else if (el.tagName() == QLatin1String("forwarded") && el.attribute(QLatin1String("xmlns")) == QLatin1String("urn:xmpp:forward:0")) {
+		else if (el.tagName() == QLatin1String("forwarded")
+			 && el.attribute(QLatin1String("xmlns")) == QLatin1String("urn:xmpp:forward:0")) {
 			forward = el.firstChildElement(QLatin1String("message")); // currently only messages are supportted
 			// TODO <delay> element support
 			if (!forward.isNull()) {
@@ -913,7 +918,6 @@ bool JT_PushMessage::take(const QDomElement &e)
 		}
 	}
 
-	QString from = e1.attribute(QLatin1String("from"));
 	Stanza s = client()->stream().createStanza(addCorrectNS(forward.isNull()? e1 : forward));
 	if(s.isNull()) {
 		//printf("take: bad stanza??\n");
@@ -926,7 +930,7 @@ bool JT_PushMessage::take(const QDomElement &e)
 		return false;
 	}
 	if (!forward.isNull()) {
-		m.setForwardedFrom(Jid(from));
+		m.setForwardedFrom(fromJid);
 		m.setCarbonDirection(cd);
 	}
path: src/xmpp/xmpp-im/xmpp_tasks.cpp