Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(737)

Unified Diff: chrome/android/java/src/org/chromium/chrome/browser/UrlUtilities.java

Issue 1059413004: Add a validator for intent:// URLs. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Allow Extras; require that standard keys appear only once; use Matchers; more test cases. Created 5 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/android/java/src/org/chromium/chrome/browser/UrlUtilities.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/UrlUtilities.java b/chrome/android/java/src/org/chromium/chrome/browser/UrlUtilities.java
index 1eca2c96dce9aba68b4853176d8e0fab675a370f..4ae28d319e6325f40ae4e77a128e12f79bc6a68a 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/UrlUtilities.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/UrlUtilities.java
@@ -11,6 +11,8 @@ import org.chromium.base.CollectionUtil;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.HashSet;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
/**
* Utilities for working with URIs (and URLs). These methods may be used in security-sensitive
@@ -207,6 +209,89 @@ public class UrlUtilities {
return nativeGetDomainAndRegistry(uri, includePrivateRegistries);
}
+ // Patterns used in validateIntentUrl.
+ private static final Pattern DNS_HOSTNAME_PATTERN =
+ Pattern.compile("^[\\w\\.-]*$");
+ private static final Pattern JAVA_PACKAGE_NAME_PATTERN =
+ Pattern.compile("^[\\w\\.-]*$");
+ private static final Pattern URL_SCHEME_PATTERN =
+ Pattern.compile("^[a-zA-Z]+$");
+
+ /**
+ * @param url An Android intent:// URL to validate.
+ *
+ * @throws URISyntaxException if url is not a valid Android intent://
+ * URL, as specified at
+ * https://developer.chrome.com/multidevice/android/intents#syntax.
+ */
+ public static boolean validateIntentUrl(String url) {
+ URI parsed = null;
+ try {
+ parsed = new URI(url);
+ } catch (URISyntaxException e) {
+ return false;
+ }
+
+ if (!parsed.getScheme().equals("intent")) return false;
+
+ Matcher m = DNS_HOSTNAME_PATTERN.matcher(parsed.getHost());
+ if (!m.matches()) return false;
+
+ if (!parsed.getPath().isEmpty() && !parsed.getPath().equals("/")) {
+ return false;
+ }
+
+ String[] parts = parsed.getFragment().split(";");
+ if (parts.length < 3
+ || !parts[0].equals("Intent")
+ || !parts[parts.length - 1].equals("end")) {
+ return false;
+ }
+
+ boolean seenPackage = false;
+ boolean seenAction = false;
+ boolean seenCategory = false;
+ boolean seenComponent = false;
+ boolean seenScheme = false;
+
+ for (int i = 1; i < parts.length - 1; ++i) {
+ // This is OK *only* because no valid package, action, category,
+ // component, or scheme contains "=".
+ String[] pair = parts[i].split("=");
+ if (2 != pair.length) return false;
Yaron 2015/04/24 20:38:17 I think this breaks "extras". see below
+
+ m = JAVA_PACKAGE_NAME_PATTERN.matcher(pair[1]);
+ if (pair[0].equals("package")) {
+ if (seenPackage || !m.matches()) return false;
+ seenPackage = true;
+ } else if (pair[0].equals("action")) {
+ if (seenAction || !m.matches()) return false;
+ seenAction = true;
+ } else if (pair[0].equals("category")) {
+ if (seenCategory || !m.matches()) return false;
+ seenCategory = true;
+ } else if (pair[0].equals("component")) {
+ if (seenComponent || !m.matches()) return false;
+ seenComponent = true;
+ } else if (pair[0].equals("scheme")) {
+ if (seenScheme) return false;
+ Matcher schemeMatcher = URL_SCHEME_PATTERN.matcher(pair[1]);
+ if (!schemeMatcher.matches()) return false;
+ seenScheme = true;
+ } else {
+ // Assume we are seeing an Intent Extra. Require that the
+ // key be a valid Java package name, and that the value also
+ // fit that rather restrictive pattern. The latter
+ // requirement may have to be relaxed in the future, but
+ // beware: Here Lurks Madness.
Yaron 2015/04/24 20:38:17 I guess this is a deliberate "fail closed" but I w
+ Matcher keyMatcher = JAVA_PACKAGE_NAME_PATTERN.matcher(pair[0]);
+ if (!m.matches() || !keyMatcher.matches()) return false;
+ }
+ }
+
+ return true;
+ }
+
private static native boolean nativeSameDomainOrHost(String primaryUrl, String secondaryUrl,
boolean includePrivateRegistries);
private static native String nativeGetDomainAndRegistry(String url,

Powered by Google App Engine
This is Rietveld 408576698