From ce5f3c5157340899f9c0cb186e589e9c672dc715 Mon Sep 17 00:00:00 2001 From: Moxie Marlinspike Date: Tue, 26 Nov 2013 12:44:15 -0800 Subject: [PATCH] Validate phone numbers when formatting. --- .../textsecure/directory/Directory.java | 9 ++- .../textsecure/push/PushDestination.java | 2 + .../util/InvalidNumberException.java | 7 +++ .../textsecure/util/PhoneNumberFormatter.java | 16 +++++- .../securesms/transport/PushTransport.java | 4 ++ .../transport/UniversalTransport.java | 55 ++++++++++++------- src/org/thoughtcrime/securesms/util/Util.java | 5 +- 7 files changed, 72 insertions(+), 26 deletions(-) create mode 100644 library/src/org/whispersystems/textsecure/util/InvalidNumberException.java diff --git a/library/src/org/whispersystems/textsecure/directory/Directory.java b/library/src/org/whispersystems/textsecure/directory/Directory.java index dac6f14165..85f5c6132e 100644 --- a/library/src/org/whispersystems/textsecure/directory/Directory.java +++ b/library/src/org/whispersystems/textsecure/directory/Directory.java @@ -11,6 +11,7 @@ import android.util.Log; import org.whispersystems.textsecure.push.ContactTokenDetails; import org.whispersystems.textsecure.util.Base64; +import org.whispersystems.textsecure.util.InvalidNumberException; import org.whispersystems.textsecure.util.PhoneNumberFormatter; import org.whispersystems.textsecure.util.Util; @@ -157,8 +158,12 @@ public class Directory { String rawNumber = cursor.getString(0); if (rawNumber != null) { - String e164Number = PhoneNumberFormatter.formatNumber(rawNumber, localNumber); - results.add(getToken(e164Number)); + try { + String e164Number = PhoneNumberFormatter.formatNumber(rawNumber, localNumber); + results.add(getToken(e164Number)); + } catch (InvalidNumberException e) { + Log.w("Directory", "Invalid number: " + rawNumber); + } } } diff --git a/library/src/org/whispersystems/textsecure/push/PushDestination.java b/library/src/org/whispersystems/textsecure/push/PushDestination.java index 7674d93cf8..d844f53e7e 100644 --- a/library/src/org/whispersystems/textsecure/push/PushDestination.java +++ b/library/src/org/whispersystems/textsecure/push/PushDestination.java @@ -3,6 +3,7 @@ package org.whispersystems.textsecure.push; import android.content.Context; import org.whispersystems.textsecure.directory.Directory; +import org.whispersystems.textsecure.util.InvalidNumberException; import org.whispersystems.textsecure.util.PhoneNumberFormatter; public class PushDestination { @@ -26,6 +27,7 @@ public class PushDestination { public static PushDestination create(Context context, PushServiceSocket.PushCredentials credentials, String destinationNumber) + throws InvalidNumberException { String e164destination = PhoneNumberFormatter.formatNumber(destinationNumber, credentials.getLocalNumber(context)); String relay = Directory.getInstance(context).getRelay(e164destination); diff --git a/library/src/org/whispersystems/textsecure/util/InvalidNumberException.java b/library/src/org/whispersystems/textsecure/util/InvalidNumberException.java new file mode 100644 index 0000000000..9273b40515 --- /dev/null +++ b/library/src/org/whispersystems/textsecure/util/InvalidNumberException.java @@ -0,0 +1,7 @@ +package org.whispersystems.textsecure.util; + +public class InvalidNumberException extends Throwable { + public InvalidNumberException(String s) { + super(s); + } +} diff --git a/library/src/org/whispersystems/textsecure/util/PhoneNumberFormatter.java b/library/src/org/whispersystems/textsecure/util/PhoneNumberFormatter.java index a22d37cb41..e712d7de7d 100644 --- a/library/src/org/whispersystems/textsecure/util/PhoneNumberFormatter.java +++ b/library/src/org/whispersystems/textsecure/util/PhoneNumberFormatter.java @@ -21,7 +21,9 @@ public class PhoneNumberFormatter { return number.matches("^\\+[0-9]{10,}"); } - private static String impreciseFormatNumber(String number, String localNumber) { + private static String impreciseFormatNumber(String number, String localNumber) + throws InvalidNumberException + { number = number.replaceAll("[^0-9+]", ""); if (number.charAt(0) == '+') @@ -49,9 +51,19 @@ public class PhoneNumberFormatter { } } - public static String formatNumber(String number, String localNumber) { + public static String formatNumber(String number, String localNumber) + throws InvalidNumberException + { + if (number.contains("@")) { + throw new InvalidNumberException("Possible attempt to use email address."); + } + number = number.replaceAll("[^0-9+]", ""); + if (number.length() == 0) { + throw new InvalidNumberException("No valid characters found."); + } + if (number.charAt(0) == '+') return number; diff --git a/src/org/thoughtcrime/securesms/transport/PushTransport.java b/src/org/thoughtcrime/securesms/transport/PushTransport.java index 82a0bf0521..4053d01f95 100644 --- a/src/org/thoughtcrime/securesms/transport/PushTransport.java +++ b/src/org/thoughtcrime/securesms/transport/PushTransport.java @@ -48,6 +48,7 @@ import org.whispersystems.textsecure.push.PushMessageProtos.PushMessageContent; import org.whispersystems.textsecure.push.PushServiceSocket; import org.whispersystems.textsecure.push.RateLimitException; import org.whispersystems.textsecure.storage.SessionRecordV2; +import org.whispersystems.textsecure.util.InvalidNumberException; import java.io.IOException; import java.util.LinkedList; @@ -86,6 +87,9 @@ public class PushTransport extends BaseTransport { } catch (RateLimitException e) { Log.w("PushTransport", e); throw new IOException("Rate limit exceeded."); + } catch (InvalidNumberException e) { + Log.w("PushTransport", e); + throw new IOException("Badly formatted number."); } } diff --git a/src/org/thoughtcrime/securesms/transport/UniversalTransport.java b/src/org/thoughtcrime/securesms/transport/UniversalTransport.java index b2a6a7ed37..00724ed337 100644 --- a/src/org/thoughtcrime/securesms/transport/UniversalTransport.java +++ b/src/org/thoughtcrime/securesms/transport/UniversalTransport.java @@ -32,6 +32,7 @@ import org.whispersystems.textsecure.directory.NotInDirectoryException; import org.whispersystems.textsecure.push.ContactTokenDetails; import org.whispersystems.textsecure.push.PushDestination; import org.whispersystems.textsecure.push.PushServiceSocket; +import org.whispersystems.textsecure.util.InvalidNumberException; import java.io.IOException; import java.util.LinkedList; @@ -60,19 +61,24 @@ public class UniversalTransport { return; } - Recipient recipient = message.getIndividualRecipient(); - String number = Util.canonicalizeNumber(context, recipient.getNumber()); + try { + Recipient recipient = message.getIndividualRecipient(); + String number = Util.canonicalizeNumber(context, recipient.getNumber()); - if (isPushTransport(number)) { - try { - Log.w("UniversalTransport", "Delivering with GCM..."); - pushTransport.deliver(message); - } catch (IOException ioe) { - Log.w("UniversalTransport", ioe); + if (isPushTransport(number)) { + try { + Log.w("UniversalTransport", "Delivering with GCM..."); + pushTransport.deliver(message); + } catch (IOException ioe) { + Log.w("UniversalTransport", ioe); + smsTransport.deliver(message); + } + } else { + Log.w("UniversalTransport", "Delivering with SMS..."); smsTransport.deliver(message); } - } else { - Log.w("UniversalTransport", "Delivering with SMS..."); + } catch (InvalidNumberException e) { + Log.w("UniversalTransport", e); smsTransport.deliver(message); } } @@ -84,24 +90,31 @@ public class UniversalTransport { return mmsTransport.deliver(mediaMessage); } - List destinations = getMediaDestinations(mediaMessage); + try { + List destinations = getMediaDestinations(mediaMessage); - if (isPushTransport(destinations)) { - try { - Log.w("UniversalTransport", "Delivering media message with GCM..."); - pushTransport.deliver(mediaMessage, destinations, threadId); - return new MmsSendResult("push".getBytes("UTF-8"), 0, true); - } catch (IOException ioe) { - Log.w("UniversalTransport", ioe); + if (isPushTransport(destinations)) { + try { + Log.w("UniversalTransport", "Delivering media message with GCM..."); + pushTransport.deliver(mediaMessage, destinations, threadId); + return new MmsSendResult("push".getBytes("UTF-8"), 0, true); + } catch (IOException ioe) { + Log.w("UniversalTransport", ioe); + return mmsTransport.deliver(mediaMessage); + } + } else { + Log.w("UniversalTransport", "Delivering media message with MMS..."); return mmsTransport.deliver(mediaMessage); } - } else { - Log.w("UniversalTransport", "Delivering media message with MMS..."); + } catch (InvalidNumberException e) { + Log.w("UniversalTransport", e); return mmsTransport.deliver(mediaMessage); } } - private List getMediaDestinations(SendReq mediaMessage) { + private List getMediaDestinations(SendReq mediaMessage) + throws InvalidNumberException + { TextSecurePushCredentials credentials = TextSecurePushCredentials.getInstance(); LinkedList destinations = new LinkedList(); diff --git a/src/org/thoughtcrime/securesms/util/Util.java b/src/org/thoughtcrime/securesms/util/Util.java index 59779dcc22..26e44efad1 100644 --- a/src/org/thoughtcrime/securesms/util/Util.java +++ b/src/org/thoughtcrime/securesms/util/Util.java @@ -27,6 +27,7 @@ import android.os.Build; import android.provider.Telephony; import org.thoughtcrime.securesms.mms.MmsRadio; +import org.whispersystems.textsecure.util.InvalidNumberException; import org.whispersystems.textsecure.util.PhoneNumberFormatter; import java.io.ByteArrayOutputStream; @@ -122,7 +123,9 @@ public class Util { } } - public static String canonicalizeNumber(Context context, String number) { + public static String canonicalizeNumber(Context context, String number) + throws InvalidNumberException + { String localNumber = TextSecurePreferences.getLocalNumber(context); return PhoneNumberFormatter.formatNumber(number, localNumber); }