Address improper URL authorization

Author: luchua-bc

java/ql/src/experimental/CWE-939/IncorrectURLVerification.java

@@ -0,0 +1,17 @@
+public boolean shouldOverrideUrlLoading(WebView view, String url) {
+	{
+		Uri uri = Uri.parse(url);
+		// BAD: partial domain match, which allows an attacker to register a domain like myexample.com to circumvent the verification
+		if (uri.getHost() != null && uri.getHost().endsWith("example.com")) {
+			return false;
+		}
+	}
+
+	{
+		Uri uri = Uri.parse(url);
+		// GOOD: full domain match
+		if (uri.getHost() != null && uri.getHost().endsWith(".example.com")) {
+			return false;
+		}
+	}
+}

java/ql/src/experimental/CWE-939/IncorrectURLVerification.qhelp

@@ -0,0 +1,30 @@
+<!DOCTYPE qhelp PUBLIC
+  "-//Semmle//qhelp//EN"
+  "qhelp.dtd">
+<qhelp>
+
+<overview>
+<p>Apps that rely on URL Parsing to verify that a given URL is pointing to a trust server may be susceptible to many different ways to get URL parsing and verification wrong, which allows an attacker to register a fake site to break the access control.</p>
+</overview>
+
+<recommendation>
+<p>Verify the whole host and domain (FQDN) or check endsWith dot+domain.</p>
+</recommendation>
+
+<example>
+<p>The following example shows two ways of verify host domain. In the 'BAD' case,
+verification is implemented as partial domain match. In the 'GOOD' case, full domain is verified.</p>
+<sample src="IncorrectURLVerification.java" />
+</example>
+
+<references>
+<li>
+<a href="https://drive.google.com/file/d/0BwMN49Gzo3x6T1N5WGQ4TTNlMHBOb1ZRQTVEWnVBZjFUaE5N/view">Common Android app vulnerabilities from Sebastian Porst of Google</a>
+</li>
+<li>
+<a href="https://www.bugcrowd.com/resources/webinars/overview-of-common-android-app-vulnerabilities/">Common Android app vulnerabilities from bugcrowd</a>
+<li>
+<a href="https://cwe.mitre.org/data/definitions/939.html">CWE 939</a>
+</li>
+</references>
+</qhelp>

java/ql/src/experimental/CWE-939/IncorrectURLVerification.ql

@@ -0,0 +1,64 @@
+/**
+ * @id java/incorrect-url-verification
+ * @name Insertion of sensitive information into log files
+ * @description Apps that rely on URL parsing to verify that a given URL is pointing to a trusted server are susceptible to wrong ways of URL parsing and verification. 
+ * @kind problem
+ * @tags security
+ *       external/cwe-939
+ */
+
+import java
+import semmle.code.java.dataflow.FlowSources
+import semmle.code.java.dataflow.TaintTracking
+import DataFlow
+import PathGraph
+
+
+/**
+ * The Java class `android.net.Uri` and `java.net.URL`.
+ */
+class Uri extends RefType {
+    Uri() {
+        hasQualifiedName("android.net", "Uri") or
+        hasQualifiedName("java.net", "URL")
+    }
+}
+
+/**
+ * The method `getHost()` declared in `android.net.Uri` and `java.net.URL`.
+ */
+class UriGetHostMethod extends Method {
+    UriGetHostMethod() {
+        getDeclaringType() instanceof Uri and
+        hasName("getHost") and
+        getNumberOfParameters() = 0
+    }
+}
+
+/**
+ * A library method that acts like `String.format` by formatting a number of
+ * its arguments according to a format string.
+ */
+class HostVerificationMethodAccess extends MethodAccess {
+    HostVerificationMethodAccess() {
+        (
+
+            this.getMethod().hasName("endsWith") or
+            this.getMethod().hasName("contains") or
+            this.getMethod().hasName("indexOf")
+        ) and
+        this.getMethod().getNumberOfParameters() = 1 and 
+        (
+            this.getArgument(0).(StringLiteral).getRepresentedString().charAt(0) != "." or  //string constant comparison
+            this.getArgument(0).(AddExpr).getLeftOperand().(VarAccess).getVariable().getAnAssignedValue().(StringLiteral).getRepresentedString().charAt(0) != "." or   //var1+var2, check var1 starts with "."
+            this.getArgument(0).(AddExpr).getLeftOperand().(StringLiteral).getRepresentedString().charAt(0) != "." or  //"."+var2, check string constant "."
+            exists (MethodAccess ma | this.getArgument(0) = ma and ma.getMethod().hasName("getString") and ma.getArgument(0).toString().indexOf("R.string") = 0) or  //res.getString(R.string.key)
+            this.getArgument(0).(VarAccess).getVariable().getAnAssignedValue().(StringLiteral).getRepresentedString().charAt(0) != "."  //check variable starts with "."
+        )
+    }
+}
+
+from UriGetHostMethod um, MethodAccess uma, HostVerificationMethodAccess hma
+where hma.getQualifier() = uma and uma.getMethod() = um
+select "Potentially improper URL verification with $@ in $@ having $@.",
+  hma, hma.getFile(), hma.getArgument(0), "user-provided value"