Use more file openning methods

Author: joefarebrother

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

@@ -1,6 +1,7 @@
 /** 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
 
@@ -41,10 +42,9 @@ private predicate externalStorageFlow(DataFlow::Node node1, DataFlow::Node node2
  * This is controlable by third-party applications, so is treated as a remote flow source.
  */
 predicate androidExternalStorageSource(DataFlow::Node n) {
-  exists(ConstructorCall fInp, DataFlow::Node externalDir |
-    fInp.getConstructedType().hasQualifiedName("java.io", "FileInputStream") and
-    n.asExpr() = fInp and
+  exists(DataFlow::Node externalDir, DirectFileReadExpr read |
     sourceNode(externalDir, "android-external-storage-dir") and
-    externalStorageFlow(externalDir, DataFlow::exprNode(fInp.getArgument(0)))
+    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) {
+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

@@ -2,6 +2,8 @@
 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;
 
@@ -48,4 +50,10 @@ 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 
+    }
  }