Merge pull request #9207 from joefarebrother/android-external-storage

Java: Add sources for Android external storage

java/ql/lib/semmle/code/java/dataflow/ExternalFlow.qll

@@ -84,6 +84,7 @@ private module Frameworks {
   private import internal.ContainerFlow
   private import semmle.code.java.frameworks.android.Android
   private import semmle.code.java.frameworks.android.ContentProviders
+  private import semmle.code.java.frameworks.android.ExternalStorage
   private import semmle.code.java.frameworks.android.Intent
   private import semmle.code.java.frameworks.android.Notifications
   private import semmle.code.java.frameworks.android.SharedPreferences
@@ -646,7 +647,7 @@ module CsvValidation {
     or
     exists(string row, string kind | sourceModel(row) |
       kind = row.splitAt(";", 7) and
-      not kind = ["remote", "contentprovider", "android-widget"] and
+      not kind = ["remote", "contentprovider", "android-widget", "android-external-storage-dir"] and
       not kind.matches("qltest%") and
       msg = "Invalid kind \"" + kind + "\" in source model."
     )

java/ql/lib/semmle/code/java/dataflow/FlowSources.qll

@@ -17,6 +17,7 @@ import semmle.code.java.frameworks.android.WebView
 import semmle.code.java.frameworks.JaxWS
 import semmle.code.java.frameworks.javase.WebSocket
 import semmle.code.java.frameworks.android.Android
+import semmle.code.java.frameworks.android.ExternalStorage
 import semmle.code.java.frameworks.android.OnActivityResultSource
 import semmle.code.java.frameworks.android.Intent
 import semmle.code.java.frameworks.play.Play
@@ -152,6 +153,12 @@ private class ThriftIfaceParameterSource extends RemoteFlowSource {
   override string getSourceType() { result = "Thrift Iface parameter" }
 }
 
+private class AndroidExternalStorageSource extends RemoteFlowSource {
+  AndroidExternalStorageSource() { androidExternalStorageSource(this) }
+
+  override string getSourceType() { result = "Android external storage" }
+}
+
 /** Class for `tainted` user input. */
 abstract class UserInput extends DataFlow::Node { }
 

java/ql/lib/semmle/code/java/frameworks/android/ExternalStorage.qll

@@ -0,0 +1,50 @@
+/** Provides definitions for working with uses of Android external storage */
+
+import java
+private import semmle.code.java.security.FileReadWrite
+private import semmle.code.java.dataflow.DataFlow
+private import semmle.code.java.dataflow.ExternalFlow
+
+private class ExternalStorageDirSourceModel extends SourceModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        //"package;type;overrides;name;signature;ext;spec;kind"
+        "android.content;Context;true;getExternalFilesDir;(String);;ReturnValue;android-external-storage-dir;manual",
+        "android.content;Context;true;getExternalFilesDirs;(String);;ReturnValue;android-external-storage-dir;manual",
+        "android.content;Context;true;getExternalCacheDir;();;ReturnValue;android-external-storage-dir;manual",
+        "android.content;Context;true;getExternalCacheDirs;();;ReturnValue;android-external-storage-dir;manual",
+        "android.os;Environment;false;getExternalStorageDirectory;();;ReturnValue;android-external-storage-dir;manual",
+        "android.os;Environment;false;getExternalStoragePublicDirectory;(String);;ReturnValue;android-external-storage-dir;manual",
+      ]
+  }
+}
+
+private predicate externalStorageFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+  DataFlow::localFlowStep(node1, node2)
+  or
+  exists(ConstructorCall c | c.getConstructedType() instanceof TypeFile |
+    node1.asExpr() = c.getArgument(0) and
+    node2.asExpr() = c
+  )
+  or
+  node2.asExpr().(ArrayAccess).getArray() = node1.asExpr()
+  or
+  node2.asExpr().(FieldRead).getField().getInitializer() = node1.asExpr()
+}
+
+private predicate externalStorageFlow(DataFlow::Node node1, DataFlow::Node node2) {
+  externalStorageFlowStep*(node1, node2)
+}
+
+/**
+ * Holds if `n` is a node that reads the contents of an external file in Android.
+ * This is controllable by third-party applications, so is treated as a remote flow source.
+ */
+predicate androidExternalStorageSource(DataFlow::Node n) {
+  exists(DataFlow::Node externalDir, DirectFileReadExpr read |
+    sourceNode(externalDir, "android-external-storage-dir") and
+    n.asExpr() = read and
+    externalStorageFlow(externalDir, DataFlow::exprNode(read.getFileExpr()))
+  )
+}

java/ql/lib/semmle/code/java/security/FileReadWrite.qll

@@ -1,9 +1,9 @@
 import java
 
 /**
- * Holds if `fileAccess` is used in the `fileReadingExpr` to read the represented file.
+ * Holds if `fileAccess` is directly used in the `fileReadingExpr` to read the represented file.
  */
-private predicate fileRead(VarAccess fileAccess, Expr fileReadingExpr) {
+private predicate directFileRead(Expr fileAccess, Expr fileReadingExpr) {
   // `fileAccess` used to construct a class that reads a file.
   exists(ClassInstanceExpr cie |
     cie = fileReadingExpr and
@@ -28,6 +28,13 @@ private predicate fileRead(VarAccess fileAccess, Expr fileReadingExpr) {
             ])
     )
   )
+}
+
+/**
+ * Holds if `fileAccess` is used in the `fileReadingExpr` to read the represented file.
+ */
+private predicate fileRead(VarAccess fileAccess, Expr fileReadingExpr) {
+  directFileRead(fileAccess, fileReadingExpr)
   or
   // The `fileAccess` is used in a call which directly or indirectly accesses the file.
   exists(Call call, int parameterPos, VarAccess nestedFileAccess, Expr nestedFileReadingExpr |
@@ -49,3 +56,15 @@ class FileReadExpr extends Expr {
    */
   VarAccess getFileVarAccess() { fileRead(result, this) }
 }
+
+/**
+ * An expression that directly reads from a file and returns its contents.
+ */
+class DirectFileReadExpr extends Expr {
+  DirectFileReadExpr() { directFileRead(_, this) }
+
+  /**
+   * Gets the `Expr` representing the file that is read.
+   */
+  Expr getFileExpr() { directFileRead(result, this) }
+}