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

Java: Add sources for Android external storage

Author: atorralba

java/ql/lib/change-notes/2022-05-18-android-external-storage.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+Added additional flow sources for uses of external storage on Android. 

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) }
+}

java/ql/test/library-tests/frameworks/android/external-storage/Test.java

@@ -0,0 +1,59 @@
+import java.io.File;
+import java.io.InputStream;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.FileReader;
+import java.io.RandomAccessFile;
+import android.content.Context;
+import android.os.Environment;
+
+class Test {
+    void sink(Object o) {}
+
+    void test1(Context ctx) throws IOException {
+        File f = new File(ctx.getExternalFilesDir(null), "file.txt");
+        InputStream is = new FileInputStream(f);
+        byte[] data = new byte[is.available()];
+        is.read(data);
+        sink(data); // $ hasTaintFlow
+        is.close();
+    }
+
+    void test2(Context ctx) throws IOException {
+        File f = new File(new File(new File(ctx.getExternalFilesDirs(null)[0], "things"), "stuff"), "file.txt");
+        sink(new FileInputStream(f)); // $ hasTaintFlow
+    }
+
+    void test3(Context ctx) throws IOException {
+        File f = new File(ctx.getExternalCacheDir(), "file.txt");
+        sink(new FileInputStream(f)); // $ hasTaintFlow
+    }
+
+    void test4(Context ctx) throws IOException {
+        File f = new File(ctx.getExternalCacheDirs()[0], "file.txt");
+        sink(new FileInputStream(f)); // $ hasTaintFlow
+    }
+
+    void test5(Context ctx) throws IOException {
+        File f = new File(Environment.getExternalStorageDirectory(), "file.txt");
+        sink(new FileInputStream(f)); // $ hasTaintFlow
+    }
+
+    void test6(Context ctx) throws IOException {
+        File f = new File(Environment.getExternalStoragePublicDirectory(null), "file.txt");
+        sink(new FileInputStream(f)); // $ hasTaintFlow
+    }
+
+    static final File dir = Environment.getExternalStorageDirectory();
+ 
+    void test7(Context ctx) throws IOException {
+        File f = new File(dir, "file.txt");
+        sink(new FileInputStream(f)); // $ hasTaintFlow 
+    }
+
+    void test8() throws IOException {
+        File f = new File(dir, "file.txt");
+        sink(new FileReader(f)); // $ hasTaintFlow 
+        sink(new RandomAccessFile(f, "r")); // $ hasTaintFlow 
+    }
+ }

java/ql/test/library-tests/frameworks/android/external-storage/options

@@ -0,0 +1 @@
+//semmle-extractor-options: --javac-args -cp ${testdir}/../../../../stubs/google-android-9.0.0

java/ql/test/library-tests/frameworks/android/external-storage/test.expected

@@ -0,0 +1,20 @@
+import java
+import semmle.code.java.dataflow.DataFlow
+import semmle.code.java.dataflow.FlowSources
+import TestUtilities.InlineFlowTest
+
+class Conf extends TaintTracking::Configuration {
+  Conf() { this = "test:AndroidExternalFlowConf" }
+
+  override predicate isSource(DataFlow::Node src) { src instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node sink) {
+    sink.asExpr().(Argument).getCall().getCallee().hasName("sink")
+  }
+}
+
+class ExternalStorageTest extends InlineFlowTest {
+  override DataFlow::Configuration getValueFlowConfig() { none() }
+
+  override DataFlow::Configuration getTaintFlowConfig() { result instanceof Conf }
+}