Java: Add taint step for InputStream wrappers

Author: atorralba

java/ql/lib/semmle/code/java/JDK.qll

@@ -4,6 +4,7 @@
 
 import Member
 import semmle.code.java.security.ExternalProcess
+private import semmle.code.java.dataflow.DataFlow
 private import semmle.code.java.dataflow.FlowSteps
 
 // --- Standard types ---
@@ -177,6 +178,11 @@ class TypeObjectInputStream extends RefType {
   TypeObjectInputStream() { this.hasQualifiedName("java.io", "ObjectInputStream") }
 }
 
+/** The class `java.io.InputStream`. */
+class TypeInputStream extends RefType {
+  TypeInputStream() { this.hasQualifiedName("java.io", "InputStream") }
+}
+
 /** The class `java.nio.file.Paths`. */
 class TypePaths extends Class {
   TypePaths() { this.hasQualifiedName("java.nio.file", "Paths") }
@@ -197,6 +203,48 @@ class TypeFile extends Class {
   TypeFile() { this.hasQualifiedName("java.io", "File") }
 }
 
+/**
+ * A taint step from an update of the `bytes[]` parameter in an override of the `InputStream.read` method
+ * to a class instance expression of the type extending `InputStream`.
+ *
+ * This models how a subtype of `InputStream` could be tainted by the definition of its methods, which will
+ * normally only happen in anonymous classes.
+ */
+private class InputStreamWrapperAnonymousStep extends AdditionalTaintStep {
+  override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
+    exists(Method m, AnonymousClass wrapper |
+      m.hasName("read") and
+      m.getDeclaringType() = wrapper and
+      wrapper.getASourceSupertype+() instanceof TypeInputStream
+    |
+      n1.(DataFlow::PostUpdateNode).getPreUpdateNode().asExpr() = m.getParameter(0).getAnAccess() and
+      n2.asExpr() = wrapper.getClassInstanceExpr()
+    )
+  }
+}
+
+/**
+ * A taint step from an `InputStream` argument of the constructor of an `InputStream` subtype
+ * to the call of the constructor, only if the argument is assigned to a class field.
+ *
+ * This models how it's assumed that an `InputStream` wrapper is tainted by the wrapped stream,
+ * and is a workaround to low `fieldFlowBranchLimit`s in dataflow configurations.
+ */
+private class InputStreamWrapperConstructorStep extends AdditionalTaintStep {
+  override predicate step(DataFlow::Node n1, DataFlow::Node n2) {
+    exists(ClassInstanceExpr cc, Argument a, AssignExpr ae |
+      cc.getConstructedType().getASourceSupertype+() instanceof TypeInputStream and
+      cc.getAnArgument() = a and
+      cc.getCallee().getParameter(a.getParameterPos()).getAnAccess() = ae.getRhs() and
+      ae.getDest().(FieldWrite).getField().getType().(RefType).getASourceSupertype*() instanceof
+        TypeInputStream
+    |
+      n1.asExpr() = a and
+      n2.asExpr() = cc
+    )
+  }
+}
+
 // --- Standard methods ---
 /**
  * DEPRECATED: Any constructor of class `java.lang.ProcessBuilder`.

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

@@ -757,7 +757,7 @@ private predicate baseBound(Expr e, int b, boolean upper) {
   or
   exists(Method read |
     e.(MethodAccess).getMethod().overrides*(read) and
-    read.getDeclaringType().hasQualifiedName("java.io", "InputStream") and
+    read.getDeclaringType() instanceof TypeInputStream and
     read.hasName("read") and
     read.getNumberOfParameters() = 0
   |

java/ql/test/library-tests/dataflow/stream-read/A.java

@@ -0,0 +1,87 @@
+import java.io.InputStream;
+import java.io.IOException;
+
+public class A {
+
+    private static InputStream source() {
+        return null;
+    }
+
+    private static void sink(Object s) {}
+
+    static class MyStream extends InputStream {
+        private InputStream wrapped;
+
+        MyStream(InputStream wrapped) {
+            this.wrapped = wrapped;
+        }
+
+        @Override
+        public int read() throws IOException {
+            return 0;
+        }
+
+        @Override
+        public int read(byte[] b) throws IOException {
+            return wrapped.read(b);
+        }
+    }
+
+    public static void testSeveralWrappers() {
+        InputStream src = source();
+        InputStream wrapper1 = new MyStream(src);
+        sink(wrapper1); // $ hasTaintFlow
+        InputStream wrapper2 = new MyStream(wrapper1);
+        sink(wrapper2); // $ hasTaintFlow
+        InputStream wrapper3 = new MyStream(wrapper2);
+        sink(wrapper3); // $ hasTaintFlow
+
+        InputStream wrapper4 = new InputStream() {
+            @Override
+            public int read() throws IOException {
+                return 0;
+            }
+
+            @Override
+            public int read(byte[] b) throws IOException {
+                return wrapper3.read(b);
+
+            }
+        };
+        sink(wrapper4); // $ hasTaintFlow
+    }
+
+    public static void testAnonymous() throws Exception {
+        InputStream wrapper = new InputStream() {
+            @Override
+            public int read() throws IOException {
+                return 0;
+            }
+
+            @Override
+            public int read(byte[] b) throws IOException {
+                InputStream in = source();
+                return in.read(b);
+            }
+        };
+        sink(wrapper); // $ hasTaintFlow
+    }
+
+    public static void testAnonymousVarCapture() throws Exception {
+        InputStream in = source();
+        InputStream wrapper = new InputStream() {
+            @Override
+            public int read() throws IOException {
+                return 0;
+            }
+
+            @Override
+            public int read(byte[] b) throws IOException {
+                return in.read(b);
+
+            }
+        };
+        sink(wrapper); // $ hasTaintFlow
+    }
+
+}

java/ql/test/library-tests/dataflow/stream-read/test.expected

@@ -0,0 +1,2 @@
+failures
+testFailures

java/ql/test/library-tests/dataflow/stream-read/test.ql

@@ -0,0 +1,2 @@
+import TestUtilities.InlineFlowTest
+import DefaultFlowTest