From 7da7bec241af7a2512da07de252955afa2973b05 Mon Sep 17 00:00:00 2001 From: Moxie Marlinspike Date: Mon, 24 Dec 2018 16:19:04 -0800 Subject: [PATCH] Do more thorough phone number validation --- pom.xml | 7 +++ .../controllers/AccountController.java | 2 +- .../textsecuregcm/util/Util.java | 12 ++--- .../controllers/AccountControllerTest.java | 4 +- .../tests/util/ValidNumberTest.java | 47 +++++++++++++++++++ 5 files changed, 60 insertions(+), 12 deletions(-) create mode 100644 src/test/java/org/whispersystems/textsecuregcm/tests/util/ValidNumberTest.java diff --git a/pom.xml b/pom.xml index 1ed27b5c7..1f6aa9c69 100644 --- a/pom.xml +++ b/pom.xml @@ -126,6 +126,13 @@ 0.1.5 + + com.googlecode.libphonenumber + libphonenumber + 8.10.2 + + + org.glassfish.jersey.test-framework.providers jersey-test-framework-provider-grizzly2 diff --git a/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java b/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java index 867eae509..b6081ba54 100644 --- a/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java +++ b/src/main/java/org/whispersystems/textsecuregcm/controllers/AccountController.java @@ -127,7 +127,7 @@ public class AccountController { throws IOException, RateLimitExceededException { if (!Util.isValidNumber(number)) { - logger.debug("Invalid number: " + number); + logger.info("Invalid number: " + number); throw new WebApplicationException(Response.status(400).build()); } diff --git a/src/main/java/org/whispersystems/textsecuregcm/util/Util.java b/src/main/java/org/whispersystems/textsecuregcm/util/Util.java index 4618bcc95..8164a98a5 100644 --- a/src/main/java/org/whispersystems/textsecuregcm/util/Util.java +++ b/src/main/java/org/whispersystems/textsecuregcm/util/Util.java @@ -16,6 +16,8 @@ */ package org.whispersystems.textsecuregcm.util; +import com.google.i18n.phonenumbers.PhoneNumberUtil; + import java.io.UnsupportedEncodingException; import java.net.URLEncoder; import java.security.MessageDigest; @@ -48,15 +50,7 @@ public class Util { } public static boolean isValidNumber(String number) { - return number.matches("^\\+[0-9]{10,}") || - number.matches("^\\+240[0-9]{6}") || // Equatorial Guinea - number.matches("^\\+298[0-9]{6}") || // Faroe Islands - number.matches("^\\+299[0-9]{6}") || // Greenland - number.matches("^\\+376[0-9]{6}") || // Andorra - number.matches("^\\+597[0-9]{6}") || // Suriname - number.matches("^\\+685[0-9]{5}") || // Samoa - number.matches("^\\+687[0-9]{6}") || // New Caledonia - number.matches("^\\+689[0-9]{6}"); // French Polynesia + return number.matches("^\\+[0-9]+") && PhoneNumberUtil.getInstance().isPossibleNumber(number, null); } public static String getCountryCode(String number) { diff --git a/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java b/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java index fa690c402..d924e2903 100644 --- a/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java +++ b/src/test/java/org/whispersystems/textsecuregcm/tests/controllers/AccountControllerTest.java @@ -213,14 +213,14 @@ public class AccountControllerTest { public void testSendRestrictedIn() throws Exception { Response response = resources.getJerseyTest() - .target(String.format("/v1/accounts/sms/code/%s", "+1234567890")) + .target(String.format("/v1/accounts/sms/code/%s", "+12345678901")) .request() .header("X-Forwarded-For", RESTRICTED_HOST) .get(); assertThat(response.getStatus()).isEqualTo(200); - verify(smsSender).deliverSmsVerification(eq("+1234567890"), eq(Optional.empty()), anyString()); + verify(smsSender).deliverSmsVerification(eq("+12345678901"), eq(Optional.empty()), anyString()); } @Test diff --git a/src/test/java/org/whispersystems/textsecuregcm/tests/util/ValidNumberTest.java b/src/test/java/org/whispersystems/textsecuregcm/tests/util/ValidNumberTest.java new file mode 100644 index 000000000..b08822d53 --- /dev/null +++ b/src/test/java/org/whispersystems/textsecuregcm/tests/util/ValidNumberTest.java @@ -0,0 +1,47 @@ +package org.whispersystems.textsecuregcm.tests.util; + +import org.junit.Test; +import org.whispersystems.textsecuregcm.util.Util; + +import static junit.framework.TestCase.assertTrue; +import static org.junit.Assert.assertFalse; + +public class ValidNumberTest { + + @Test + public void testValidE164() { + assertTrue(Util.isValidNumber("+14151231234")); + assertTrue(Util.isValidNumber("+71234567890")); + assertTrue(Util.isValidNumber("+447535742222")); + assertTrue(Util.isValidNumber("+4915174108888")); + } + + @Test + public void testInvalidE164() { + assertFalse(Util.isValidNumber("+141512312341")); + assertFalse(Util.isValidNumber("+712345678901")); + assertFalse(Util.isValidNumber("+4475357422221")); + assertFalse(Util.isValidNumber("+491517410888811111")); + } + + @Test + public void testNotE164() { + assertFalse(Util.isValidNumber("+1 415 123 1234")); + assertFalse(Util.isValidNumber("+1 (415) 123-1234")); + assertFalse(Util.isValidNumber("+1 415)123-1234")); + assertFalse(Util.isValidNumber("71234567890")); + assertFalse(Util.isValidNumber("001447535742222")); + assertFalse(Util.isValidNumber(" +14151231234")); + assertFalse(Util.isValidNumber("+1415123123a")); + } + + @Test + public void testShortRegions() { + assertTrue(Util.isValidNumber("+298123456")); + assertTrue(Util.isValidNumber("+299123456")); + assertTrue(Util.isValidNumber("+376123456")); + assertTrue(Util.isValidNumber("+68512345")); + assertTrue(Util.isValidNumber("+689123456")); + } + +}