diff --git a/core/src/main/java/com/google/zxing/client/result/URIParsedResult.java b/core/src/main/java/com/google/zxing/client/result/URIParsedResult.java index e26e361b4..b1900ba65 100644 --- a/core/src/main/java/com/google/zxing/client/result/URIParsedResult.java +++ b/core/src/main/java/com/google/zxing/client/result/URIParsedResult.java @@ -16,8 +16,6 @@ package com.google.zxing.client.result; -import java.util.regex.Pattern; - /** * A simple result type encapsulating a URI that has no further interpretation. * @@ -25,8 +23,6 @@ import java.util.regex.Pattern; */ public final class URIParsedResult extends ParsedResult { - private static final Pattern USER_IN_HOST = Pattern.compile(":/*([^/@]+)@[^/]+"); - private final String uri; private final String title; @@ -45,15 +41,11 @@ public final class URIParsedResult extends ParsedResult { } /** - * @return true if the URI contains suspicious patterns that may suggest it intends to - * mislead the user about its true nature. At the moment this looks for the presence - * of user/password syntax in the host/authority portion of a URI which may be used - * in attempts to make the URI's host appear to be other than it is. Example: - * http://yourbank.com@phisher.com This URI connects to phisher.com but may appear - * to connect to yourbank.com at first glance. + * @deprecated see {@link URIResultParser#isPossiblyMaliciousURI(String)} */ + @Deprecated public boolean isPossiblyMaliciousURI() { - return USER_IN_HOST.matcher(uri).find(); + return URIResultParser.isPossiblyMaliciousURI(uri); } @Override diff --git a/core/src/main/java/com/google/zxing/client/result/URIResultParser.java b/core/src/main/java/com/google/zxing/client/result/URIResultParser.java index 852f39c16..21adc34f5 100644 --- a/core/src/main/java/com/google/zxing/client/result/URIResultParser.java +++ b/core/src/main/java/com/google/zxing/client/result/URIResultParser.java @@ -28,6 +28,9 @@ import java.util.regex.Pattern; */ public final class URIResultParser extends ResultParser { + private static final Pattern ALLOWED_URI_CHARS_PATTERN = + Pattern.compile("[-._~:/?#\\[\\]@!$&'()*+,;=%A-Za-z0-9]+"); + private static final Pattern USER_IN_HOST = Pattern.compile(":/*([^/@]+)@[^/]+"); // See http://www.ietf.org/rfc/rfc2396.txt private static final Pattern URL_WITH_PROTOCOL_PATTERN = Pattern.compile("[a-zA-Z][a-zA-Z0-9+-.]+:"); private static final Pattern URL_WITHOUT_PROTOCOL_PATTERN = Pattern.compile( @@ -44,7 +47,22 @@ public final class URIResultParser extends ResultParser { return new URIParsedResult(rawText.substring(4).trim(), null); } rawText = rawText.trim(); - return isBasicallyValidURI(rawText) ? new URIParsedResult(rawText, null) : null; + if (!isBasicallyValidURI(rawText) || isPossiblyMaliciousURI(rawText)) { + return null; + } + return new URIParsedResult(rawText, null); + } + + /** + * @return true if the URI contains suspicious patterns that may suggest it intends to + * mislead the user about its true nature. At the moment this looks for the presence + * of user/password syntax in the host/authority portion of a URI which may be used + * in attempts to make the URI's host appear to be other than it is. Example: + * http://yourbank.com@phisher.com This URI connects to phisher.com but may appear + * to connect to yourbank.com at first glance. + */ + static boolean isPossiblyMaliciousURI(String uri) { + return !ALLOWED_URI_CHARS_PATTERN.matcher(uri).matches() || USER_IN_HOST.matcher(uri).find(); } static boolean isBasicallyValidURI(String uri) { diff --git a/core/src/test/java/com/google/zxing/client/result/CalendarParsedResultTestCase.java b/core/src/test/java/com/google/zxing/client/result/CalendarParsedResultTestCase.java index e81f10979..2b7910727 100644 --- a/core/src/test/java/com/google/zxing/client/result/CalendarParsedResultTestCase.java +++ b/core/src/test/java/com/google/zxing/client/result/CalendarParsedResultTestCase.java @@ -146,7 +146,7 @@ public final class CalendarParsedResultTestCase extends Assert { "GEO:-12.345\r\n" + "END:VEVENT\r\nEND:VCALENDAR", null, null, BarcodeFormat.QR_CODE); ParsedResult result = ResultParser.parseResult(fakeResult); - assertSame(ParsedResultType.URI, result.getType()); + assertSame(ParsedResultType.TEXT, result.getType()); } @Test diff --git a/core/src/test/java/com/google/zxing/client/result/ParsedReaderResultTestCase.java b/core/src/test/java/com/google/zxing/client/result/ParsedReaderResultTestCase.java index e8d6020b6..024c201fa 100644 --- a/core/src/test/java/com/google/zxing/client/result/ParsedReaderResultTestCase.java +++ b/core/src/test/java/com/google/zxing/client/result/ParsedReaderResultTestCase.java @@ -238,7 +238,7 @@ public final class ParsedReaderResultTestCase extends Assert { doTestResult("BEGIN:VEVENT\r\nSUMMARY:foo\r\nDTSTART:20080504\r\nEND:VEVENT", "foo\n" + formatDate(2008, 5, 4), ParsedResultType.CALENDAR); doTestResult("BEGIN:VEVENT\r\nDTEND:20080505T\r\nEND:VEVENT", - "BEGIN:VEVENT\r\nDTEND:20080505T\r\nEND:VEVENT", ParsedResultType.URI); + "BEGIN:VEVENT\r\nDTEND:20080505T\r\nEND:VEVENT", ParsedResultType.TEXT); // Yeah, it's OK that this is thought of as maybe a URI as long as it's not CALENDAR // Make sure illegal entries without newlines don't crash doTestResult( diff --git a/core/src/test/java/com/google/zxing/client/result/URIParsedResultTestCase.java b/core/src/test/java/com/google/zxing/client/result/URIParsedResultTestCase.java index 0a7e43c1b..b5af83e50 100644 --- a/core/src/test/java/com/google/zxing/client/result/URIParsedResultTestCase.java +++ b/core/src/test/java/com/google/zxing/client/result/URIParsedResultTestCase.java @@ -93,6 +93,12 @@ public final class URIParsedResultTestCase extends Assert { doTestIsPossiblyMalicious("http://google.com/@@", false); } + @Test + public void testMaliciousUnicode() { + doTestIsPossiblyMalicious("https://google.com\u2215.evil.com/stuff", true); + doTestIsPossiblyMalicious("\u202ehttps://dylankatz.com/moc.elgoog.www//:sptth", true); + } + @Test public void testExotic() { doTest("bitcoin:mySD89iqpmptrK3PhHFW9fa7BXiP7ANy3Y", "bitcoin:mySD89iqpmptrK3PhHFW9fa7BXiP7ANy3Y", null); @@ -124,9 +130,10 @@ public final class URIParsedResultTestCase extends Assert { assertEquals(text, result.getDisplayResult()); } - private static void doTestIsPossiblyMalicious(String uri, boolean expected) { - URIParsedResult result = new URIParsedResult(uri, null); - assertEquals(expected, result.isPossiblyMaliciousURI()); + private static void doTestIsPossiblyMalicious(String uri, boolean malicious) { + Result fakeResult = new Result(uri, null, null, BarcodeFormat.QR_CODE); + ParsedResult result = ResultParser.parseResult(fakeResult); + assertSame(malicious ? ParsedResultType.TEXT : ParsedResultType.URI, result.getType()); } } \ No newline at end of file