From aacf50316db8a2a29beed493e5e93248ee5aa91d Mon Sep 17 00:00:00 2001 From: Moxie Marlinspike Date: Wed, 2 Aug 2017 11:04:10 -0700 Subject: [PATCH] Escape addresses in thread, recipient pref, and group databases Fixes #6847 // FREEBIE --- .../securesms/database/Address.java | 31 +++++++--- .../securesms/database/GroupDatabase.java | 18 ++---- .../database/RecipientPreferenceDatabase.java | 42 ++++++-------- .../securesms/database/ThreadDatabase.java | 40 +++++-------- .../securesms/util/DelimiterUtil.java | 21 +++++++ src/org/thoughtcrime/securesms/util/Util.java | 14 ----- .../securesms/util/DelimiterUtilTest.java | 56 +++++++++++++++++++ 7 files changed, 139 insertions(+), 83 deletions(-) create mode 100644 src/org/thoughtcrime/securesms/util/DelimiterUtil.java create mode 100644 test/unitTest/java/org/thoughtcrime/securesms/util/DelimiterUtilTest.java diff --git a/src/org/thoughtcrime/securesms/database/Address.java b/src/org/thoughtcrime/securesms/database/Address.java index d90e669c75..8902daea93 100644 --- a/src/org/thoughtcrime/securesms/database/Address.java +++ b/src/org/thoughtcrime/securesms/database/Address.java @@ -14,11 +14,14 @@ import com.google.i18n.phonenumbers.PhoneNumberUtil; import com.google.i18n.phonenumbers.Phonenumber; import com.google.i18n.phonenumbers.ShortNumberInfo; +import org.thoughtcrime.securesms.util.DelimiterUtil; import org.thoughtcrime.securesms.util.GroupUtil; import org.thoughtcrime.securesms.util.NumberUtil; import org.thoughtcrime.securesms.util.TextSecurePreferences; import org.thoughtcrime.securesms.util.Util; +import java.util.Arrays; +import java.util.Collections; import java.util.HashSet; import java.util.LinkedList; import java.util.List; @@ -52,23 +55,35 @@ public class Address implements Parcelable, Comparable
{ this(in.readString()); } - public static Address fromSerialized(@NonNull String serialized) { + public static @NonNull Address fromSerialized(@NonNull String serialized) { return new Address(serialized); } - public static List
fromSerializedList(@NonNull String serialized, @NonNull String delimiter) { - List elements = Util.split(serialized, delimiter); - List
addresses = new LinkedList<>(); + public static Address fromExternal(@NonNull Context context, @Nullable String external) { + return new Address(new ExternalAddressFormatter(TextSecurePreferences.getLocalNumber(context)).format(external)); + } - for (String element : elements) { - addresses.add(Address.fromSerialized(element)); + public static @NonNull List
fromSerializedList(@NonNull String serialized, char delimiter) { + String[] escapedAddresses = DelimiterUtil.split(serialized, delimiter); + List
addresses = new LinkedList<>(); + + for (String escapedAddress : escapedAddresses) { + addresses.add(Address.fromSerialized(DelimiterUtil.unescape(escapedAddress, delimiter))); } return addresses; } - public static Address fromExternal(@NonNull Context context, @Nullable String external) { - return new Address(new ExternalAddressFormatter(TextSecurePreferences.getLocalNumber(context)).format(external)); + public static @NonNull String toSerializedList(@NonNull List
addresses, char delimiter) { + Collections.sort(addresses); + + List escapedAddresses = new LinkedList<>(); + + for (Address address : addresses) { + escapedAddresses.add(DelimiterUtil.escape(address.serialize(), delimiter)); + } + + return Util.join(escapedAddresses, delimiter + ""); } public static Address[] fromParcelable(Parcelable[] parcelables) { diff --git a/src/org/thoughtcrime/securesms/database/GroupDatabase.java b/src/org/thoughtcrime/securesms/database/GroupDatabase.java index bad4ccdbc4..4a44a96850 100644 --- a/src/org/thoughtcrime/securesms/database/GroupDatabase.java +++ b/src/org/thoughtcrime/securesms/database/GroupDatabase.java @@ -17,7 +17,6 @@ import org.thoughtcrime.securesms.recipients.RecipientFactory; import org.thoughtcrime.securesms.recipients.Recipients; import org.thoughtcrime.securesms.util.BitmapUtil; import org.thoughtcrime.securesms.util.GroupUtil; -import org.thoughtcrime.securesms.util.TextSecurePreferences; import org.thoughtcrime.securesms.util.Util; import org.whispersystems.signalservice.api.messages.SignalServiceAttachmentPointer; @@ -120,7 +119,7 @@ public class GroupDatabase extends Database { ContentValues contentValues = new ContentValues(); contentValues.put(GROUP_ID, GroupUtil.getEncodedId(groupId)); contentValues.put(TITLE, title); - contentValues.put(MEMBERS, Util.join(members, ",")); + contentValues.put(MEMBERS, Address.toSerializedList(members, ',')); if (avatar != null) { contentValues.put(AVATAR_ID, avatar.getId()); @@ -185,7 +184,7 @@ public class GroupDatabase extends Database { public void updateMembers(byte[] id, List
members) { ContentValues contents = new ContentValues(); - contents.put(MEMBERS, Util.join(members, ",")); + contents.put(MEMBERS, Address.toSerializedList(members, ',')); contents.put(ACTIVE, 1); databaseHelper.getWritableDatabase().update(TABLE_NAME, contents, GROUP_ID + " = ?", @@ -197,7 +196,7 @@ public class GroupDatabase extends Database { currentMembers.remove(source); ContentValues contents = new ContentValues(); - contents.put(MEMBERS, Util.join(currentMembers, ",")); + contents.put(MEMBERS, Address.toSerializedList(currentMembers, ',')); databaseHelper.getWritableDatabase().update(TABLE_NAME, contents, GROUP_ID + " = ?", new String[] {GroupUtil.getEncodedId(id)}); @@ -213,13 +212,8 @@ public class GroupDatabase extends Database { null, null, null); if (cursor != null && cursor.moveToFirst()) { - List
results = new LinkedList<>(); - - for (String member : Util.split(cursor.getString(cursor.getColumnIndexOrThrow(MEMBERS)), ",")) { - results.add(Address.fromSerialized(member)); - } - - return results; + String serializedMembers = cursor.getString(cursor.getColumnIndexOrThrow(MEMBERS)); + return Address.fromSerializedList(serializedMembers, ','); } return new LinkedList<>(); @@ -307,7 +301,7 @@ public class GroupDatabase extends Database { { this.id = id; this.title = title; - this.members = Address.fromSerializedList(members, ","); + this.members = Address.fromSerializedList(members, ','); this.avatar = avatar; this.avatarId = avatarId; this.avatarKey = avatarKey; diff --git a/src/org/thoughtcrime/securesms/database/RecipientPreferenceDatabase.java b/src/org/thoughtcrime/securesms/database/RecipientPreferenceDatabase.java index 5ab76b329a..2734f024f2 100644 --- a/src/org/thoughtcrime/securesms/database/RecipientPreferenceDatabase.java +++ b/src/org/thoughtcrime/securesms/database/RecipientPreferenceDatabase.java @@ -14,10 +14,10 @@ import org.greenrobot.eventbus.EventBus; import org.thoughtcrime.securesms.color.MaterialColor; import org.thoughtcrime.securesms.recipients.RecipientFactory; import org.thoughtcrime.securesms.recipients.Recipients; -import org.thoughtcrime.securesms.util.Util; import org.whispersystems.libsignal.util.guava.Optional; import java.util.Arrays; +import java.util.List; public class RecipientPreferenceDatabase extends Database { @@ -86,14 +86,12 @@ public class RecipientPreferenceDatabase extends Database { } public Optional getRecipientsPreferences(@NonNull Address[] addresses) { - Arrays.sort(addresses); - SQLiteDatabase database = databaseHelper.getReadableDatabase(); Cursor cursor = null; try { cursor = database.query(TABLE_NAME, null, ADDRESSES + " = ?", - new String[] {Util.join(addresses, " ")}, + new String[] {Address.toSerializedList(Arrays.asList(addresses), ' ')}, null, null, null); if (cursor != null && cursor.moveToNext()) { @@ -187,11 +185,12 @@ public class RecipientPreferenceDatabase extends Database { database.beginTransaction(); - int updated = database.update(TABLE_NAME, contentValues, ADDRESSES + " = ?", - new String[] {Util.join(recipients.getAddresses(), " ")}); + List
addresses = recipients.getAddressesList(); + String serializedAddresses = Address.toSerializedList(addresses, ' '); + int updated = database.update(TABLE_NAME, contentValues, ADDRESSES + " = ?", new String[]{serializedAddresses}); if (updated < 1) { - contentValues.put(ADDRESSES, Util.join(recipients.getAddresses(), " ")); + contentValues.put(ADDRESSES, serializedAddresses); database.insert(TABLE_NAME, null, contentValues); } @@ -211,13 +210,13 @@ public class RecipientPreferenceDatabase extends Database { private final int defaultSubscriptionId; private final int expireMessages; - public RecipientsPreferences(boolean blocked, long muteUntil, - @NonNull VibrateState vibrateState, - @Nullable Uri notification, - @Nullable MaterialColor color, - boolean seenInviteReminder, - int defaultSubscriptionId, - int expireMessages) + RecipientsPreferences(boolean blocked, long muteUntil, + @NonNull VibrateState vibrateState, + @Nullable Uri notification, + @Nullable MaterialColor color, + boolean seenInviteReminder, + int defaultSubscriptionId, + int expireMessages) { this.blocked = blocked; this.muteUntil = muteUntil; @@ -267,21 +266,16 @@ public class RecipientPreferenceDatabase extends Database { private final Context context; private final Cursor cursor; - public BlockedReader(Context context, Cursor cursor) { + BlockedReader(Context context, Cursor cursor) { this.context = context; this.cursor = cursor; } public @NonNull Recipients getCurrent() { - String serialized = cursor.getString(cursor.getColumnIndexOrThrow(ADDRESSES)); - String[] addresses = serialized.split(" "); - Address[] addressList = new Address[addresses.length]; + String serialized = cursor.getString(cursor.getColumnIndexOrThrow(ADDRESSES)); + List
addressList = Address.fromSerializedList(serialized, ' '); - for (int i=0;i 1) contentValues.put(TYPE, distributionType); @@ -288,7 +289,7 @@ public class ThreadDatabase extends Database { int i= 0; for (Address address : addresses) { - selectionArgs[i++] = address.serialize(); + selectionArgs[i++] = DelimiterUtil.escape(address.serialize(), ' '); } cursors.add(db.query(TABLE_NAME, null, selection, selectionArgs, null, null, DATE + " DESC")); @@ -410,11 +411,11 @@ public class ThreadDatabase extends Database { } public long getThreadIdIfExistsFor(Recipients recipients) { - Address[] addresses = recipients.getAddresses(); - SQLiteDatabase db = databaseHelper.getReadableDatabase(); - String where = ADDRESSES + " = ?"; - String[] recipientsArg = new String[] {Util.join(addresses, " ")}; - Cursor cursor = null; + List
addresses = Arrays.asList(recipients.getAddresses()); + SQLiteDatabase db = databaseHelper.getReadableDatabase(); + String where = ADDRESSES + " = ?"; + String[] recipientsArg = new String[]{Address.toSerializedList(addresses, ' ')}; + Cursor cursor = null; try { cursor = db.query(TABLE_NAME, new String[]{ID}, where, recipientsArg, null, null, null); @@ -437,7 +438,7 @@ public class ThreadDatabase extends Database { Address[] addresses = recipients.getAddresses(); SQLiteDatabase db = databaseHelper.getReadableDatabase(); String where = ADDRESSES + " = ?"; - String[] recipientsArg = new String[]{Util.join(addresses, " ")}; + String[] recipientsArg = new String[]{Address.toSerializedList(Arrays.asList(addresses), ' ')}; Cursor cursor = null; try { @@ -462,8 +463,8 @@ public class ThreadDatabase extends Database { cursor = db.query(TABLE_NAME, null, ID + " = ?", new String[] {threadId+""}, null, null, null); if (cursor != null && cursor.moveToFirst()) { - Address[] addresses = getAddressesFromSerialized(cursor.getString(cursor.getColumnIndexOrThrow(ADDRESSES))); - return RecipientFactory.getRecipientsFor(context, addresses, false); + List
addresses = Address.fromSerializedList(cursor.getString(cursor.getColumnIndexOrThrow(ADDRESSES)), ' '); + return RecipientFactory.getRecipientsFor(context, addresses.toArray(new Address[0]), false); } } finally { if (cursor != null) @@ -527,17 +528,6 @@ public class ThreadDatabase extends Database { return thumbnail != null ? thumbnail.getThumbnailUri() : null; } - private @NonNull Address[] getAddressesFromSerialized(String serializedAddresses) { - String[] serializedAddressParts = serializedAddresses.split(" "); - Address[] addresses = new Address[serializedAddressParts.length]; - - for (int i=0;i addresses = Address.fromSerializedList(cursor.getString(cursor.getColumnIndexOrThrow(ThreadDatabase.ADDRESSES)), ' '); + Recipients recipients = RecipientFactory.getRecipientsFor(context, addresses.toArray(new Address[0]), true); DisplayRecord.Body body = getPlaintextBody(cursor); long date = cursor.getLong(cursor.getColumnIndexOrThrow(ThreadDatabase.DATE)); diff --git a/src/org/thoughtcrime/securesms/util/DelimiterUtil.java b/src/org/thoughtcrime/securesms/util/DelimiterUtil.java new file mode 100644 index 0000000000..174749e456 --- /dev/null +++ b/src/org/thoughtcrime/securesms/util/DelimiterUtil.java @@ -0,0 +1,21 @@ +package org.thoughtcrime.securesms.util; + + +import java.util.regex.Pattern; + +public class DelimiterUtil { + + public static String escape(String value, char delimiter) { + return value.replace("" + delimiter, "\\" + delimiter); + } + + public static String unescape(String value, char delimiter) { + return value.replace("\\" + delimiter, "" + delimiter); + } + + public static String[] split(String value, char delimiter) { + String regex = "(? list, String delimiter) { - return join(list.toArray(new Address[0]), delimiter); - } - - public static String join(Address[] list, String delimiter) { - List stringList = new LinkedList<>(); - - for (Address address : list) { - stringList.add(address.serialize()); - } - - return join(stringList, delimiter); - } - public static String join(String[] list, String delimiter) { return join(Arrays.asList(list), delimiter); } diff --git a/test/unitTest/java/org/thoughtcrime/securesms/util/DelimiterUtilTest.java b/test/unitTest/java/org/thoughtcrime/securesms/util/DelimiterUtilTest.java new file mode 100644 index 0000000000..8d7a25e88e --- /dev/null +++ b/test/unitTest/java/org/thoughtcrime/securesms/util/DelimiterUtilTest.java @@ -0,0 +1,56 @@ +package org.thoughtcrime.securesms.util; + + +import org.junit.Before; +import org.junit.Test; + +import static junit.framework.Assert.assertEquals; + +public class DelimiterUtilTest { + + @Before + public void setup() {} + + @Test + public void testEscape() { + assertEquals(DelimiterUtil.escape("MTV Music", ' '), "MTV\\ Music"); + assertEquals(DelimiterUtil.escape("MTV Music", ' '), "MTV\\ \\ Music"); + + assertEquals(DelimiterUtil.escape("MTV,Music", ','), "MTV\\,Music"); + assertEquals(DelimiterUtil.escape("MTV,,Music", ','), "MTV\\,\\,Music"); + + assertEquals(DelimiterUtil.escape("MTV Music", '+'), "MTV Music"); + } + + @Test + public void testSplit() { + String[] parts = DelimiterUtil.split("MTV\\ Music", ' '); + assertEquals(parts.length, 1); + assertEquals(parts[0], "MTV\\ Music"); + + parts = DelimiterUtil.split("MTV Music", ' '); + assertEquals(parts.length, 2); + assertEquals(parts[0], "MTV"); + assertEquals(parts[1], "Music"); + } + + @Test + public void testEscapeSplit() { + String input = "MTV Music"; + String intermediate = DelimiterUtil.escape(input, ' '); + String[] parts = DelimiterUtil.split(intermediate, ' '); + + assertEquals(parts.length, 1); + assertEquals(parts[0], "MTV\\ Music"); + assertEquals(DelimiterUtil.unescape(parts[0], ' '), "MTV Music"); + + input = "MTV\\ Music"; + intermediate = DelimiterUtil.escape(input, ' '); + parts = DelimiterUtil.split(intermediate, ' '); + + assertEquals(parts.length, 1); + assertEquals(parts[0], "MTV\\\\ Music"); + assertEquals(DelimiterUtil.unescape(parts[0], ' '), "MTV\\ Music"); + } + +}