Merge pull request #6098 from atorralba/atorralba/entrypoint-field-steps

Java: Preserve taint on field-read-steps on entrypoint types

java/ql/lib/change-notes/2021-12-15-tainted-field-read-step-on-entrypoint-types.md

@@ -0,0 +1,4 @@
+---
+category: majorAnalysis
+---
+* Data flow now propagates taint from remote source `Parameter` types to read steps of their fields (e.g. `tainted.publicField` or `tainted.getField()`). This also applies to their subtypes and the types of their fields, recursively.

java/ql/lib/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll

@@ -11,6 +11,7 @@ private import semmle.code.java.frameworks.spring.SpringController
 private import semmle.code.java.frameworks.spring.SpringHttp
 private import semmle.code.java.frameworks.Networking
 private import semmle.code.java.dataflow.ExternalFlow
+private import semmle.code.java.dataflow.FlowSources
 private import semmle.code.java.dataflow.internal.DataFlowPrivate
 import semmle.code.java.dataflow.FlowSteps
 private import FlowSummaryImpl as FlowSummaryImpl
@@ -91,6 +92,8 @@ private module Cached {
     )
     or
     FlowSummaryImpl::Private::Steps::summaryLocalStep(src, sink, false)
+    or
+    entrypointFieldStep(src, sink)
   }
 
   /**
@@ -591,3 +594,19 @@ private MethodAccess callReturningSameType(Expr ref) {
   ref = result.getQualifier() and
   result.getMethod().getReturnType() = ref.getType()
 }
+
+private SrcRefType entrypointType() {
+  exists(RemoteFlowSource s, RefType t |
+    s instanceof DataFlow::ExplicitParameterNode and
+    t = pragma[only_bind_out](s).getType() and
+    not t instanceof TypeObject and
+    result = t.getASubtype*().getSourceDeclaration()
+  )
+  or
+  result = entrypointType().getAField().getType().(RefType).getSourceDeclaration()
+}
+
+private predicate entrypointFieldStep(DataFlow::Node src, DataFlow::Node sink) {
+  src = DataFlow::getFieldQualifier(sink.asExpr().(FieldRead)) and
+  src.getType().(RefType).getSourceDeclaration() = entrypointType()
+}

java/ql/test/library-tests/dataflow/entrypoint-types/EntryPointTypesTest.java

@@ -0,0 +1,73 @@
+public class EntryPointTypesTest {
+
+    static class TestObject {
+        public String field1;
+        private String field2;
+        private AnotherTestObject field3;
+
+        public String getField2() {
+            return field2;
+        }
+
+        public AnotherTestObject getField3() {
+            return field3;
+        }
+    }
+
+    static class AnotherTestObject {
+        public String field4;
+        private String field5;
+
+        public String getField5() {
+            return field5;
+        }
+    }
+
+    static class ParameterizedTestObject<T, K> {
+        public String field6;
+        public T field7;
+        private K field8;
+
+        public K getField8() {
+            return field8;
+        }
+    }
+
+    static class ChildObject extends ParameterizedTestObject<TestObject, Object> {
+        public Object field9;
+    }
+
+    class UnrelatedObject {
+        public String safeField;
+    }
+
+    private static void sink(String sink) {}
+
+    public static void test(TestObject source) {
+        sink(source.field1); // $hasTaintFlow
+        sink(source.getField2()); // $hasTaintFlow
+        sink(source.getField3().field4); // $hasTaintFlow
+        sink(source.getField3().getField5()); // $hasTaintFlow
+    }
+
+    public static void testParameterized(
+            ParameterizedTestObject<TestObject, AnotherTestObject> source) {
+        sink(source.field6); // $hasTaintFlow
+        sink(source.field7.field1); // $hasTaintFlow
+        sink(source.field7.getField2()); // $hasTaintFlow
+        sink(source.getField8().field4); // $hasTaintFlow
+        sink(source.getField8().getField5()); // $hasTaintFlow
+    }
+
+    public static void testSubtype(ParameterizedTestObject<?, ?> source) {
+        ChildObject subtypeSource = (ChildObject) source;
+        sink(subtypeSource.field6); // $hasTaintFlow
+        sink(subtypeSource.field7.field1); // $hasTaintFlow
+        sink(subtypeSource.field7.getField2()); // $hasTaintFlow
+        sink((String) subtypeSource.getField8()); // $hasTaintFlow
+        sink((String) subtypeSource.field9); // $hasTaintFlow
+        // Ensure that we are not tainting every subclass of Object
+        UnrelatedObject unrelated = (UnrelatedObject) subtypeSource.getField8();
+        sink(unrelated.safeField); // Safe
+    }
+}

java/ql/test/library-tests/dataflow/entrypoint-types/EntryPointTypesTest.ql

@@ -0,0 +1,34 @@
+import java
+import semmle.code.java.dataflow.FlowSources
+import TestUtilities.InlineExpectationsTest
+
+class TestRemoteFlowSource extends RemoteFlowSource {
+  TestRemoteFlowSource() { this.asParameter().hasName("source") }
+
+  override string getSourceType() { result = "test" }
+}
+
+class TaintFlowConf extends TaintTracking::Configuration {
+  TaintFlowConf() { this = "qltest:dataflow:entrypoint-types-taint" }
+
+  override predicate isSource(DataFlow::Node n) { n instanceof RemoteFlowSource }
+
+  override predicate isSink(DataFlow::Node n) {
+    exists(MethodAccess ma | ma.getMethod().hasName("sink") | n.asExpr() = ma.getAnArgument())
+  }
+}
+
+class HasFlowTest extends InlineExpectationsTest {
+  HasFlowTest() { this = "HasFlowTest" }
+
+  override string getARelevantTag() { result = ["hasTaintFlow"] }
+
+  override predicate hasActualResult(Location location, string element, string tag, string value) {
+    tag = "hasTaintFlow" and
+    exists(DataFlow::Node src, DataFlow::Node sink, TaintFlowConf conf | conf.hasFlow(src, sink) |
+      sink.getLocation() = location and
+      element = sink.toString() and
+      value = ""
+    )
+  }
+}