Merge tag '2.1.3'

CHANGELOG.md

@@ -1,5 +1,8 @@
 # Changelog
 
+### Version 2.1.3
+* Do not process stanzas with invalid JIDs
+
 ### Version 2.1.2
 * Fixed avatars not being displayed on new installs
 

build.gradle

@@ -66,8 +66,8 @@ android {
     defaultConfig {
         minSdkVersion 19
         targetSdkVersion 25
-        versionCode 266
+        versionCode 267
-        versionName "2.1.2"
+        versionName "2.1.3"
         archivesBaseName += "-$versionName"
         applicationId "eu.siacs.conversations"
         resValue "string", "applicationId", applicationId

src/main/java/eu/siacs/conversations/entities/Bookmark.java

@@ -11,6 +11,7 @@ import java.util.Locale;
 
 import eu.siacs.conversations.utils.UIHelper;
 import eu.siacs.conversations.xml.Element;
+import eu.siacs.conversations.xmpp.InvalidJid;
 import rocks.xmpp.addr.Jid;
 
 public class Bookmark extends Element implements ListItem {
@@ -35,7 +36,7 @@ public class Bookmark extends Element implements ListItem {
 		Bookmark bookmark = new Bookmark(account);
 		bookmark.setAttributes(element.getAttributes());
 		bookmark.setChildren(element.getChildren());
-		bookmark.jid =  bookmark.getAttributeAsJid("jid");
+		bookmark.jid = InvalidJid.getNullForInvalid(bookmark.getAttributeAsJid("jid"));
 		return bookmark;
 	}
 

src/main/java/eu/siacs/conversations/parser/AbstractParser.java

@@ -11,6 +11,7 @@ import eu.siacs.conversations.entities.Conversation;
 import eu.siacs.conversations.entities.MucOptions;
 import eu.siacs.conversations.services.XmppConnectionService;
 import eu.siacs.conversations.xml.Element;
+import eu.siacs.conversations.xmpp.InvalidJid;
 import eu.siacs.conversations.xmpp.stanzas.AbstractStanza;
 import rocks.xmpp.addr.Jid;
 
@@ -37,7 +38,7 @@ public abstract class AbstractParser {
 		}
 		for(Element child : element.getChildren()) {
 			if ("delay".equals(child.getName()) && "urn:xmpp:delay".equals(child.getNamespace())) {
-				final Jid f = to == null ? null : child.getAttributeAsJid("from");
+				final Jid f = to == null ? null : InvalidJid.getNullForInvalid(child.getAttributeAsJid("from"));
 				if (f != null && (to.asBareJid().equals(f) || to.getDomain().equals(f.toString()))) {
 					continue;
 				}
@@ -115,7 +116,9 @@ public abstract class AbstractParser {
 		}
 		Jid realJid = item.getAttributeAsJid("jid");
 		MucOptions.User user = new MucOptions.User(conference.getMucOptions(), fullJid);
+		if (InvalidJid.isValid(realJid)) {
 			user.setRealJid(realJid);
+		}
 		user.setAffiliation(affiliation);
 		user.setRole(role);
 		return user;

src/main/java/eu/siacs/conversations/parser/IqParser.java

@@ -29,6 +29,7 @@ import eu.siacs.conversations.entities.Contact;
 import eu.siacs.conversations.services.XmppConnectionService;
 import eu.siacs.conversations.xml.Namespace;
 import eu.siacs.conversations.xml.Element;
+import eu.siacs.conversations.xmpp.InvalidJid;
 import eu.siacs.conversations.xmpp.OnIqPacketReceived;
 import eu.siacs.conversations.xmpp.OnUpdateBlocklist;
 import eu.siacs.conversations.xmpp.stanzas.IqPacket;
@@ -47,7 +48,7 @@ public class IqParser extends AbstractParser implements OnIqPacketReceived {
 		}
 		for (final Element item : query.getChildren()) {
 			if (item.getName().equals("item")) {
-				final Jid jid = item.getAttributeAsJid("jid");
+				final Jid jid = InvalidJid.getNullForInvalid(item.getAttributeAsJid("jid"));
 				if (jid == null) {
 					continue;
 				}
@@ -310,7 +311,7 @@ public class IqParser extends AbstractParser implements OnIqPacketReceived {
 				// Create a collection of Jids from the packet
 				for (final Element item : items) {
 					if (item.getName().equals("item")) {
-						final Jid jid = item.getAttributeAsJid("jid");
+						final Jid jid = InvalidJid.getNullForInvalid(item.getAttributeAsJid("jid"));
 						if (jid != null) {
 							jids.add(jid);
 						}
@@ -344,7 +345,7 @@ public class IqParser extends AbstractParser implements OnIqPacketReceived {
 				final Collection<Jid> jids = new ArrayList<>(items.size());
 				for (final Element item : items) {
 					if (item.getName().equals("item")) {
-						final Jid jid = item.getAttributeAsJid("jid");
+						final Jid jid = InvalidJid.getNullForInvalid(item.getAttributeAsJid("jid"));
 						if (jid != null) {
 							jids.add(jid);
 						}

src/main/java/eu/siacs/conversations/parser/MessageParser.java

@@ -32,6 +32,7 @@ import eu.siacs.conversations.services.XmppConnectionService;
 import eu.siacs.conversations.utils.CryptoHelper;
 import eu.siacs.conversations.xml.Namespace;
 import eu.siacs.conversations.xml.Element;
+import eu.siacs.conversations.xmpp.InvalidJid;
 import eu.siacs.conversations.xmpp.OnMessagePacketReceived;
 import eu.siacs.conversations.xmpp.chatstate.ChatState;
 import eu.siacs.conversations.xmpp.pep.Avatar;
@@ -64,7 +65,7 @@ public class MessageParser extends AbstractParser implements OnMessagePacketRece
 		for (Element child : packet.getChildren()) {
 			if (child.getName().equals("stanza-id")
 					&& Namespace.STANZA_IDS.equals(child.getNamespace())
-					&& by.equals(child.getAttributeAsJid("by"))) {
+					&& by.equals(InvalidJid.getNullForInvalid(child.getAttributeAsJid("by")))) {
 				return child.getAttribute("id");
 			}
 		}
@@ -73,7 +74,7 @@ public class MessageParser extends AbstractParser implements OnMessagePacketRece
 
 	private static Jid getTrueCounterpart(Element mucUserElement, Jid fallback) {
 		final Element item = mucUserElement == null ? null : mucUserElement.findChild("item");
-		Jid result = item == null ? null : item.getAttributeAsJid("jid");
+		Jid result = item == null ? null : InvalidJid.getNullForInvalid(item.getAttributeAsJid("jid"));
 		return result != null ? result : fallback;
 	}
 
@@ -139,17 +140,25 @@ public class MessageParser extends AbstractParser implements OnMessagePacketRece
 		if (x != null) {
 			Element invite = x.findChild("invite");
 			if (invite != null) {
-				Element pw = x.findChild("password");
+				String password = x.findChildContent("password");
-				Jid from = invite.getAttributeAsJid("from");
+				Jid from = InvalidJid.getNullForInvalid(invite.getAttributeAsJid("from"));
 				Contact contact = from == null ? null : account.getRoster().getContact(from);
-				return new Invite(message.getAttributeAsJid("from"), pw != null ? pw.getContent() : null, contact);
+				Jid room = InvalidJid.getNullForInvalid(message.getAttributeAsJid("from"));
+				if (room == null) {
+					return null;
+				}
+				return new Invite(room, password, contact);
 			}
 		} else {
 			x = message.findChild("x", "jabber:x:conference");
 			if (x != null) {
-				Jid from = message.getAttributeAsJid("from");
+				Jid from = InvalidJid.getNullForInvalid(message.getAttributeAsJid("from"));
 				Contact contact = from == null ? null : account.getRoster().getContact(from);
-				return new Invite(x.getAttributeAsJid("jid"), x.getAttribute("password"), contact);
+				Jid room = InvalidJid.getNullForInvalid(x.getAttributeAsJid("jid"));
+				if (room == null) {
+					return null;
+				}
+				return new Invite(room, x.getAttribute("password"), contact);
 			}
 		}
 		return null;
@@ -697,7 +706,7 @@ public class MessageParser extends AbstractParser implements OnMessagePacketRece
 		Element displayed = packet.findChild("displayed", "urn:xmpp:chat-markers:0");
 		if (displayed != null) {
 			final String id = displayed.getAttribute("id");
-			final Jid sender = displayed.getAttributeAsJid("sender");
+			final Jid sender = InvalidJid.getNullForInvalid(displayed.getAttributeAsJid("sender"));
 			if (packet.fromAccount(account) && !selfAddressed) {
 				dismissNotification(account, counterpart, query);
 			} else if (isTypeGroupChat) {

src/main/java/eu/siacs/conversations/parser/PresenceParser.java

@@ -22,6 +22,7 @@ import eu.siacs.conversations.generator.PresenceGenerator;
 import eu.siacs.conversations.services.XmppConnectionService;
 import eu.siacs.conversations.xml.Element;
 import eu.siacs.conversations.xml.Namespace;
+import eu.siacs.conversations.xmpp.InvalidJid;
 import eu.siacs.conversations.xmpp.OnPresencePacketReceived;
 import eu.siacs.conversations.xmpp.pep.Avatar;
 import eu.siacs.conversations.xmpp.stanzas.PresencePacket;
@@ -69,7 +70,7 @@ public class PresenceParser extends AbstractParser implements
 					if (item != null && !from.isBareJid()) {
 						mucOptions.setError(MucOptions.Error.NONE);
 						MucOptions.User user = parseItem(conversation, item, from);
-						if (codes.contains(MucOptions.STATUS_CODE_SELF_PRESENCE) || (codes.contains(MucOptions.STATUS_CODE_ROOM_CREATED) && jid.equals(item.getAttributeAsJid("jid")))) {
+						if (codes.contains(MucOptions.STATUS_CODE_SELF_PRESENCE) || (codes.contains(MucOptions.STATUS_CODE_ROOM_CREATED) && jid.equals(InvalidJid.getNullForInvalid(item.getAttributeAsJid("jid"))))) {
 							if (mucOptions.setOnline()) {
 								mXmppConnectionService.getAvatarService().clear(mucOptions);
 							}

src/main/java/eu/siacs/conversations/xml/Element.java

@@ -11,6 +11,7 @@ import java.util.Locale;
 
 import eu.siacs.conversations.Config;
 import eu.siacs.conversations.utils.XmlHelper;
+import eu.siacs.conversations.xmpp.InvalidJid;
 import rocks.xmpp.addr.Jid;
 
 public class Element {
@@ -153,7 +154,7 @@ public class Element {
 			try {
 				return Jid.ofEscaped(jid);
 			} catch (final IllegalArgumentException e) {
-				return null;
+				return new InvalidJid(jid);
 			}
 		}
 		return null;

src/main/java/eu/siacs/conversations/xmpp/InvalidJid.java

@@ -0,0 +1,140 @@
+/*
+ * Copyright (c) 2018, Daniel Gultsch All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without modification,
+ * are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice, this
+ * list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright notice,
+ * this list of conditions and the following disclaimer in the documentation and/or
+ * other materials provided with the distribution.
+ *
+ * 3. Neither the name of the copyright holder nor the names of its contributors
+ * may be used to endorse or promote products derived from this software without
+ * specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR
+ * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+ * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+package eu.siacs.conversations.xmpp;
+
+import android.support.annotation.NonNull;
+
+import rocks.xmpp.addr.Jid;
+
+public class InvalidJid implements Jid {
+
+	private final String value;
+
+	public InvalidJid(String jid) {
+		this.value = jid;
+	}
+
+	@Override
+	@NonNull
+	public String toString() {
+		return value;
+	}
+
+	@Override
+	public boolean isFullJid() {
+		throw new AssertionError("Not implemented");
+	}
+
+	@Override
+	public boolean isBareJid() {
+		throw new AssertionError("Not implemented");
+	}
+
+	@Override
+	public Jid asBareJid() {
+		throw new AssertionError("Not implemented");
+	}
+
+	@Override
+	public Jid withLocal(CharSequence charSequence) {
+		throw new AssertionError("Not implemented");
+	}
+
+	@Override
+	public Jid withResource(CharSequence charSequence) {
+		throw new AssertionError("Not implemented");
+	}
+
+	@Override
+	public Jid atSubdomain(CharSequence charSequence) {
+		throw new AssertionError("Not implemented");
+	}
+
+	@Override
+	public String getLocal() {
+		throw new AssertionError("Not implemented");
+	}
+
+	@Override
+	public String getEscapedLocal() {
+		throw new AssertionError("Not implemented");
+	}
+
+	@Override
+	public String getDomain() {
+		throw new AssertionError("Not implemented");
+	}
+
+	@Override
+	public String getResource() {
+		throw new AssertionError("Not implemented");
+	}
+
+	@Override
+	public String toEscapedString() {
+		throw new AssertionError("Not implemented");
+	}
+
+	@Override
+	public int length() {
+		return value.length();
+	}
+
+	@Override
+	public char charAt(int index) {
+		return value.charAt(index);
+	}
+
+	@Override
+	public CharSequence subSequence(int start, int end) {
+		return value.subSequence(start, end);
+	}
+
+	@Override
+	public int compareTo(@NonNull Jid o) {
+		throw new AssertionError("Not implemented");
+	}
+
+	public static Jid getNullForInvalid(Jid jid) {
+		if (jid != null && jid instanceof InvalidJid) {
+			return null;
+		} else {
+			return jid;
+		}
+	}
+
+	public static boolean isValid(Jid jid) {
+		if (jid != null && jid instanceof InvalidJid) {
+			return false;
+		} else {
+			return true;
+		}
+	}
+}

src/main/java/eu/siacs/conversations/xmpp/XmppConnection.java

@@ -5,6 +5,7 @@ import android.graphics.Bitmap;
 import android.graphics.BitmapFactory;
 import android.os.SystemClock;
 import android.security.KeyChain;
+import android.support.annotation.NonNull;
 import android.util.Base64;
 import android.util.Log;
 import android.util.Pair;
@@ -179,6 +180,25 @@ public class XmppConnection implements Runnable {
 		mXmppConnectionService = service;
 	}
 
+	private static void fixResource(Context context, Account account) {
+		String resource = account.getResource();
+		int fixedPartLength = context.getString(R.string.app_name).length() + 1; //include the trailing dot
+		int randomPartLength = 4; // 3 bytes
+		if (resource != null && resource.length() > fixedPartLength + randomPartLength) {
+			if (validBase64(resource.substring(fixedPartLength, fixedPartLength + randomPartLength))) {
+				account.setResource(resource.substring(0, fixedPartLength + randomPartLength));
+			}
+		}
+	}
+
+	private static boolean validBase64(String input) {
+		try {
+			return Base64.decode(input, Base64.URL_SAFE).length == 3;
+		} catch (Throwable throwable) {
+			return false;
+		}
+	}
+
 	protected void changeStatus(final Account.State nextStatus) {
 		synchronized (this) {
 			if (Thread.currentThread().isInterrupted()) {
@@ -661,8 +681,8 @@ public class XmppConnection implements Runnable {
 		}
 	}
 
-	private Element processPacket(final Tag currentTag, final int packetType)
+	private @NonNull
-			throws XmlPullParserException, IOException {
+	Element processPacket(final Tag currentTag, final int packetType) throws XmlPullParserException, IOException {
 		Element element;
 		switch (packetType) {
 			case PACKET_IQ:
@@ -675,7 +695,7 @@ public class XmppConnection implements Runnable {
 				element = new PresencePacket();
 				break;
 			default:
-				return null;
+				throw new AssertionError("Should never encounter invalid type");
 		}
 		element.setAttributes(currentTag.getAttributes());
 		Tag nextTag = tagReader.readTag();
@@ -718,11 +738,10 @@ public class XmppConnection implements Runnable {
 
 	private void processIq(final Tag currentTag) throws XmlPullParserException, IOException {
 		final IqPacket packet = (IqPacket) processPacket(currentTag, PACKET_IQ);
-
+		if (!packet.valid()) {
-		if (packet.getId() == null) {
+			Log.e(Config.LOGTAG, "encountered invalid iq from='" + packet.getFrom() + "' to='" + packet.getTo() + "'");
-			return; // an iq packet without id is definitely invalid
+			return;
 		}
-
 		if (packet instanceof JinglePacket) {
 			if (this.jingleListener != null) {
 				this.jingleListener.onJinglePacketReceived(account, (JinglePacket) packet);
@@ -764,11 +783,19 @@ public class XmppConnection implements Runnable {
 
 	private void processMessage(final Tag currentTag) throws XmlPullParserException, IOException {
 		final MessagePacket packet = (MessagePacket) processPacket(currentTag, PACKET_MESSAGE);
+		if (!packet.valid()) {
+			Log.e(Config.LOGTAG, "encountered invalid message from='" + packet.getFrom() + "' to='" + packet.getTo() + "'");
+			return;
+		}
 		this.messageListener.onMessagePacketReceived(account, packet);
 	}
 
 	private void processPresence(final Tag currentTag) throws XmlPullParserException, IOException {
 		PresencePacket packet = (PresencePacket) processPacket(currentTag, PACKET_PRESENCE);
+		if (!packet.valid()) {
+			Log.e(Config.LOGTAG, "encountered invalid presence from='" + packet.getFrom() + "' to='" + packet.getTo() + "'");
+			return;
+		}
 		this.presenceListener.onPresencePacketReceived(account, packet);
 	}
 
@@ -1225,7 +1252,7 @@ public class XmppConnection implements Runnable {
 				final List<Element> elements = packet.query().getChildren();
 				for (final Element element : elements) {
 					if (element.getName().equals("item")) {
-						final Jid jid = element.getAttributeAsJid("jid");
+						final Jid jid = InvalidJid.getNullForInvalid(element.getAttributeAsJid("jid"));
 						if (jid != null && !jid.equals(Jid.of(account.getServer()))) {
 							items.add(jid);
 						}
@@ -1298,25 +1325,6 @@ public class XmppConnection implements Runnable {
 		return mXmppConnectionService.getString(R.string.app_name) + '.' + nextRandomId(true);
 	}
 
-	private static void fixResource(Context context, Account account) {
-		String resource = account.getResource();
-		int fixedPartLength = context.getString(R.string.app_name).length() + 1; //include the trailing dot
-		int randomPartLength = 4; // 3 bytes
-		if (resource != null && resource.length() > fixedPartLength + randomPartLength) {
-			if (validBase64(resource.substring(fixedPartLength,fixedPartLength + randomPartLength))) {
-				account.setResource(resource.substring(0,fixedPartLength + randomPartLength));
-			}
-		}
-	}
-
-	private static boolean validBase64(String input) {
-		try {
-			return Base64.decode(input, Base64.URL_SAFE).length == 3;
-		} catch (Throwable throwable) {
-			return false;
-		}
-	}
-
 	private String nextRandomId() {
 		return nextRandomId(false);
 	}

src/main/java/eu/siacs/conversations/xmpp/jingle/JingleCandidate.java

@@ -4,6 +4,7 @@ import java.util.ArrayList;
 import java.util.List;
 
 import eu.siacs.conversations.xml.Element;
+import eu.siacs.conversations.xmpp.InvalidJid;
 import rocks.xmpp.addr.Jid;
 
 public class JingleCandidate {
@@ -108,7 +109,7 @@ public class JingleCandidate {
 		JingleCandidate parsedCandidate = new JingleCandidate(
 				candidate.getAttribute("cid"), false);
 		parsedCandidate.setHost(candidate.getAttribute("host"));
-		parsedCandidate.setJid(candidate.getAttributeAsJid("jid"));
+		parsedCandidate.setJid(InvalidJid.getNullForInvalid(candidate.getAttributeAsJid("jid")));
 		parsedCandidate.setType(candidate.getAttribute("type"));
 		parsedCandidate.setPriority(Integer.parseInt(candidate
 				.getAttribute("priority")));

src/main/java/eu/siacs/conversations/xmpp/stanzas/AbstractAcknowledgeableStanza.java

@@ -1,6 +1,8 @@
 package eu.siacs.conversations.xmpp.stanzas;
 
 import eu.siacs.conversations.xml.Element;
+import eu.siacs.conversations.xmpp.InvalidJid;
+import rocks.xmpp.addr.Jid;
 
 abstract public class AbstractAcknowledgeableStanza extends AbstractStanza {
 
@@ -28,4 +30,8 @@ abstract public class AbstractAcknowledgeableStanza extends AbstractStanza {
 		}
 		return null;
 	}
+
+	public boolean valid() {
+		return InvalidJid.isValid(getFrom()) && InvalidJid.isValid(getTo());
+	}
 }

src/main/java/eu/siacs/conversations/xmpp/stanzas/AbstractStanza.java

@@ -31,20 +31,23 @@ public class AbstractStanza extends Element {
 	}
 
 	public boolean fromServer(final Account account) {
-		return getFrom() == null
+		final Jid from = getFrom();
-			|| getFrom().equals(Jid.of(account.getServer()))
+		return from == null
-			|| getFrom().equals(account.getJid().asBareJid())
+			|| from.equals(Jid.of(account.getServer()))
-			|| getFrom().equals(account.getJid());
+			|| from.equals(account.getJid().asBareJid())
+			|| from.equals(account.getJid());
 	}
 
 	public boolean toServer(final Account account) {
-		return getTo() == null
+		final Jid to = getTo();
-			|| getTo().equals(Jid.of(account.getServer()))
+		return to == null
-			|| getTo().equals(account.getJid().asBareJid())
+			|| to.equals(Jid.of(account.getServer()))
-			|| getTo().equals(account.getJid());
+			|| to.equals(account.getJid().asBareJid())
+			|| to.equals(account.getJid());
 	}
 
 	public boolean fromAccount(final Account account) {
-		return getFrom() != null && getFrom().asBareJid().equals(account.getJid().asBareJid());
+		final Jid from = getFrom();
+		return from != null && from.asBareJid().equals(account.getJid().asBareJid());
 	}
 }

src/main/java/eu/siacs/conversations/xmpp/stanzas/IqPacket.java

@@ -66,4 +66,10 @@ public class IqPacket extends AbstractAcknowledgeableStanza {
 		return packet;
 	}
 
+	@Override
+	public boolean valid() {
+		String id = getId();
+		return id != null && super.valid();
+	}
+
 }