Merge pull request #4947 from porcupineyhairs/DexLoading

Java : add query to detect insecure loading of Dex File

java/ql/src/experimental/Security/CWE/CWE-094/InsecureDexLoading.qhelp

@@ -0,0 +1,42 @@
+<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
+<qhelp>
+  <overview>
+    <p>
+It is dangerous to load Dex libraries from shared world-writable storage spaces. A malicious actor can replace a dex file with a maliciously crafted file 
+which when loaded by the app can lead to code execution. 
+</p>
+  </overview>
+
+  <recommendation>
+    <p>
+      Loading a file from private storage instead of a world-writable one can prevent this issue,
+      because the attacker cannot access files stored there. 
+    </p>
+  </recommendation>
+
+  <example>
+    <p>
+      The following example loads a Dex file from a shared world-writable location. in this case, 
+      since the `/sdcard` directory is on external storage, anyone can read/write to the location. 
+      bypassing all Android security policies. Hence, this is insecure.
+    </p>
+    <sample src="InsecureDexLoadingBad.java" />
+
+    <p>
+    The next example loads a Dex file stored inside the app's private storage. 
+    This is not exploitable as nobody else except the app can access the data stored there.
+    </p>
+    <sample src="InsecureDexLoadingGood.java" />
+  </example>
+
+  <references>
+    <li>
+      Android Documentation:
+      <a href="https://developer.android.com/training/data-storage/">Data and file storage overview</a>.
+    </li>
+    <li>
+      Android Documentation:
+      <a href="https://developer.android.com/reference/dalvik/system/DexClassLoader">DexClassLoader</a>.
+    </li>
+  </references>
+</qhelp>

java/ql/src/experimental/Security/CWE/CWE-094/InsecureDexLoading.ql

@@ -0,0 +1,20 @@
+/**
+ * @name Insecure loading of an Android Dex File
+ * @description Loading a DEX library located in a world-writable location such as
+ * an SD card can lead to arbitrary code execution vulnerabilities.
+ * @kind path-problem
+ * @problem.severity error
+ * @precision high
+ * @id java/android-insecure-dex-loading
+ * @tags security
+ *       external/cwe/cwe-094
+ */
+
+import java
+import InsecureDexLoading
+import DataFlow::PathGraph
+
+from DataFlow::PathNode source, DataFlow::PathNode sink, InsecureDexConfiguration conf
+where conf.hasFlowPath(source, sink)
+select sink.getNode(), source, sink, "Potential arbitrary code execution due to $@.",
+  source.getNode(), "a value loaded from a world-writable source."

java/ql/src/experimental/Security/CWE/CWE-094/InsecureDexLoading.qll

@@ -0,0 +1,100 @@
+import java
+import semmle.code.java.dataflow.FlowSources
+
+/**
+ * A taint-tracking configuration detecting unsafe use of a
+ * `DexClassLoader` by an Android app.
+ */
+class InsecureDexConfiguration extends TaintTracking::Configuration {
+  InsecureDexConfiguration() { this = "Insecure Dex File Load" }
+
+  override predicate isSource(DataFlow::Node source) { source instanceof InsecureDexSource }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof InsecureDexSink }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
+    flowStep(pred, succ)
+  }
+}
+
+/** A data flow source for insecure Dex class loading vulnerabilities. */
+abstract class InsecureDexSource extends DataFlow::Node { }
+
+/** A data flow sink for insecure Dex class loading vulnerabilities. */
+abstract class InsecureDexSink extends DataFlow::Node { }
+
+private predicate flowStep(DataFlow::Node pred, DataFlow::Node succ) {
+  // propagate from a `java.io.File` via the `File.getAbsolutePath` call.
+  exists(MethodAccess m |
+    m.getMethod().getDeclaringType() instanceof TypeFile and
+    m.getMethod().hasName("getAbsolutePath") and
+    m.getQualifier() = pred.asExpr() and
+    m = succ.asExpr()
+  )
+  or
+  // propagate from a `java.io.File` via the `File.toString` call.
+  exists(MethodAccess m |
+    m.getMethod().getDeclaringType() instanceof TypeFile and
+    m.getMethod().hasName("toString") and
+    m.getQualifier() = pred.asExpr() and
+    m = succ.asExpr()
+  )
+  or
+  // propagate to newly created `File` if the parent directory of the new `File` is tainted
+  exists(ConstructorCall cc |
+    cc.getConstructedType() instanceof TypeFile and
+    cc.getArgument(0) = pred.asExpr() and
+    cc = succ.asExpr()
+  )
+}
+
+/**
+ * An argument to a `DexClassLoader` call taken as a sink for
+ * insecure Dex class loading vulnerabilities.
+ */
+private class DexClassLoader extends InsecureDexSink {
+  DexClassLoader() {
+    exists(ConstructorCall cc |
+      cc.getConstructedType().hasQualifiedName("dalvik.system", "DexClassLoader")
+    |
+      this.asExpr() = cc.getArgument(0)
+    )
+  }
+}
+
+/**
+ * A `File` instance which reads from an SD card
+ * taken as a source for insecure Dex class loading vulnerabilities.
+ */
+private class ExternalFile extends InsecureDexSource {
+  ExternalFile() {
+    exists(ConstructorCall cc, Argument a |
+      cc.getConstructedType() instanceof TypeFile and
+      a = cc.getArgument(0) and
+      a.(CompileTimeConstantExpr).getStringValue().matches("%sdcard%")
+    |
+      this.asExpr() = a
+    )
+  }
+}
+
+/**
+ * A directory or file which may be stored in an world writable directory
+ * taken as a source for insecure Dex class loading vulnerabilities.
+ */
+private class ExternalStorageDirSource extends InsecureDexSource {
+  ExternalStorageDirSource() {
+    exists(Method m |
+      m.getDeclaringType().hasQualifiedName("android.os", "Environment") and
+      m.hasName("getExternalStorageDirectory")
+      or
+      m.getDeclaringType().hasQualifiedName("android.content", "Context") and
+      m.hasName([
+          "getExternalFilesDir", "getExternalFilesDirs", "getExternalMediaDirs",
+          "getExternalCacheDir", "getExternalCacheDirs"
+        ])
+    |
+      this.asExpr() = m.getAReference()
+    )
+  }
+}

java/ql/src/experimental/Security/CWE/CWE-094/InsecureDexLoadingBad.java

@@ -0,0 +1,32 @@
+
+import android.app.Application;
+import android.content.Context;
+import android.content.pm.PackageInfo;
+import android.os.Bundle;
+
+import dalvik.system.DexClassLoader;
+import dalvik.system.DexFile;
+
+public class InsecureDexLoading extends Application {
+	@Override
+	public void onCreate() {
+		super.onCreate();
+		updateChecker();
+	}
+
+	private void updateChecker() {
+		try {
+			File file = new File("/sdcard/updater.apk");
+			if (file.exists() && file.isFile() && file.length() <= 1000) {
+				DexClassLoader cl = new DexClassLoader(file.getAbsolutePath(), getCacheDir().getAbsolutePath(), null,
+						getClassLoader());
+				int version = (int) cl.loadClass("my.package.class").getDeclaredMethod("myMethod").invoke(null);
+				if (Build.VERSION.SDK_INT < version) {
+					Toast.makeText(this, "Loaded Dex!", Toast.LENGTH_LONG).show();
+				}
+			}
+		} catch (Exception e) {
+			// ignore
+		}
+	}
+}

java/ql/src/experimental/Security/CWE/CWE-094/InsecureDexLoadingGood.java

@@ -0,0 +1,23 @@
+public class SecureDexLoading extends Application {
+	@Override
+	public void onCreate() {
+		super.onCreate();
+		updateChecker();
+	}
+
+	private void updateChecker() {
+		try {
+			File file = new File(getCacheDir() + "/updater.apk");
+			if (file.exists() && file.isFile() && file.length() <= 1000) {
+				DexClassLoader cl = new DexClassLoader(file.getAbsolutePath(), getCacheDir().getAbsolutePath(), null,
+						getClassLoader());
+				int version = (int) cl.loadClass("my.package.class").getDeclaredMethod("myMethod").invoke(null);
+				if (Build.VERSION.SDK_INT < version) {
+					Toast.makeText(this, "Securely loaded Dex!", Toast.LENGTH_LONG).show();
+				}
+			}
+		} catch (Exception e) {
+			// ignore
+		}
+	}
+}