From 551d86c6eae8c2ff5e89509a4e449ab058970d98 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Mon, 5 Oct 2020 11:33:12 +0100
Subject: [PATCH 01/16] Java: Define classes for taint propagation methods

---
 .../java/dataflow/TaintTrackingFrameworks.qll | 12 +++++
 .../dataflow/internal/TaintTrackingUtil.qll   | 48 +++++++++++++++----
 2 files changed, 52 insertions(+), 8 deletions(-)
 create mode 100644 java/ql/src/semmle/code/java/dataflow/TaintTrackingFrameworks.qll

diff --git a/java/ql/src/semmle/code/java/dataflow/TaintTrackingFrameworks.qll b/java/ql/src/semmle/code/java/dataflow/TaintTrackingFrameworks.qll
new file mode 100644
index 000000000000..26463c33d1ee
--- /dev/null
+++ b/java/ql/src/semmle/code/java/dataflow/TaintTrackingFrameworks.qll
@@ -0,0 +1,12 @@
+/**
+ * Provides taint tracking information for frameworks.
+ * When a new framework is added that provides taint tracking steps, it should be imported here.
+ */
+
+import semmle.code.java.frameworks.jackson.JacksonSerializability
+import semmle.code.java.frameworks.android.Intent
+import semmle.code.java.frameworks.android.SQLite
+import semmle.code.java.frameworks.Guice
+import semmle.code.java.frameworks.Protobuf
+import semmle.code.java.frameworks.spring.SpringController
+import semmle.code.java.frameworks.spring.SpringHttp
diff --git a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
index 8818dc37b1a6..833e849680f2 100644
--- a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
+++ b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
@@ -5,15 +5,9 @@ private import semmle.code.java.dataflow.SSA
 private import semmle.code.java.dataflow.DefUse
 private import semmle.code.java.security.SecurityTests
 private import semmle.code.java.security.Validation
-private import semmle.code.java.frameworks.android.Intent
-private import semmle.code.java.frameworks.android.SQLite
-private import semmle.code.java.frameworks.Guice
-private import semmle.code.java.frameworks.Protobuf
-private import semmle.code.java.frameworks.spring.SpringController
-private import semmle.code.java.frameworks.spring.SpringHttp
 private import semmle.code.java.Maps
 private import semmle.code.java.dataflow.internal.ContainerFlow
-private import semmle.code.java.frameworks.jackson.JacksonSerializability
+private import semmle.code.java.dataflow.TaintTrackingFrameworks
 
 /**
  * Holds if taint can flow from `src` to `sink` in zero or more
@@ -78,6 +72,34 @@ predicate defaultAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) {
   any(AdditionalTaintStep a).step(src, sink)
 }
 
+/**
+ * A method that returns tainted data when one of its inputs (an argument or the qualifier) are tainted.
+ *
+ * Extend this class to add additional taint steps through a method that should
+ * apply to all taint configurations.
+ */
+abstract class TaintPreservingMethod extends Method {
+  /**
+   * Holds if this method returns tainted data when `arg` tainted.
+   * `arg` is a parameter index, or is -1 to indicate the qualifier.
+   */
+  abstract predicate returnsTaint(int arg);
+}
+
+/**
+ * A method that transfers taint from one of its inputs (an argument or the qualifier) to another.
+ *
+ * Extend this class to add additional taint steps through a method that should
+ * apply to all taint configurations.
+ */
+abstract class TaintTransferringMethod extends Method {
+  /**
+   * Holds if this method writes tainted data to `sink` when `src` is tainted.
+   * `src` and `sink` are parameter indices, or -1 to indicate the qualifier.
+   */
+  predicate transfersTaint(int src, int sink) { none() }
+}
+
 /**
  * Holds if `node` should be a sanitizer in all global taint flow configurations
  * but not in local taint.
@@ -300,6 +322,8 @@ private predicate taintPreservingQualifierToArgument(Method m, int arg) {
   m.getDeclaringType().getASupertype*().hasQualifiedName("java.io", "Reader") and
   m.hasName("read") and
   arg = 0
+  or
+  m.(TaintTransferringMethod).transfersTaint(-1, arg)
 }
 
 /** Access to a method that passes taint from the qualifier. */
@@ -412,6 +436,8 @@ private predicate taintPreservingQualifierToMethod(Method m) {
   // buildUnionSubQuery(String typeDiscriminatorColumn, String[] unionColumns, Set<String> columnsPresentInTable, int computedColumnsOffset, String typeDiscriminatorValue, String selection, String[] selectionArgs, String groupBy, String having)
   // buildUnionSubQuery(String typeDiscriminatorColumn, String[] unionColumns, Set<String> columnsPresentInTable, int computedColumnsOffset, String typeDiscriminatorValue, String selection, String groupBy, String having)
   m.hasName(["buildQuery", "buildUnionQuery", "buildUnionSubQuery"])
+  or
+  m.(TaintPreservingMethod).returnsTaint(-1)
 }
 
 private class StringReplaceMethod extends Method {
@@ -429,7 +455,7 @@ private predicate unsafeEscape(MethodAccess ma) {
   // Removing `<script>` tags using a string-replace method is
   // unsafe if such a tag is embedded inside another one (e.g. `<scr<script>ipt>`).
   exists(StringReplaceMethod m | ma.getMethod() = m |
-    ma.getArgument(0).(StringLiteral).getRepresentedString() = "(<script>)" and
+    ma.getArgument(0).(StringLiteral).getRepresentedString() = "<script>" and
     ma.getArgument(1).(StringLiteral).getRepresentedString() = ""
   )
 }
@@ -620,6 +646,8 @@ private predicate taintPreservingArgumentToMethod(Method method, int arg) {
   // Cursor query(Uri uri, String[] projection, String selection, String[] selectionArgs, String sortOrder)
   method.hasName("query") and
   arg = 0
+  or
+  method.(TaintPreservingMethod).returnsTaint(arg)
 }
 
 /**
@@ -678,6 +706,8 @@ private predicate taintPreservingArgToArg(Method method, int input, int output)
   method.hasName("appendColumns") and
   input = 1 and
   output = 0
+  or
+  method.(TaintTransferringMethod).transfersTaint(input, output)
 }
 
 /**
@@ -737,6 +767,8 @@ private predicate taintPreservingArgumentToQualifier(Method method, int arg) {
   // appendWhereStandalone(CharSequence inWhere)
   method.hasName(["setProjectionMap", "setTables", "appendWhere", "appendWhereStandalone"]) and
   arg = 0
+  or
+  method.(TaintTransferringMethod).transfersTaint(arg, -1)
 }
 
 /** A comparison or equality test with a constant. */

From ff6c5c219c84c1a765f48cdbea25c8f0fd94be4b Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Tue, 6 Oct 2020 11:11:24 +0100
Subject: [PATCH 02/16] Java: Start TaintTrackingUtils refactor

---
 .../dataflow/internal/TaintTrackingUtil.qll   | 91 +++++++------------
 .../jackson/JacksonSerializability.qll        | 14 ++-
 2 files changed, 48 insertions(+), 57 deletions(-)

diff --git a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
index 833e849680f2..f4f1f1765433 100644
--- a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
+++ b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
@@ -100,6 +100,24 @@ abstract class TaintTransferringMethod extends Method {
   predicate transfersTaint(int src, int sink) { none() }
 }
 
+private class StringTaintPreservingMethod extends TaintPreservingMethod {
+  StringTaintPreservingMethod() {
+    getDeclaringType() instanceof TypeString and
+    hasName(["concat", "copyValueOf", "endsWith", "format", "formatted", "getBytes", "indent",
+          "intern", "join", "repeat", "split", "strip", "stripIndent", "stripLeading",
+          "stripTrailing", "substring", "toCharArray", "toLowerCase", "toString", "toUpperCase",
+          "trim"])
+  }
+
+  override predicate returnsTaint(int arg) {
+    arg = -1
+    or
+    this.hasName(["concat", "copyValueOf"]) and arg = 0
+    or
+    this.hasName(["format", "formatted", "join"]) and arg = [0 .. getNumberOfParameters()]
+  }
+}
+
 /**
  * Holds if `node` should be a sanitizer in all global taint flow configurations
  * but not in local taint.
@@ -338,21 +356,6 @@ private predicate qualifierToMethodStep(Expr tracked, MethodAccess sink) {
 private predicate taintPreservingQualifierToMethod(Method m) {
   m instanceof CloneMethod
   or
-  m.getDeclaringType() instanceof TypeString and
-  (
-    m.getName() = "concat" or
-    m.getName() = "endsWith" or
-    m.getName() = "formatted" or
-    m.getName() = "getBytes" or
-    m.getName() = "split" or
-    m.getName() = "substring" or
-    m.getName() = "toCharArray" or
-    m.getName() = "toLowerCase" or
-    m.getName() = "toString" or
-    m.getName() = "toUpperCase" or
-    m.getName() = "trim"
-  )
-  or
   exists(Class c | c.getQualifiedName() = "java.lang.Number" | hasSubtype*(c, m.getDeclaringType())) and
   (
     m.getName().matches("to%String") or
@@ -426,9 +429,6 @@ private predicate taintPreservingQualifierToMethod(Method m) {
     )
   )
   or
-  m.getDeclaringType() instanceof TypeFormatter and
-  m.hasName(["format", "out"])
-  or
   m.getDeclaringType().getASourceSupertype*() instanceof TypeSQLiteQueryBuilder and
   // buildQuery(String[] projectionIn, String selection, String groupBy, String having, String sortOrder, String limit)
   // buildQuery(String[] projectionIn, String selection, String[] selectionArgs, String groupBy, String having, String sortOrder, String limit)
@@ -440,7 +440,7 @@ private predicate taintPreservingQualifierToMethod(Method m) {
   m.(TaintPreservingMethod).returnsTaint(-1)
 }
 
-private class StringReplaceMethod extends Method {
+private class StringReplaceMethod extends TaintPreservingMethod {
   StringReplaceMethod() {
     getDeclaringType() instanceof TypeString and
     (
@@ -449,6 +449,8 @@ private class StringReplaceMethod extends Method {
       hasName("replaceFirst")
     )
   }
+
+  override predicate returnsTaint(int arg) { arg = 1 }
 }
 
 private predicate unsafeEscape(MethodAccess ma) {
@@ -496,12 +498,6 @@ private predicate argToMethodStep(Expr tracked, MethodAccess sink) {
  * of its arguments are tainted.
  */
 private predicate taintPreservingArgumentToMethod(Method method) {
-  method.getDeclaringType() instanceof TypeString and
-  (method.hasName("format") or method.hasName("formatted") or method.hasName("join"))
-  or
-  method.getDeclaringType() instanceof TypeFormatter and
-  method.hasName("format")
-  or
   method.getDeclaringType() instanceof TypeDatabaseUtils and
   // String[] appendSelectionArgs(String[] originalValues, String[] newValues)
   // String concatenateWhere(String a, String b)
@@ -519,8 +515,6 @@ private predicate taintPreservingArgumentToMethod(Method method) {
  * `arg`th argument is tainted.
  */
 private predicate taintPreservingArgumentToMethod(Method method, int arg) {
-  method instanceof StringReplaceMethod and arg = 1
-  or
   exists(Class c | c.getQualifiedName() = "java.lang.Number" |
     hasSubtype*(c, method.getDeclaringType())
   ) and
@@ -532,10 +526,6 @@ private predicate taintPreservingArgumentToMethod(Method method, int arg) {
     method.getName().matches("to%String") and arg = 0
   )
   or
-  method.getDeclaringType() instanceof TypeString and
-  method.getName() = "concat" and
-  arg = 0
-  or
   (
     method.getDeclaringType().hasQualifiedName("java.lang", "StringBuilder") or
     method.getDeclaringType().hasQualifiedName("java.lang", "StringBuffer")
@@ -617,11 +607,6 @@ private predicate taintPreservingArgumentToMethod(Method method, int arg) {
   exists(ProtobufMessageLite m | method = m.getAParseFromMethod()) and
   arg = 0
   or
-  // Jackson serialization methods that return the serialized data
-  method instanceof JacksonWriteValueMethod and
-  method.getNumberOfParameters() = 1 and
-  arg = 0
-  or
   method.getDeclaringType().hasQualifiedName("java.io", "StringWriter") and
   method.hasName("append") and
   arg = 0
@@ -695,12 +680,6 @@ private predicate taintPreservingArgToArg(Method method, int input, int output)
   input = 0 and
   output = 2
   or
-  // Jackson serialization methods that write data to the first argument
-  method instanceof JacksonWriteValueMethod and
-  method.getNumberOfParameters() > 1 and
-  input = method.getNumberOfParameters() - 1 and
-  output = 0
-  or
   method.getDeclaringType() instanceof TypeSQLiteQueryBuilder and
   // static appendColumns(StringBuilder s, String[] columns)
   method.hasName("appendColumns") and
@@ -721,20 +700,6 @@ private predicate argToQualifierStep(Expr tracked, Expr sink) {
     tracked = ma.getArgument(i) and
     sink = ma.getQualifier()
   )
-  or
-  exists(MethodAccess ma |
-    taintPreservingArgumentToQualifier(ma.getMethod()) and
-    tracked = ma.getAnArgument() and
-    sink = ma.getQualifier()
-  )
-}
-
-/**
- * Holds if `method` is a method that transfers taint from any of its arguments to its qualifier.
- */
-private predicate taintPreservingArgumentToQualifier(Method method) {
-  method.getDeclaringType() instanceof TypeFormatter and
-  method.hasName("format")
 }
 
 /**
@@ -892,6 +857,20 @@ private class TypeFormatter extends Class {
   TypeFormatter() { this.hasQualifiedName("java.util", "Formatter") }
 }
 
+private class FormatterMethod extends TaintPreservingMethod, TaintTransferringMethod {
+  FormatterMethod() {
+    getDeclaringType() instanceof TypeFormatter and
+    hasName(["format", "out", "toString"])
+  }
+
+  override predicate returnsTaint(int arg) { arg = [-1 .. getNumberOfParameters()] }
+
+  override predicate transfersTaint(int src, int sink) {
+    sink = -1 and
+    src = [0 .. getNumberOfParameters()]
+  }
+}
+
 private import StringBuilderVarModule
 
 module StringBuilderVarModule {
diff --git a/java/ql/src/semmle/code/java/frameworks/jackson/JacksonSerializability.qll b/java/ql/src/semmle/code/java/frameworks/jackson/JacksonSerializability.qll
index 99d733671628..68e23d82670f 100644
--- a/java/ql/src/semmle/code/java/frameworks/jackson/JacksonSerializability.qll
+++ b/java/ql/src/semmle/code/java/frameworks/jackson/JacksonSerializability.qll
@@ -4,6 +4,7 @@
  */
 
 import java
+import semmle.code.java.dataflow.TaintTracking::TaintTracking as TT
 import semmle.code.java.Serializability
 import semmle.code.java.Reflection
 import semmle.code.java.dataflow.DataFlow
@@ -27,7 +28,7 @@ abstract class JacksonSerializableType extends Type { }
  * A method used for serializing objects using Jackson. The final parameter is the object to be
  * serialized.
  */
-library class JacksonWriteValueMethod extends Method {
+library class JacksonWriteValueMethod extends TT::TaintPreservingMethod, TT::TaintTransferringMethod {
   JacksonWriteValueMethod() {
     (
       getDeclaringType().hasQualifiedName("com.fasterxml.jackson.databind", "ObjectWriter") or
@@ -36,6 +37,17 @@ library class JacksonWriteValueMethod extends Method {
     getName().matches("writeValue%") and
     getParameter(getNumberOfParameters() - 1).getType() instanceof TypeObject
   }
+
+  override predicate returnsTaint(int arg) {
+    getNumberOfParameters() = 1 and
+    arg = 0
+  }
+
+  override predicate transfersTaint(int src, int sink) {
+    getNumberOfParameters() > 1 and
+    src = getNumberOfParameters() - 1 and
+    sink = 0
+  }
 }
 
 /** A type whose values are explicitly serialized in a call to a Jackson method. */

From ca60f2cc18097f16abdb3dad6332cc1ca870555c Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Tue, 6 Oct 2020 13:49:02 +0100
Subject: [PATCH 03/16] Java: Fix failing tests

---
 .../semmle/code/java/dataflow/internal/TaintTrackingUtil.qll    | 2 +-
 java/ql/test/library-tests/dataflow/taint-format/test.expected  | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
index f4f1f1765433..93186afb128f 100644
--- a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
+++ b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
@@ -457,7 +457,7 @@ private predicate unsafeEscape(MethodAccess ma) {
   // Removing `<script>` tags using a string-replace method is
   // unsafe if such a tag is embedded inside another one (e.g. `<scr<script>ipt>`).
   exists(StringReplaceMethod m | ma.getMethod() = m |
-    ma.getArgument(0).(StringLiteral).getRepresentedString() = "<script>" and
+    ma.getArgument(0).(StringLiteral).getRepresentedString() = "(<script>)" and
     ma.getArgument(1).(StringLiteral).getRepresentedString() = ""
   )
 }
diff --git a/java/ql/test/library-tests/dataflow/taint-format/test.expected b/java/ql/test/library-tests/dataflow/taint-format/test.expected
index c52918fadc5e..33abce9ebec6 100644
--- a/java/ql/test/library-tests/dataflow/taint-format/test.expected
+++ b/java/ql/test/library-tests/dataflow/taint-format/test.expected
@@ -13,6 +13,7 @@
 | A.java:20:22:20:28 | taint(...) | A.java:24:9:24:27 | new ..[] { .. } |
 | A.java:20:22:20:28 | taint(...) | A.java:24:24:24:26 | bad |
 | A.java:20:22:20:28 | taint(...) | A.java:25:9:25:9 | f |
+| A.java:20:22:20:28 | taint(...) | A.java:25:9:25:20 | toString(...) |
 | A.java:29:22:29:28 | taint(...) | A.java:29:22:29:28 | taint(...) |
 | A.java:29:22:29:28 | taint(...) | A.java:33:9:33:10 | sb |
 | A.java:29:22:29:28 | taint(...) | A.java:33:9:33:21 | toString(...) |

From 60a7666105309176686bfbe0742b14036e18ce25 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Tue, 6 Oct 2020 16:50:44 +0100
Subject: [PATCH 04/16] Java: Refactor Android SQLite flow steps

---
 .../dataflow/internal/TaintTrackingUtil.qll   | 59 +------------------
 .../code/java/frameworks/android/SQLite.qll   | 57 ++++++++++++++++++
 .../jackson/JacksonSerializability.qll        |  2 +-
 3 files changed, 59 insertions(+), 59 deletions(-)

diff --git a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
index 93186afb128f..344e2e73a227 100644
--- a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
+++ b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
@@ -97,7 +97,7 @@ abstract class TaintTransferringMethod extends Method {
    * Holds if this method writes tainted data to `sink` when `src` is tainted.
    * `src` and `sink` are parameter indices, or -1 to indicate the qualifier.
    */
-  predicate transfersTaint(int src, int sink) { none() }
+  abstract predicate transfersTaint(int src, int sink);
 }
 
 private class StringTaintPreservingMethod extends TaintPreservingMethod {
@@ -429,14 +429,6 @@ private predicate taintPreservingQualifierToMethod(Method m) {
     )
   )
   or
-  m.getDeclaringType().getASourceSupertype*() instanceof TypeSQLiteQueryBuilder and
-  // buildQuery(String[] projectionIn, String selection, String groupBy, String having, String sortOrder, String limit)
-  // buildQuery(String[] projectionIn, String selection, String[] selectionArgs, String groupBy, String having, String sortOrder, String limit)
-  // buildUnionQuery(String[] subQueries, String sortOrder, String limit)
-  // buildUnionSubQuery(String typeDiscriminatorColumn, String[] unionColumns, Set<String> columnsPresentInTable, int computedColumnsOffset, String typeDiscriminatorValue, String selection, String[] selectionArgs, String groupBy, String having)
-  // buildUnionSubQuery(String typeDiscriminatorColumn, String[] unionColumns, Set<String> columnsPresentInTable, int computedColumnsOffset, String typeDiscriminatorValue, String selection, String groupBy, String having)
-  m.hasName(["buildQuery", "buildUnionQuery", "buildUnionSubQuery"])
-  or
   m.(TaintPreservingMethod).returnsTaint(-1)
 }
 
@@ -470,12 +462,6 @@ private predicate argToMethodStep(Expr tracked, MethodAccess sink) {
     tracked = sink.getArgument(i)
   )
   or
-  exists(MethodAccess ma |
-    taintPreservingArgumentToMethod(ma.getMethod()) and
-    tracked = ma.getAnArgument() and
-    sink = ma
-  )
-  or
   exists(Method springResponseEntityOfOk |
     sink.getMethod() = springResponseEntityOfOk and
     springResponseEntityOfOk.getDeclaringType() instanceof SpringResponseEntity and
@@ -493,23 +479,6 @@ private predicate argToMethodStep(Expr tracked, MethodAccess sink) {
   )
 }
 
-/**
- * Holds if `method` is a library method that returns tainted data if any
- * of its arguments are tainted.
- */
-private predicate taintPreservingArgumentToMethod(Method method) {
-  method.getDeclaringType() instanceof TypeDatabaseUtils and
-  // String[] appendSelectionArgs(String[] originalValues, String[] newValues)
-  // String concatenateWhere(String a, String b)
-  method.hasName(["appendSelectionArgs", "concatenateWhere"])
-  or
-  method.getDeclaringType().getASourceSupertype*() instanceof TypeSQLiteQueryBuilder and
-  // buildQuery(String[] projectionIn, String selection, String groupBy, String having, String sortOrder, String limit)
-  // buildQuery(String[] projectionIn, String selection, String[] selectionArgs, String groupBy, String having, String sortOrder, String limit)
-  // buildUnionQuery(String[] subQueries, String sortOrder, String limit)
-  method.hasName(["buildQuery", "buildUnionQuery"])
-}
-
 /**
  * Holds if `method` is a library method that returns tainted data if its
  * `arg`th argument is tainted.
@@ -611,18 +580,6 @@ private predicate taintPreservingArgumentToMethod(Method method, int arg) {
   method.hasName("append") and
   arg = 0
   or
-  method.getDeclaringType().getASourceSupertype*() instanceof TypeSQLiteQueryBuilder and
-  (
-    // static buildQueryString(boolean distinct, String tables, String[] columns, String where, String groupBy, String having, String orderBy, String limit)
-    method.hasName("buildQueryString") and arg = [1 .. method.getNumberOfParameters()]
-    or
-    // buildUnionSubQuery(String typeDiscriminatorColumn, String[] unionColumns, Set<String> columnsPresentInTable, int computedColumnsOffset, String typeDiscriminatorValue, String selection, String[] selectionArgs, String groupBy, String having)
-    // buildUnionSubQuery(String typeDiscriminatorColumn, String[] unionColumns, Set<String> columnsPresentInTable, int computedColumnsOffset, String typeDiscriminatorValue, String selection, String groupBy, String having)
-    method.hasName("buildUnionSubQuery") and
-    arg = [0 .. method.getNumberOfParameters()] and
-    arg != 3
-  )
-  or
   (
     method.getDeclaringType() instanceof AndroidContentProvider or
     method.getDeclaringType() instanceof AndroidContentResolver
@@ -680,12 +637,6 @@ private predicate taintPreservingArgToArg(Method method, int input, int output)
   input = 0 and
   output = 2
   or
-  method.getDeclaringType() instanceof TypeSQLiteQueryBuilder and
-  // static appendColumns(StringBuilder s, String[] columns)
-  method.hasName("appendColumns") and
-  input = 1 and
-  output = 0
-  or
   method.(TaintTransferringMethod).transfersTaint(input, output)
 }
 
@@ -725,14 +676,6 @@ private predicate taintPreservingArgumentToQualifier(Method method, int arg) {
     append.getDeclaringType().hasQualifiedName("java.io", "StringWriter")
   )
   or
-  method.getDeclaringType().getASourceSupertype*() instanceof TypeSQLiteQueryBuilder and
-  // setProjectionMap(Map<String, String> columnMap)
-  // setTables(String inTables)
-  // appendWhere(CharSequence inWhere)
-  // appendWhereStandalone(CharSequence inWhere)
-  method.hasName(["setProjectionMap", "setTables", "appendWhere", "appendWhereStandalone"]) and
-  arg = 0
-  or
   method.(TaintTransferringMethod).transfersTaint(arg, -1)
 }
 
diff --git a/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll b/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll
index e0b41027232e..288130401c4f 100644
--- a/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll
+++ b/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll
@@ -1,5 +1,6 @@
 import java
 import Android
+private import semmle.code.java.dataflow.TaintTracking::TaintTracking as TT
 
 /**
  * The class `android.database.sqlite.SQLiteDatabase`.
@@ -226,3 +227,59 @@ private class ContentProviderUpdateMethod extends SQLiteRunner {
 
   override int sqlIndex() { result = 2 }
 }
+
+private class QueryBuilderBuildMethod extends TT::TaintPreservingMethod {
+  QueryBuilderBuildMethod() {
+    this.getDeclaringType().getASourceSupertype*() instanceof TypeSQLiteQueryBuilder and
+    // buildQuery(String[] projectionIn, String selection, String groupBy, String having, String sortOrder, String limit)
+    // buildQuery(String[] projectionIn, String selection, String[] selectionArgs, String groupBy, String having, String sortOrder, String limit)
+    // buildUnionQuery(String[] subQueries, String sortOrder, String limit)
+    // buildUnionSubQuery(String typeDiscriminatorColumn, String[] unionColumns, Set<String> columnsPresentInTable, int computedColumnsOffset, String typeDiscriminatorValue, String selection, String[] selectionArgs, String groupBy, String having)
+    // buildUnionSubQuery(String typeDiscriminatorColumn, String[] unionColumns, Set<String> columnsPresentInTable, int computedColumnsOffset, String typeDiscriminatorValue, String selection, String groupBy, String having)
+    // static buildQueryString(boolean distinct, String tables, String[] columns, String where, String groupBy, String having, String orderBy, String limit)
+    this.hasName(["buildQuery", "buildUnionQuery", "buildUnionSubQuery", "buildQueryString"])
+  }
+
+  override predicate returnsTaint(int arg) {
+    arg = -1
+    or
+    hasName(["buildQuery", "buildUnionQuery"]) and
+    arg = [0 .. getNumberOfParameters()]
+    or
+    hasName("buildQueryString") and
+    arg = [1 .. getNumberOfParameters()]
+    or
+    hasName("buildUnionSubQuery") and
+    arg = [0 .. getNumberOfParameters()] and
+    arg != 3
+  }
+}
+
+private class QueryBuilderAppendMethod extends TT::TaintTransferringMethod {
+  QueryBuilderAppendMethod() {
+    this.getDeclaringType().getASourceSupertype*() instanceof TypeSQLiteQueryBuilder and
+    // setProjectionMap(Map<String, String> columnMap)
+    // setTables(String inTables)
+    // appendWhere(CharSequence inWhere)
+    // appendWhereStandalone(CharSequence inWhere)
+    // static appendColumns(StringBuilder s, String[] columns)
+    this
+        .hasName(["setProjectionMap", "setTables", "appendWhere", "appendWhereStandalone",
+              "appendColumns"])
+  }
+
+  override predicate transfersTaint(int src, int sink) {
+    if hasName("appendColumns") then (src = 1 and sink = 0) else (src = 0 and sink = -1)
+  }
+}
+
+private class UnsafeAppendUtilMethod extends TT::TaintPreservingMethod {
+  UnsafeAppendUtilMethod() {
+    this.getDeclaringType() instanceof TypeDatabaseUtils and
+    // String[] appendSelectionArgs(String[] originalValues, String[] newValues)
+    // String concatenateWhere(String a, String b)
+    this.hasName(["appendSelectionArgs", "concatenateWhere"])
+  }
+
+  override predicate returnsTaint(int arg) { arg = [0 .. getNumberOfParameters()] }
+}
diff --git a/java/ql/src/semmle/code/java/frameworks/jackson/JacksonSerializability.qll b/java/ql/src/semmle/code/java/frameworks/jackson/JacksonSerializability.qll
index 68e23d82670f..c00762244a73 100644
--- a/java/ql/src/semmle/code/java/frameworks/jackson/JacksonSerializability.qll
+++ b/java/ql/src/semmle/code/java/frameworks/jackson/JacksonSerializability.qll
@@ -4,11 +4,11 @@
  */
 
 import java
-import semmle.code.java.dataflow.TaintTracking::TaintTracking as TT
 import semmle.code.java.Serializability
 import semmle.code.java.Reflection
 import semmle.code.java.dataflow.DataFlow
 import semmle.code.java.dataflow.DataFlow5
+private import semmle.code.java.dataflow.TaintTracking::TaintTracking as TT
 
 /**
  * A `@com.fasterxml.jackson.annotation.JsonIgnore` annoation.

From 92fd8c4128f50667ab8a78de371513b10af9e7be Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Tue, 6 Oct 2020 17:37:01 +0100
Subject: [PATCH 05/16] Java: Move new definitions to new file

---
 .../semmle/code/java/dataflow/FlowSteps.qll   | 51 +++++++++++++++++++
 .../TaintTrackingFrameworks.qll               |  0
 .../dataflow/internal/TaintTrackingUtil.qll   | 49 +-----------------
 .../code/java/frameworks/android/SQLite.qll   |  8 +--
 .../jackson/JacksonSerializability.qll        |  4 +-
 5 files changed, 59 insertions(+), 53 deletions(-)
 create mode 100644 java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
 rename java/ql/src/semmle/code/java/dataflow/{ => internal}/TaintTrackingFrameworks.qll (100%)

diff --git a/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll b/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
new file mode 100644
index 000000000000..eb34ed0b4f06
--- /dev/null
+++ b/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
@@ -0,0 +1,51 @@
+/**
+ * Provides classes representing various flow steps for taint tracking.
+ */
+
+import java
+
+/**
+ * A method that returns tainted data when one of its inputs (an argument or the qualifier) are tainted.
+ *
+ * Extend this class to add additional taint steps through a method that should
+ * apply to all taint configurations.
+ */
+abstract class TaintPreservingMethod extends Method {
+  /**
+   * Holds if this method returns tainted data when `arg` tainted.
+   * `arg` is a parameter index, or is -1 to indicate the qualifier.
+   */
+  abstract predicate returnsTaint(int arg);
+}
+
+/**
+ * A method that transfers taint from one of its inputs (an argument or the qualifier) to another.
+ *
+ * Extend this class to add additional taint steps through a method that should
+ * apply to all taint configurations.
+ */
+abstract class TaintTransferringMethod extends Method {
+  /**
+   * Holds if this method writes tainted data to `sink` when `src` is tainted.
+   * `src` and `sink` are parameter indices, or -1 to indicate the qualifier.
+   */
+  abstract predicate transfersTaint(int src, int sink);
+}
+
+private class StringTaintPreservingMethod extends TaintPreservingMethod {
+  StringTaintPreservingMethod() {
+    getDeclaringType() instanceof TypeString and
+    hasName(["concat", "copyValueOf", "endsWith", "format", "formatted", "getBytes", "indent",
+          "intern", "join", "repeat", "split", "strip", "stripIndent", "stripLeading",
+          "stripTrailing", "substring", "toCharArray", "toLowerCase", "toString", "toUpperCase",
+          "trim"])
+  }
+
+  override predicate returnsTaint(int arg) {
+    arg = -1
+    or
+    this.hasName(["concat", "copyValueOf"]) and arg = 0
+    or
+    this.hasName(["format", "formatted", "join"]) and arg = [0 .. getNumberOfParameters()]
+  }
+}
diff --git a/java/ql/src/semmle/code/java/dataflow/TaintTrackingFrameworks.qll b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingFrameworks.qll
similarity index 100%
rename from java/ql/src/semmle/code/java/dataflow/TaintTrackingFrameworks.qll
rename to java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingFrameworks.qll
diff --git a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
index 344e2e73a227..4ad32af76e3c 100644
--- a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
+++ b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
@@ -7,7 +7,8 @@ private import semmle.code.java.security.SecurityTests
 private import semmle.code.java.security.Validation
 private import semmle.code.java.Maps
 private import semmle.code.java.dataflow.internal.ContainerFlow
-private import semmle.code.java.dataflow.TaintTrackingFrameworks
+private import semmle.code.java.dataflow.FlowSteps
+private import semmle.code.java.dataflow.internal.TaintTrackingFrameworks
 
 /**
  * Holds if taint can flow from `src` to `sink` in zero or more
@@ -72,52 +73,6 @@ predicate defaultAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) {
   any(AdditionalTaintStep a).step(src, sink)
 }
 
-/**
- * A method that returns tainted data when one of its inputs (an argument or the qualifier) are tainted.
- *
- * Extend this class to add additional taint steps through a method that should
- * apply to all taint configurations.
- */
-abstract class TaintPreservingMethod extends Method {
-  /**
-   * Holds if this method returns tainted data when `arg` tainted.
-   * `arg` is a parameter index, or is -1 to indicate the qualifier.
-   */
-  abstract predicate returnsTaint(int arg);
-}
-
-/**
- * A method that transfers taint from one of its inputs (an argument or the qualifier) to another.
- *
- * Extend this class to add additional taint steps through a method that should
- * apply to all taint configurations.
- */
-abstract class TaintTransferringMethod extends Method {
-  /**
-   * Holds if this method writes tainted data to `sink` when `src` is tainted.
-   * `src` and `sink` are parameter indices, or -1 to indicate the qualifier.
-   */
-  abstract predicate transfersTaint(int src, int sink);
-}
-
-private class StringTaintPreservingMethod extends TaintPreservingMethod {
-  StringTaintPreservingMethod() {
-    getDeclaringType() instanceof TypeString and
-    hasName(["concat", "copyValueOf", "endsWith", "format", "formatted", "getBytes", "indent",
-          "intern", "join", "repeat", "split", "strip", "stripIndent", "stripLeading",
-          "stripTrailing", "substring", "toCharArray", "toLowerCase", "toString", "toUpperCase",
-          "trim"])
-  }
-
-  override predicate returnsTaint(int arg) {
-    arg = -1
-    or
-    this.hasName(["concat", "copyValueOf"]) and arg = 0
-    or
-    this.hasName(["format", "formatted", "join"]) and arg = [0 .. getNumberOfParameters()]
-  }
-}
-
 /**
  * Holds if `node` should be a sanitizer in all global taint flow configurations
  * but not in local taint.
diff --git a/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll b/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll
index 288130401c4f..ff72695c4a00 100644
--- a/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll
+++ b/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll
@@ -1,6 +1,6 @@
 import java
 import Android
-private import semmle.code.java.dataflow.TaintTracking::TaintTracking as TT
+import semmle.code.java.dataflow.FlowSteps
 
 /**
  * The class `android.database.sqlite.SQLiteDatabase`.
@@ -228,7 +228,7 @@ private class ContentProviderUpdateMethod extends SQLiteRunner {
   override int sqlIndex() { result = 2 }
 }
 
-private class QueryBuilderBuildMethod extends TT::TaintPreservingMethod {
+private class QueryBuilderBuildMethod extends TaintPreservingMethod {
   QueryBuilderBuildMethod() {
     this.getDeclaringType().getASourceSupertype*() instanceof TypeSQLiteQueryBuilder and
     // buildQuery(String[] projectionIn, String selection, String groupBy, String having, String sortOrder, String limit)
@@ -255,7 +255,7 @@ private class QueryBuilderBuildMethod extends TT::TaintPreservingMethod {
   }
 }
 
-private class QueryBuilderAppendMethod extends TT::TaintTransferringMethod {
+private class QueryBuilderAppendMethod extends TaintTransferringMethod {
   QueryBuilderAppendMethod() {
     this.getDeclaringType().getASourceSupertype*() instanceof TypeSQLiteQueryBuilder and
     // setProjectionMap(Map<String, String> columnMap)
@@ -273,7 +273,7 @@ private class QueryBuilderAppendMethod extends TT::TaintTransferringMethod {
   }
 }
 
-private class UnsafeAppendUtilMethod extends TT::TaintPreservingMethod {
+private class UnsafeAppendUtilMethod extends TaintPreservingMethod {
   UnsafeAppendUtilMethod() {
     this.getDeclaringType() instanceof TypeDatabaseUtils and
     // String[] appendSelectionArgs(String[] originalValues, String[] newValues)
diff --git a/java/ql/src/semmle/code/java/frameworks/jackson/JacksonSerializability.qll b/java/ql/src/semmle/code/java/frameworks/jackson/JacksonSerializability.qll
index c00762244a73..456158d18b60 100644
--- a/java/ql/src/semmle/code/java/frameworks/jackson/JacksonSerializability.qll
+++ b/java/ql/src/semmle/code/java/frameworks/jackson/JacksonSerializability.qll
@@ -8,7 +8,7 @@ import semmle.code.java.Serializability
 import semmle.code.java.Reflection
 import semmle.code.java.dataflow.DataFlow
 import semmle.code.java.dataflow.DataFlow5
-private import semmle.code.java.dataflow.TaintTracking::TaintTracking as TT
+import semmle.code.java.dataflow.FlowSteps
 
 /**
  * A `@com.fasterxml.jackson.annotation.JsonIgnore` annoation.
@@ -28,7 +28,7 @@ abstract class JacksonSerializableType extends Type { }
  * A method used for serializing objects using Jackson. The final parameter is the object to be
  * serialized.
  */
-library class JacksonWriteValueMethod extends TT::TaintPreservingMethod, TT::TaintTransferringMethod {
+library class JacksonWriteValueMethod extends TaintPreservingMethod, TaintTransferringMethod {
   JacksonWriteValueMethod() {
     (
       getDeclaringType().hasQualifiedName("com.fasterxml.jackson.databind", "ObjectWriter") or

From 79209af9c0b2fc1299a9c5e5f83cf71274ce14ed Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Wed, 7 Oct 2020 12:58:11 +0100
Subject: [PATCH 06/16] Java: Refactor out flow steps for more frameworks.

---
 .../semmle/code/java/dataflow/FlowSteps.qll   | 31 +++++++++++++-
 .../internal/TaintTrackingFrameworks.qll      | 12 ------
 .../dataflow/internal/TaintTrackingUtil.qll   | 40 ++-----------------
 .../src/semmle/code/java/frameworks/Guice.qll |  7 ++++
 .../semmle/code/java/frameworks/Protobuf.qll  | 17 ++++++++
 .../code/java/frameworks/android/Intent.qll   |  5 ++-
 .../code/java/frameworks/android/SQLite.qll   | 14 +++++++
 7 files changed, 74 insertions(+), 52 deletions(-)
 delete mode 100644 java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingFrameworks.qll

diff --git a/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll b/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
index eb34ed0b4f06..3837643b1e84 100644
--- a/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
+++ b/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
@@ -2,10 +2,37 @@
  * Provides classes representing various flow steps for taint tracking.
  */
 
-import java
+private import java
+private import semmle.code.java.dataflow.DataFlow
 
 /**
- * A method that returns tainted data when one of its inputs (an argument or the qualifier) are tainted.
+ * A module importing the frameworks that implement additional flow steps,
+ * ensuring that they are visible to the taint tracking library.
+ */
+module Frameworks {
+  private import semmle.code.java.frameworks.jackson.JacksonSerializability
+  private import semmle.code.java.frameworks.android.Intent
+  private import semmle.code.java.frameworks.android.SQLite
+  private import semmle.code.java.frameworks.Guice
+  private import semmle.code.java.frameworks.Protobuf
+}
+
+/**
+ * A unit class for adding additional taint steps.
+ *
+ * Extend this class to add additional taint steps that should apply to all
+ * taint configurations.
+ */
+class AdditionalTaintStep extends Unit {
+  /**
+   * Holds if the step from `node1` to `node2` should be considered a taint
+   * step for all configurations.
+   */
+  abstract predicate step(DataFlow::Node node1, DataFlow::Node node2);
+}
+
+/**
+ * A method that returns tainted data when one of its inputs (an argument or the qualifier) is tainted.
  *
  * Extend this class to add additional taint steps through a method that should
  * apply to all taint configurations.
diff --git a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingFrameworks.qll b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingFrameworks.qll
deleted file mode 100644
index 26463c33d1ee..000000000000
--- a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingFrameworks.qll
+++ /dev/null
@@ -1,12 +0,0 @@
-/**
- * Provides taint tracking information for frameworks.
- * When a new framework is added that provides taint tracking steps, it should be imported here.
- */
-
-import semmle.code.java.frameworks.jackson.JacksonSerializability
-import semmle.code.java.frameworks.android.Intent
-import semmle.code.java.frameworks.android.SQLite
-import semmle.code.java.frameworks.Guice
-import semmle.code.java.frameworks.Protobuf
-import semmle.code.java.frameworks.spring.SpringController
-import semmle.code.java.frameworks.spring.SpringHttp
diff --git a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
index 4ad32af76e3c..05fe6075b601 100644
--- a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
+++ b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
@@ -7,8 +7,9 @@ private import semmle.code.java.security.SecurityTests
 private import semmle.code.java.security.Validation
 private import semmle.code.java.Maps
 private import semmle.code.java.dataflow.internal.ContainerFlow
-private import semmle.code.java.dataflow.FlowSteps
-private import semmle.code.java.dataflow.internal.TaintTrackingFrameworks
+private import semmle.code.java.frameworks.spring.SpringController
+private import semmle.code.java.frameworks.spring.SpringHttp
+import semmle.code.java.dataflow.FlowSteps
 
 /**
  * Holds if taint can flow from `src` to `sink` in zero or more
@@ -50,20 +51,6 @@ predicate localAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) {
   )
 }
 
-/**
- * A unit class for adding additional taint steps.
- *
- * Extend this class to add additional taint steps that should apply to all
- * taint configurations.
- */
-class AdditionalTaintStep extends Unit {
-  /**
-   * Holds if the step from `node1` to `node2` should be considered a taint
-   * step for all configurations.
-   */
-  abstract predicate step(DataFlow::Node node1, DataFlow::Node node2);
-}
-
 /**
  * Holds if the additional step from `src` to `sink` should be included in all
  * global taint flow configurations.
@@ -354,8 +341,6 @@ private predicate taintPreservingQualifierToMethod(Method m) {
   m.getDeclaringType().hasQualifiedName("javax.xml.transform.stream", "StreamSource") and
   m.hasName("getInputStream")
   or
-  m instanceof IntentGetExtraMethod
-  or
   m.getDeclaringType().hasQualifiedName("java.nio", "ByteBuffer") and
   m.hasName("get")
   or
@@ -365,10 +350,6 @@ private predicate taintPreservingQualifierToMethod(Method m) {
   m.getDeclaringType().hasQualifiedName("java.net", "URI") and
   m.hasName("toURL")
   or
-  m = any(GuiceProvider gp).getAnOverridingGetMethod()
-  or
-  m = any(ProtobufMessageLite p).getAGetterMethod()
-  or
   m instanceof GetterMethod and m.getDeclaringType() instanceof SpringUntrustedDataType
   or
   m.getDeclaringType() instanceof SpringHttpEntity and
@@ -525,25 +506,10 @@ private predicate taintPreservingArgumentToMethod(Method method, int arg) {
   method.hasName("sourceToInputSource") and
   arg = 0
   or
-  exists(ProtobufParser p | method = p.getAParseFromMethod()) and
-  arg = 0
-  or
-  exists(ProtobufMessageLite m | method = m.getAParseFromMethod()) and
-  arg = 0
-  or
   method.getDeclaringType().hasQualifiedName("java.io", "StringWriter") and
   method.hasName("append") and
   arg = 0
   or
-  (
-    method.getDeclaringType() instanceof AndroidContentProvider or
-    method.getDeclaringType() instanceof AndroidContentResolver
-  ) and
-  // Cursor query(Uri uri, String[] projection, String selection, String[] selectionArgs, String sortOrder, CancellationSignal cancellationSignal)
-  // Cursor query(Uri uri, String[] projection, String selection, String[] selectionArgs, String sortOrder)
-  method.hasName("query") and
-  arg = 0
-  or
   method.(TaintPreservingMethod).returnsTaint(arg)
 }
 
diff --git a/java/ql/src/semmle/code/java/frameworks/Guice.qll b/java/ql/src/semmle/code/java/frameworks/Guice.qll
index ad1735c8f618..c171ab2fb3aa 100644
--- a/java/ql/src/semmle/code/java/frameworks/Guice.qll
+++ b/java/ql/src/semmle/code/java/frameworks/Guice.qll
@@ -3,6 +3,7 @@
  */
 
 import java
+import semmle.code.java.dataflow.FlowSteps
 
 /**
  * A `@com.google.inject.servlet.RequestParameters` annotation.
@@ -33,3 +34,9 @@ class GuiceProvider extends Interface {
     exists(Method m | m.getSourceDeclaration() = getGetMethod() | result.overrides*(m))
   }
 }
+
+private class OverridingGetMethod extends TaintPreservingMethod {
+  OverridingGetMethod() { this = any(GuiceProvider gp).getAnOverridingGetMethod() }
+
+  override predicate returnsTaint(int arg) { arg = -1 }
+}
diff --git a/java/ql/src/semmle/code/java/frameworks/Protobuf.qll b/java/ql/src/semmle/code/java/frameworks/Protobuf.qll
index 8af99d7f6086..4fadaec26ad8 100644
--- a/java/ql/src/semmle/code/java/frameworks/Protobuf.qll
+++ b/java/ql/src/semmle/code/java/frameworks/Protobuf.qll
@@ -3,6 +3,7 @@
  */
 
 import java
+import semmle.code.java.dataflow.FlowSteps
 
 /**
  * The interface `com.google.protobuf.Parser`.
@@ -54,3 +55,19 @@ class ProtobufMessageLite extends Interface {
     )
   }
 }
+
+private class TaintPreservingGetterMethod extends TaintPreservingMethod {
+  TaintPreservingGetterMethod() { this = any(ProtobufMessageLite p).getAGetterMethod() }
+
+  override predicate returnsTaint(int arg) { arg = -1 }
+}
+
+private class TaintPreservingParseFromMethod extends TaintPreservingMethod {
+  TaintPreservingParseFromMethod() {
+    exists(ProtobufParser p | this = p.getAParseFromMethod())
+    or
+    exists(ProtobufMessageLite m | this = m.getAParseFromMethod())
+  }
+
+  override predicate returnsTaint(int arg) { arg = 0 }
+}
diff --git a/java/ql/src/semmle/code/java/frameworks/android/Intent.qll b/java/ql/src/semmle/code/java/frameworks/android/Intent.qll
index b0925b5716a7..73863df8034c 100644
--- a/java/ql/src/semmle/code/java/frameworks/android/Intent.qll
+++ b/java/ql/src/semmle/code/java/frameworks/android/Intent.qll
@@ -1,4 +1,5 @@
 import java
+import semmle.code.java.dataflow.FlowSteps
 
 class TypeIntent extends Class {
   TypeIntent() { hasQualifiedName("android.content", "Intent") }
@@ -33,9 +34,11 @@ class ContextStartActivityMethod extends Method {
   }
 }
 
-class IntentGetExtraMethod extends Method {
+class IntentGetExtraMethod extends Method, TaintPreservingMethod {
   IntentGetExtraMethod() {
     (getName().regexpMatch("get\\w+Extra") or hasName("getExtras")) and
     getDeclaringType() instanceof TypeIntent
   }
+
+  override predicate returnsTaint(int arg) { arg = -1 }
 }
diff --git a/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll b/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll
index ff72695c4a00..c01d61a34184 100644
--- a/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll
+++ b/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll
@@ -283,3 +283,17 @@ private class UnsafeAppendUtilMethod extends TaintPreservingMethod {
 
   override predicate returnsTaint(int arg) { arg = [0 .. getNumberOfParameters()] }
 }
+
+private class TaintPreservingQueryMethod extends TaintPreservingMethod {
+  TaintPreservingQueryMethod() {
+    (
+      this.getDeclaringType() instanceof AndroidContentProvider or
+      this.getDeclaringType() instanceof AndroidContentResolver
+    ) and
+    // Cursor query(Uri uri, String[] projection, String selection, String[] selectionArgs, String sortOrder, CancellationSignal cancellationSignal)
+    // Cursor query(Uri uri, String[] projection, String selection, String[] selectionArgs, String sortOrder)
+    this.hasName("query")
+  }
+
+  override predicate returnsTaint(int arg) { arg = 0 }
+}

From 91ce02aad46fba33ba74bc83266984299c51f665 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Thu, 8 Oct 2020 11:32:28 +0100
Subject: [PATCH 07/16] Java: Fix bug involving varadic parameters

---
 .../dataflow/internal/TaintTrackingUtil.qll   | 21 +++++++--
 .../dataflow/taint-format/A.java              |  1 +
 .../dataflow/taint-format/test.expected       | 43 ++++++++++---------
 3 files changed, 41 insertions(+), 24 deletions(-)

diff --git a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
index 05fe6075b601..20483fe77757 100644
--- a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
+++ b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
@@ -257,10 +257,23 @@ private predicate constructorStep(Expr tracked, ConstructorCall sink) {
   )
 }
 
+/**
+ * Converts an argument index to a formal parameter index.
+ * This is relevant for varadic methods.
+ */
+private int argToParam(MethodAccess ma, int arg) {
+  exists(ma.getArgument(arg)) and
+  exists(Method m | m = ma.getMethod() |
+    if m.isVarargs() and arg >= m.getNumberOfParameters()
+    then result = m.getNumberOfParameters() - 2
+    else result = arg
+  )
+}
+
 /** Access to a method that passes taint from qualifier to argument. */
 private predicate qualifierToArgumentStep(Expr tracked, Expr sink) {
   exists(MethodAccess ma, int arg |
-    taintPreservingQualifierToArgument(ma.getMethod(), arg) and
+    taintPreservingQualifierToArgument(ma.getMethod(), argToParam(ma, arg)) and
     tracked = ma.getQualifier() and
     sink = ma.getArgument(arg)
   )
@@ -394,7 +407,7 @@ private predicate unsafeEscape(MethodAccess ma) {
 private predicate argToMethodStep(Expr tracked, MethodAccess sink) {
   exists(Method m, int i |
     m = sink.getMethod() and
-    taintPreservingArgumentToMethod(m, i) and
+    taintPreservingArgumentToMethod(m, argToParam(sink, i)) and
     tracked = sink.getArgument(i)
   )
   or
@@ -519,7 +532,7 @@ private predicate taintPreservingArgumentToMethod(Method method, int arg) {
  */
 private predicate argToArgStep(Expr tracked, Expr sink) {
   exists(MethodAccess ma, Method method, int input, int output |
-    taintPreservingArgToArg(method, input, output) and
+    taintPreservingArgToArg(method, argToParam(ma, input), argToParam(ma, output)) and
     ma.getMethod() = method and
     ma.getArgument(input) = tracked and
     ma.getArgument(output) = sink
@@ -567,7 +580,7 @@ private predicate taintPreservingArgToArg(Method method, int input, int output)
  */
 private predicate argToQualifierStep(Expr tracked, Expr sink) {
   exists(Method m, int i, MethodAccess ma |
-    taintPreservingArgumentToQualifier(m, i) and
+    taintPreservingArgumentToQualifier(m, argToParam(ma, i)) and
     ma.getMethod() = m and
     tracked = ma.getArgument(i) and
     sink = ma.getQualifier()
diff --git a/java/ql/test/library-tests/dataflow/taint-format/A.java b/java/ql/test/library-tests/dataflow/taint-format/A.java
index 3612c23f950a..cb4a3395ae4a 100644
--- a/java/ql/test/library-tests/dataflow/taint-format/A.java
+++ b/java/ql/test/library-tests/dataflow/taint-format/A.java
@@ -14,6 +14,7 @@ public static void test1() {
         good.formatted("a", bad, "b", good);
         String.format("%s%s", bad, good);
         String.format("%s", good);
+        String.format("%s %s %s %s %s %s %s %s %s %s ", "a", "a", "a", "a", "a", "a", "a", "a", "a", bad);
     }
 
     public static void test2() {
diff --git a/java/ql/test/library-tests/dataflow/taint-format/test.expected b/java/ql/test/library-tests/dataflow/taint-format/test.expected
index 33abce9ebec6..22f8a10ad1ae 100644
--- a/java/ql/test/library-tests/dataflow/taint-format/test.expected
+++ b/java/ql/test/library-tests/dataflow/taint-format/test.expected
@@ -7,23 +7,26 @@
 | A.java:10:22:10:28 | taint(...) | A.java:15:9:15:40 | format(...) |
 | A.java:10:22:10:28 | taint(...) | A.java:15:9:15:40 | new ..[] { .. } |
 | A.java:10:22:10:28 | taint(...) | A.java:15:31:15:33 | bad |
-| A.java:20:22:20:28 | taint(...) | A.java:20:22:20:28 | taint(...) |
-| A.java:20:22:20:28 | taint(...) | A.java:24:9:24:9 | f [post update] |
-| A.java:20:22:20:28 | taint(...) | A.java:24:9:24:27 | format(...) |
-| A.java:20:22:20:28 | taint(...) | A.java:24:9:24:27 | new ..[] { .. } |
-| A.java:20:22:20:28 | taint(...) | A.java:24:24:24:26 | bad |
-| A.java:20:22:20:28 | taint(...) | A.java:25:9:25:9 | f |
-| A.java:20:22:20:28 | taint(...) | A.java:25:9:25:20 | toString(...) |
-| A.java:29:22:29:28 | taint(...) | A.java:29:22:29:28 | taint(...) |
-| A.java:29:22:29:28 | taint(...) | A.java:33:9:33:10 | sb |
-| A.java:29:22:29:28 | taint(...) | A.java:33:9:33:21 | toString(...) |
-| A.java:29:22:29:28 | taint(...) | A.java:34:9:34:9 | f [post update] |
-| A.java:29:22:29:28 | taint(...) | A.java:34:9:34:27 | format(...) |
-| A.java:29:22:29:28 | taint(...) | A.java:34:9:34:27 | new ..[] { .. } |
-| A.java:29:22:29:28 | taint(...) | A.java:34:24:34:26 | bad |
-| A.java:29:22:29:28 | taint(...) | A.java:35:9:35:10 | sb |
-| A.java:29:22:29:28 | taint(...) | A.java:35:9:35:21 | toString(...) |
-| A.java:39:22:39:28 | taint(...) | A.java:39:22:39:28 | taint(...) |
-| A.java:39:22:39:28 | taint(...) | A.java:42:18:42:20 | bad |
-| A.java:39:22:39:28 | taint(...) | A.java:43:9:43:46 | new ..[] { .. } |
-| A.java:39:22:39:28 | taint(...) | A.java:43:43:43:45 | bad |
+| A.java:10:22:10:28 | taint(...) | A.java:17:9:17:105 | format(...) |
+| A.java:10:22:10:28 | taint(...) | A.java:17:9:17:105 | new ..[] { .. } |
+| A.java:10:22:10:28 | taint(...) | A.java:17:102:17:104 | bad |
+| A.java:21:22:21:28 | taint(...) | A.java:21:22:21:28 | taint(...) |
+| A.java:21:22:21:28 | taint(...) | A.java:25:9:25:9 | f [post update] |
+| A.java:21:22:21:28 | taint(...) | A.java:25:9:25:27 | format(...) |
+| A.java:21:22:21:28 | taint(...) | A.java:25:9:25:27 | new ..[] { .. } |
+| A.java:21:22:21:28 | taint(...) | A.java:25:24:25:26 | bad |
+| A.java:21:22:21:28 | taint(...) | A.java:26:9:26:9 | f |
+| A.java:21:22:21:28 | taint(...) | A.java:26:9:26:20 | toString(...) |
+| A.java:30:22:30:28 | taint(...) | A.java:30:22:30:28 | taint(...) |
+| A.java:30:22:30:28 | taint(...) | A.java:34:9:34:10 | sb |
+| A.java:30:22:30:28 | taint(...) | A.java:34:9:34:21 | toString(...) |
+| A.java:30:22:30:28 | taint(...) | A.java:35:9:35:9 | f [post update] |
+| A.java:30:22:30:28 | taint(...) | A.java:35:9:35:27 | format(...) |
+| A.java:30:22:30:28 | taint(...) | A.java:35:9:35:27 | new ..[] { .. } |
+| A.java:30:22:30:28 | taint(...) | A.java:35:24:35:26 | bad |
+| A.java:30:22:30:28 | taint(...) | A.java:36:9:36:10 | sb |
+| A.java:30:22:30:28 | taint(...) | A.java:36:9:36:21 | toString(...) |
+| A.java:40:22:40:28 | taint(...) | A.java:40:22:40:28 | taint(...) |
+| A.java:40:22:40:28 | taint(...) | A.java:43:18:43:20 | bad |
+| A.java:40:22:40:28 | taint(...) | A.java:44:9:44:46 | new ..[] { .. } |
+| A.java:40:22:40:28 | taint(...) | A.java:44:43:44:45 | bad |

From a510f5886528864cc27b1cb052a80132ad0c4df7 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Thu, 8 Oct 2020 16:34:04 +0100
Subject: [PATCH 08/16] Java: Implement code review changes

---
 .../dataflow/internal/TaintTrackingUtil.qll   |  2 +-
 .../code/java/frameworks/android/SQLite.qll   | 21 ++++++++++---------
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
index 20483fe77757..94996f9c391d 100644
--- a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
+++ b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
@@ -265,7 +265,7 @@ private int argToParam(MethodAccess ma, int arg) {
   exists(ma.getArgument(arg)) and
   exists(Method m | m = ma.getMethod() |
     if m.isVarargs() and arg >= m.getNumberOfParameters()
-    then result = m.getNumberOfParameters() - 2
+    then result = m.getNumberOfParameters() - 1
     else result = arg
   )
 }
diff --git a/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll b/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll
index c01d61a34184..49e38ec97e1e 100644
--- a/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll
+++ b/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll
@@ -229,30 +229,31 @@ private class ContentProviderUpdateMethod extends SQLiteRunner {
 }
 
 private class QueryBuilderBuildMethod extends TaintPreservingMethod {
+  int argument;
+
   QueryBuilderBuildMethod() {
-    this.getDeclaringType().getASourceSupertype*() instanceof TypeSQLiteQueryBuilder and
+    this.getDeclaringType().getASourceSupertype*() instanceof Class and
     // buildQuery(String[] projectionIn, String selection, String groupBy, String having, String sortOrder, String limit)
     // buildQuery(String[] projectionIn, String selection, String[] selectionArgs, String groupBy, String having, String sortOrder, String limit)
     // buildUnionQuery(String[] subQueries, String sortOrder, String limit)
     // buildUnionSubQuery(String typeDiscriminatorColumn, String[] unionColumns, Set<String> columnsPresentInTable, int computedColumnsOffset, String typeDiscriminatorValue, String selection, String[] selectionArgs, String groupBy, String having)
     // buildUnionSubQuery(String typeDiscriminatorColumn, String[] unionColumns, Set<String> columnsPresentInTable, int computedColumnsOffset, String typeDiscriminatorValue, String selection, String groupBy, String having)
     // static buildQueryString(boolean distinct, String tables, String[] columns, String where, String groupBy, String having, String orderBy, String limit)
-    this.hasName(["buildQuery", "buildUnionQuery", "buildUnionSubQuery", "buildQueryString"])
-  }
-
-  override predicate returnsTaint(int arg) {
-    arg = -1
+    this.hasName(["buildQuery", "buildUnionQuery", "buildUnionSubQuery"]) and
+    argument = -1
     or
     hasName(["buildQuery", "buildUnionQuery"]) and
-    arg = [0 .. getNumberOfParameters()]
+    argument = [0 .. getNumberOfParameters()]
     or
     hasName("buildQueryString") and
-    arg = [1 .. getNumberOfParameters()]
+    argument = [1 .. getNumberOfParameters()]
     or
     hasName("buildUnionSubQuery") and
-    arg = [0 .. getNumberOfParameters()] and
-    arg != 3
+    argument = [0 .. getNumberOfParameters()] and
+    argument != 3
   }
+
+  override predicate returnsTaint(int arg) { argument = arg }
 }
 
 private class QueryBuilderAppendMethod extends TaintTransferringMethod {

From 5d487b97da8a7336bed12029a3d790eb03c41ef9 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Thu, 8 Oct 2020 17:02:49 +0100
Subject: [PATCH 09/16] Java: Merge `TaintPreservingMethod` with
 `TaintTransferringMethod`

---
 .../semmle/code/java/dataflow/FlowSteps.qll    | 18 +++++-------------
 .../dataflow/internal/TaintTrackingUtil.qll    |  8 ++++----
 .../code/java/frameworks/android/SQLite.qll    |  2 +-
 .../jackson/JacksonSerializability.qll         |  2 +-
 4 files changed, 11 insertions(+), 19 deletions(-)

diff --git a/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll b/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
index 3837643b1e84..42e730b41b46 100644
--- a/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
+++ b/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
@@ -32,31 +32,23 @@ class AdditionalTaintStep extends Unit {
 }
 
 /**
- * A method that returns tainted data when one of its inputs (an argument or the qualifier) is tainted.
+ * A method that preserves taint.
  *
- * Extend this class to add additional taint steps through a method that should
- * apply to all taint configurations.
+ * Extend this class and override at least one of `returnsTaint` or `transfersTaint`
+ * to add additional taint steps through a method that should apply to all taint configurations.
  */
 abstract class TaintPreservingMethod extends Method {
   /**
    * Holds if this method returns tainted data when `arg` tainted.
    * `arg` is a parameter index, or is -1 to indicate the qualifier.
    */
-  abstract predicate returnsTaint(int arg);
-}
+  predicate returnsTaint(int arg) { none() }
 
-/**
- * A method that transfers taint from one of its inputs (an argument or the qualifier) to another.
- *
- * Extend this class to add additional taint steps through a method that should
- * apply to all taint configurations.
- */
-abstract class TaintTransferringMethod extends Method {
   /**
    * Holds if this method writes tainted data to `sink` when `src` is tainted.
    * `src` and `sink` are parameter indices, or -1 to indicate the qualifier.
    */
-  abstract predicate transfersTaint(int src, int sink);
+  predicate transfersTaint(int src, int sink) { none() }
 }
 
 private class StringTaintPreservingMethod extends TaintPreservingMethod {
diff --git a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
index 94996f9c391d..673f0f45e2c0 100644
--- a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
+++ b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
@@ -296,7 +296,7 @@ private predicate taintPreservingQualifierToArgument(Method m, int arg) {
   m.hasName("read") and
   arg = 0
   or
-  m.(TaintTransferringMethod).transfersTaint(-1, arg)
+  m.(TaintPreservingMethod).transfersTaint(-1, arg)
 }
 
 /** Access to a method that passes taint from the qualifier. */
@@ -571,7 +571,7 @@ private predicate taintPreservingArgToArg(Method method, int input, int output)
   input = 0 and
   output = 2
   or
-  method.(TaintTransferringMethod).transfersTaint(input, output)
+  method.(TaintPreservingMethod).transfersTaint(input, output)
 }
 
 /**
@@ -610,7 +610,7 @@ private predicate taintPreservingArgumentToQualifier(Method method, int arg) {
     append.getDeclaringType().hasQualifiedName("java.io", "StringWriter")
   )
   or
-  method.(TaintTransferringMethod).transfersTaint(arg, -1)
+  method.(TaintPreservingMethod).transfersTaint(arg, -1)
 }
 
 /** A comparison or equality test with a constant. */
@@ -734,7 +734,7 @@ private class TypeFormatter extends Class {
   TypeFormatter() { this.hasQualifiedName("java.util", "Formatter") }
 }
 
-private class FormatterMethod extends TaintPreservingMethod, TaintTransferringMethod {
+private class FormatterMethod extends TaintPreservingMethod {
   FormatterMethod() {
     getDeclaringType() instanceof TypeFormatter and
     hasName(["format", "out", "toString"])
diff --git a/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll b/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll
index 49e38ec97e1e..325db8ac6795 100644
--- a/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll
+++ b/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll
@@ -256,7 +256,7 @@ private class QueryBuilderBuildMethod extends TaintPreservingMethod {
   override predicate returnsTaint(int arg) { argument = arg }
 }
 
-private class QueryBuilderAppendMethod extends TaintTransferringMethod {
+private class QueryBuilderAppendMethod extends TaintPreservingMethod {
   QueryBuilderAppendMethod() {
     this.getDeclaringType().getASourceSupertype*() instanceof TypeSQLiteQueryBuilder and
     // setProjectionMap(Map<String, String> columnMap)
diff --git a/java/ql/src/semmle/code/java/frameworks/jackson/JacksonSerializability.qll b/java/ql/src/semmle/code/java/frameworks/jackson/JacksonSerializability.qll
index 456158d18b60..c57c35d8bd86 100644
--- a/java/ql/src/semmle/code/java/frameworks/jackson/JacksonSerializability.qll
+++ b/java/ql/src/semmle/code/java/frameworks/jackson/JacksonSerializability.qll
@@ -28,7 +28,7 @@ abstract class JacksonSerializableType extends Type { }
  * A method used for serializing objects using Jackson. The final parameter is the object to be
  * serialized.
  */
-library class JacksonWriteValueMethod extends TaintPreservingMethod, TaintTransferringMethod {
+library class JacksonWriteValueMethod extends TaintPreservingMethod {
   JacksonWriteValueMethod() {
     (
       getDeclaringType().hasQualifiedName("com.fasterxml.jackson.databind", "ObjectWriter") or

From ca9038350cff194e0536f5c97889c748fc48c16d Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Fri, 9 Oct 2020 11:30:30 +0100
Subject: [PATCH 10/16] Java: Add `this.` and fix mistake

---
 java/ql/src/semmle/code/java/dataflow/FlowSteps.qll   | 11 ++++++-----
 .../semmle/code/java/frameworks/android/SQLite.qll    |  2 +-
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll b/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
index 42e730b41b46..d6fb5518a68e 100644
--- a/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
+++ b/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
@@ -53,11 +53,12 @@ abstract class TaintPreservingMethod extends Method {
 
 private class StringTaintPreservingMethod extends TaintPreservingMethod {
   StringTaintPreservingMethod() {
-    getDeclaringType() instanceof TypeString and
-    hasName(["concat", "copyValueOf", "endsWith", "format", "formatted", "getBytes", "indent",
-          "intern", "join", "repeat", "split", "strip", "stripIndent", "stripLeading",
-          "stripTrailing", "substring", "toCharArray", "toLowerCase", "toString", "toUpperCase",
-          "trim"])
+    this.getDeclaringType() instanceof TypeString and
+    this
+        .hasName(["concat", "copyValueOf", "endsWith", "format", "formatted", "getBytes", "indent",
+              "intern", "join", "repeat", "split", "strip", "stripIndent", "stripLeading",
+              "stripTrailing", "substring", "toCharArray", "toLowerCase", "toString", "toUpperCase",
+              "trim"])
   }
 
   override predicate returnsTaint(int arg) {
diff --git a/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll b/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll
index 325db8ac6795..18a22e8ad5f2 100644
--- a/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll
+++ b/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll
@@ -232,7 +232,7 @@ private class QueryBuilderBuildMethod extends TaintPreservingMethod {
   int argument;
 
   QueryBuilderBuildMethod() {
-    this.getDeclaringType().getASourceSupertype*() instanceof Class and
+    this.getDeclaringType().getASourceSupertype*() instanceof TypeSQLiteQueryBuilder and
     // buildQuery(String[] projectionIn, String selection, String groupBy, String having, String sortOrder, String limit)
     // buildQuery(String[] projectionIn, String selection, String[] selectionArgs, String groupBy, String having, String sortOrder, String limit)
     // buildUnionQuery(String[] subQueries, String sortOrder, String limit)

From 4a8b7f64e860acc84301fdef0c937b07a47e17e6 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Fri, 9 Oct 2020 12:20:09 +0100
Subject: [PATCH 11/16] Java: Rename returnsTaint to returnsTaintFrom

---
 java/ql/src/semmle/code/java/dataflow/FlowSteps.qll       | 6 +++---
 .../code/java/dataflow/internal/TaintTrackingUtil.qll     | 8 ++++----
 java/ql/src/semmle/code/java/frameworks/Guice.qll         | 2 +-
 java/ql/src/semmle/code/java/frameworks/Protobuf.qll      | 4 ++--
 .../ql/src/semmle/code/java/frameworks/android/Intent.qll | 2 +-
 .../ql/src/semmle/code/java/frameworks/android/SQLite.qll | 6 +++---
 .../java/frameworks/jackson/JacksonSerializability.qll    | 2 +-
 7 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll b/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
index d6fb5518a68e..99dbfda4171b 100644
--- a/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
+++ b/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
@@ -34,7 +34,7 @@ class AdditionalTaintStep extends Unit {
 /**
  * A method that preserves taint.
  *
- * Extend this class and override at least one of `returnsTaint` or `transfersTaint`
+ * Extend this class and override at least one of `returnsTaintFrom` or `transfersTaint`
  * to add additional taint steps through a method that should apply to all taint configurations.
  */
 abstract class TaintPreservingMethod extends Method {
@@ -42,7 +42,7 @@ abstract class TaintPreservingMethod extends Method {
    * Holds if this method returns tainted data when `arg` tainted.
    * `arg` is a parameter index, or is -1 to indicate the qualifier.
    */
-  predicate returnsTaint(int arg) { none() }
+  predicate returnsTaintFrom(int arg) { none() }
 
   /**
    * Holds if this method writes tainted data to `sink` when `src` is tainted.
@@ -61,7 +61,7 @@ private class StringTaintPreservingMethod extends TaintPreservingMethod {
               "trim"])
   }
 
-  override predicate returnsTaint(int arg) {
+  override predicate returnsTaintFrom(int arg) {
     arg = -1
     or
     this.hasName(["concat", "copyValueOf"]) and arg = 0
diff --git a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
index 673f0f45e2c0..8c76d621c216 100644
--- a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
+++ b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
@@ -378,7 +378,7 @@ private predicate taintPreservingQualifierToMethod(Method m) {
     )
   )
   or
-  m.(TaintPreservingMethod).returnsTaint(-1)
+  m.(TaintPreservingMethod).returnsTaintFrom(-1)
 }
 
 private class StringReplaceMethod extends TaintPreservingMethod {
@@ -391,7 +391,7 @@ private class StringReplaceMethod extends TaintPreservingMethod {
     )
   }
 
-  override predicate returnsTaint(int arg) { arg = 1 }
+  override predicate returnsTaintFrom(int arg) { arg = 1 }
 }
 
 private predicate unsafeEscape(MethodAccess ma) {
@@ -523,7 +523,7 @@ private predicate taintPreservingArgumentToMethod(Method method, int arg) {
   method.hasName("append") and
   arg = 0
   or
-  method.(TaintPreservingMethod).returnsTaint(arg)
+  method.(TaintPreservingMethod).returnsTaintFrom(arg)
 }
 
 /**
@@ -740,7 +740,7 @@ private class FormatterMethod extends TaintPreservingMethod {
     hasName(["format", "out", "toString"])
   }
 
-  override predicate returnsTaint(int arg) { arg = [-1 .. getNumberOfParameters()] }
+  override predicate returnsTaintFrom(int arg) { arg = [-1 .. getNumberOfParameters()] }
 
   override predicate transfersTaint(int src, int sink) {
     sink = -1 and
diff --git a/java/ql/src/semmle/code/java/frameworks/Guice.qll b/java/ql/src/semmle/code/java/frameworks/Guice.qll
index c171ab2fb3aa..e79fb494b4de 100644
--- a/java/ql/src/semmle/code/java/frameworks/Guice.qll
+++ b/java/ql/src/semmle/code/java/frameworks/Guice.qll
@@ -38,5 +38,5 @@ class GuiceProvider extends Interface {
 private class OverridingGetMethod extends TaintPreservingMethod {
   OverridingGetMethod() { this = any(GuiceProvider gp).getAnOverridingGetMethod() }
 
-  override predicate returnsTaint(int arg) { arg = -1 }
+  override predicate returnsTaintFrom(int arg) { arg = -1 }
 }
diff --git a/java/ql/src/semmle/code/java/frameworks/Protobuf.qll b/java/ql/src/semmle/code/java/frameworks/Protobuf.qll
index 4fadaec26ad8..bb7d0e815c14 100644
--- a/java/ql/src/semmle/code/java/frameworks/Protobuf.qll
+++ b/java/ql/src/semmle/code/java/frameworks/Protobuf.qll
@@ -59,7 +59,7 @@ class ProtobufMessageLite extends Interface {
 private class TaintPreservingGetterMethod extends TaintPreservingMethod {
   TaintPreservingGetterMethod() { this = any(ProtobufMessageLite p).getAGetterMethod() }
 
-  override predicate returnsTaint(int arg) { arg = -1 }
+  override predicate returnsTaintFrom(int arg) { arg = -1 }
 }
 
 private class TaintPreservingParseFromMethod extends TaintPreservingMethod {
@@ -69,5 +69,5 @@ private class TaintPreservingParseFromMethod extends TaintPreservingMethod {
     exists(ProtobufMessageLite m | this = m.getAParseFromMethod())
   }
 
-  override predicate returnsTaint(int arg) { arg = 0 }
+  override predicate returnsTaintFrom(int arg) { arg = 0 }
 }
diff --git a/java/ql/src/semmle/code/java/frameworks/android/Intent.qll b/java/ql/src/semmle/code/java/frameworks/android/Intent.qll
index 73863df8034c..6bd184a0bb30 100644
--- a/java/ql/src/semmle/code/java/frameworks/android/Intent.qll
+++ b/java/ql/src/semmle/code/java/frameworks/android/Intent.qll
@@ -40,5 +40,5 @@ class IntentGetExtraMethod extends Method, TaintPreservingMethod {
     getDeclaringType() instanceof TypeIntent
   }
 
-  override predicate returnsTaint(int arg) { arg = -1 }
+  override predicate returnsTaintFrom(int arg) { arg = -1 }
 }
diff --git a/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll b/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll
index 18a22e8ad5f2..79f37628114f 100644
--- a/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll
+++ b/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll
@@ -253,7 +253,7 @@ private class QueryBuilderBuildMethod extends TaintPreservingMethod {
     argument != 3
   }
 
-  override predicate returnsTaint(int arg) { argument = arg }
+  override predicate returnsTaintFrom(int arg) { argument = arg }
 }
 
 private class QueryBuilderAppendMethod extends TaintPreservingMethod {
@@ -282,7 +282,7 @@ private class UnsafeAppendUtilMethod extends TaintPreservingMethod {
     this.hasName(["appendSelectionArgs", "concatenateWhere"])
   }
 
-  override predicate returnsTaint(int arg) { arg = [0 .. getNumberOfParameters()] }
+  override predicate returnsTaintFrom(int arg) { arg = [0 .. getNumberOfParameters()] }
 }
 
 private class TaintPreservingQueryMethod extends TaintPreservingMethod {
@@ -296,5 +296,5 @@ private class TaintPreservingQueryMethod extends TaintPreservingMethod {
     this.hasName("query")
   }
 
-  override predicate returnsTaint(int arg) { arg = 0 }
+  override predicate returnsTaintFrom(int arg) { arg = 0 }
 }
diff --git a/java/ql/src/semmle/code/java/frameworks/jackson/JacksonSerializability.qll b/java/ql/src/semmle/code/java/frameworks/jackson/JacksonSerializability.qll
index c57c35d8bd86..ba51ff4c6d76 100644
--- a/java/ql/src/semmle/code/java/frameworks/jackson/JacksonSerializability.qll
+++ b/java/ql/src/semmle/code/java/frameworks/jackson/JacksonSerializability.qll
@@ -38,7 +38,7 @@ library class JacksonWriteValueMethod extends TaintPreservingMethod {
     getParameter(getNumberOfParameters() - 1).getType() instanceof TypeObject
   }
 
-  override predicate returnsTaint(int arg) {
+  override predicate returnsTaintFrom(int arg) {
     getNumberOfParameters() = 1 and
     arg = 0
   }

From 7e2c49fadd80e50aa4b5d9bf84e5ce6a99739639 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Mon, 12 Oct 2020 14:05:50 +0100
Subject: [PATCH 12/16] Java: Fix a couple of flow step issues

Co-authored-by: Anders Schack-Mulligen <aschackmull@users.noreply.github.com>
---
 .../semmle/code/java/dataflow/FlowSteps.qll   |  2 +-
 .../code/java/frameworks/android/SQLite.qll   | 35 +++++++++----------
 2 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll b/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
index 99dbfda4171b..1468363c33c5 100644
--- a/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
+++ b/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
@@ -62,7 +62,7 @@ private class StringTaintPreservingMethod extends TaintPreservingMethod {
   }
 
   override predicate returnsTaintFrom(int arg) {
-    arg = -1
+    arg = -1 and not this.isStatic()
     or
     this.hasName(["concat", "copyValueOf"]) and arg = 0
     or
diff --git a/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll b/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll
index 79f37628114f..cd2232a8683d 100644
--- a/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll
+++ b/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll
@@ -233,24 +233,23 @@ private class QueryBuilderBuildMethod extends TaintPreservingMethod {
 
   QueryBuilderBuildMethod() {
     this.getDeclaringType().getASourceSupertype*() instanceof TypeSQLiteQueryBuilder and
-    // buildQuery(String[] projectionIn, String selection, String groupBy, String having, String sortOrder, String limit)
-    // buildQuery(String[] projectionIn, String selection, String[] selectionArgs, String groupBy, String having, String sortOrder, String limit)
-    // buildUnionQuery(String[] subQueries, String sortOrder, String limit)
-    // buildUnionSubQuery(String typeDiscriminatorColumn, String[] unionColumns, Set<String> columnsPresentInTable, int computedColumnsOffset, String typeDiscriminatorValue, String selection, String[] selectionArgs, String groupBy, String having)
-    // buildUnionSubQuery(String typeDiscriminatorColumn, String[] unionColumns, Set<String> columnsPresentInTable, int computedColumnsOffset, String typeDiscriminatorValue, String selection, String groupBy, String having)
-    // static buildQueryString(boolean distinct, String tables, String[] columns, String where, String groupBy, String having, String orderBy, String limit)
-    this.hasName(["buildQuery", "buildUnionQuery", "buildUnionSubQuery"]) and
-    argument = -1
-    or
-    hasName(["buildQuery", "buildUnionQuery"]) and
-    argument = [0 .. getNumberOfParameters()]
-    or
-    hasName("buildQueryString") and
-    argument = [1 .. getNumberOfParameters()]
-    or
-    hasName("buildUnionSubQuery") and
-    argument = [0 .. getNumberOfParameters()] and
-    argument != 3
+    (
+      // buildQuery(String[] projectionIn, String selection, String groupBy, String having, String sortOrder, String limit)
+      // buildQuery(String[] projectionIn, String selection, String[] selectionArgs, String groupBy, String having, String sortOrder, String limit)
+      // buildUnionQuery(String[] subQueries, String sortOrder, String limit)
+      this.hasName(["buildQuery", "buildUnionQuery"]) and
+      argument = [-1 .. getNumberOfParameters()]
+      or
+      // buildUnionSubQuery(String typeDiscriminatorColumn, String[] unionColumns, Set<String> columnsPresentInTable, int computedColumnsOffset, String typeDiscriminatorValue, String selection, String[] selectionArgs, String groupBy, String having)
+      // buildUnionSubQuery(String typeDiscriminatorColumn, String[] unionColumns, Set<String> columnsPresentInTable, int computedColumnsOffset, String typeDiscriminatorValue, String selection, String groupBy, String having)
+      this.hasName("buildUnionSubQuery") and
+      argument = [-1 .. getNumberOfParameters()] and
+      argument != 3
+      or
+      // static buildQueryString(boolean distinct, String tables, String[] columns, String where, String groupBy, String having, String orderBy, String limit)
+      hasName("buildQueryString") and
+      argument = [1 .. getNumberOfParameters()]
+    )
   }
 
   override predicate returnsTaintFrom(int arg) { argument = arg }

From eafde05a55be693e376fe4831043809ae61791fa Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Mon, 12 Oct 2020 14:43:21 +0100
Subject: [PATCH 13/16] Java: Expand flow step refactoring to Callables Also
 add some missing flow steps for StringBuilder

---
 .../semmle/code/java/dataflow/FlowSteps.qll   | 16 +++---
 .../dataflow/internal/TaintTrackingUtil.qll   | 55 ++++++++++++-------
 .../src/semmle/code/java/frameworks/Guice.qll |  2 +-
 .../semmle/code/java/frameworks/Protobuf.qll  |  4 +-
 .../code/java/frameworks/android/Intent.qll   |  2 +-
 .../code/java/frameworks/android/SQLite.qll   |  8 +--
 .../jackson/JacksonSerializability.qll        |  2 +-
 .../dataflow/taint-format/A.java              | 11 ++--
 .../dataflow/taint-format/test.expected       | 10 +++-
 9 files changed, 65 insertions(+), 45 deletions(-)

diff --git a/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll b/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
index 1468363c33c5..802e287bf435 100644
--- a/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
+++ b/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
@@ -32,39 +32,39 @@ class AdditionalTaintStep extends Unit {
 }
 
 /**
- * A method that preserves taint.
+ * A method or constructor that preserves taint.
  *
  * Extend this class and override at least one of `returnsTaintFrom` or `transfersTaint`
  * to add additional taint steps through a method that should apply to all taint configurations.
  */
-abstract class TaintPreservingMethod extends Method {
+abstract class TaintPreservingCallable extends Callable {
   /**
-   * Holds if this method returns tainted data when `arg` tainted.
+   * Holds if this callable returns tainted data when `arg` tainted.
    * `arg` is a parameter index, or is -1 to indicate the qualifier.
    */
   predicate returnsTaintFrom(int arg) { none() }
 
   /**
-   * Holds if this method writes tainted data to `sink` when `src` is tainted.
+   * Holds if this callable writes tainted data to `sink` when `src` is tainted.
    * `src` and `sink` are parameter indices, or -1 to indicate the qualifier.
    */
   predicate transfersTaint(int src, int sink) { none() }
 }
 
-private class StringTaintPreservingMethod extends TaintPreservingMethod {
-  StringTaintPreservingMethod() {
+private class StringTaintPreservingCallable extends TaintPreservingCallable {
+  StringTaintPreservingCallable() {
     this.getDeclaringType() instanceof TypeString and
     this
         .hasName(["concat", "copyValueOf", "endsWith", "format", "formatted", "getBytes", "indent",
               "intern", "join", "repeat", "split", "strip", "stripIndent", "stripLeading",
               "stripTrailing", "substring", "toCharArray", "toLowerCase", "toString", "toUpperCase",
-              "trim"])
+              "trim", "String"])
   }
 
   override predicate returnsTaintFrom(int arg) {
     arg = -1 and not this.isStatic()
     or
-    this.hasName(["concat", "copyValueOf"]) and arg = 0
+    this.hasName(["concat", "copyValueOf", "String"]) and arg = 0
     or
     this.hasName(["format", "formatted", "join"]) and arg = [0 .. getNumberOfParameters()]
   }
diff --git a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
index 8c76d621c216..8643b8e3c19c 100644
--- a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
+++ b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
@@ -162,9 +162,6 @@ private predicate inputStreamWrapper(Constructor c, int argi) {
 private predicate constructorStep(Expr tracked, ConstructorCall sink) {
   exists(int argi | sink.getArgument(argi) = tracked |
     exists(string s | sink.getConstructedType().getQualifiedName() = s |
-      // String constructor does nothing to data
-      s = "java.lang.String" and argi = 0
-      or
       // some readers preserve the content of streams
       s = "java.io.InputStreamReader" and argi = 0
       or
@@ -254,6 +251,8 @@ private predicate constructorStep(Expr tracked, ConstructorCall sink) {
       argi = 0 and
       tracked.getType() instanceof TypeString
     )
+    or
+    sink.getConstructor().(TaintPreservingCallable).returnsTaintFrom(argToParam(sink, argi))
   )
 }
 
@@ -261,11 +260,11 @@ private predicate constructorStep(Expr tracked, ConstructorCall sink) {
  * Converts an argument index to a formal parameter index.
  * This is relevant for varadic methods.
  */
-private int argToParam(MethodAccess ma, int arg) {
-  exists(ma.getArgument(arg)) and
-  exists(Method m | m = ma.getMethod() |
-    if m.isVarargs() and arg >= m.getNumberOfParameters()
-    then result = m.getNumberOfParameters() - 1
+private int argToParam(Call call, int arg) {
+  exists(call.getArgument(arg)) and
+  exists(Callable c | c = call.getCallee() |
+    if c.isVarargs() and arg >= c.getNumberOfParameters()
+    then result = c.getNumberOfParameters() - 1
     else result = arg
   )
 }
@@ -296,7 +295,7 @@ private predicate taintPreservingQualifierToArgument(Method m, int arg) {
   m.hasName("read") and
   arg = 0
   or
-  m.(TaintPreservingMethod).transfersTaint(-1, arg)
+  m.(TaintPreservingCallable).transfersTaint(-1, arg)
 }
 
 /** Access to a method that passes taint from the qualifier. */
@@ -378,10 +377,10 @@ private predicate taintPreservingQualifierToMethod(Method m) {
     )
   )
   or
-  m.(TaintPreservingMethod).returnsTaintFrom(-1)
+  m.(TaintPreservingCallable).returnsTaintFrom(-1)
 }
 
-private class StringReplaceMethod extends TaintPreservingMethod {
+private class StringReplaceMethod extends TaintPreservingCallable {
   StringReplaceMethod() {
     getDeclaringType() instanceof TypeString and
     (
@@ -523,7 +522,7 @@ private predicate taintPreservingArgumentToMethod(Method method, int arg) {
   method.hasName("append") and
   arg = 0
   or
-  method.(TaintPreservingMethod).returnsTaintFrom(arg)
+  method.(TaintPreservingCallable).returnsTaintFrom(arg)
 }
 
 /**
@@ -571,7 +570,7 @@ private predicate taintPreservingArgToArg(Method method, int input, int output)
   input = 0 and
   output = 2
   or
-  method.(TaintPreservingMethod).transfersTaint(input, output)
+  method.(TaintPreservingCallable).transfersTaint(input, output)
 }
 
 /**
@@ -607,10 +606,14 @@ private predicate taintPreservingArgumentToQualifier(Method method, int arg) {
     method.overrides*(append) and
     append.hasName("append") and
     arg = 0 and
-    append.getDeclaringType().hasQualifiedName("java.io", "StringWriter")
+    (
+      append.getDeclaringType().hasQualifiedName("java.lang", "StringBuilder") or
+      append.getDeclaringType().hasQualifiedName("java.lang", "StringBuffer") or
+      append.getDeclaringType().hasQualifiedName("java.io", "StringWriter")
+    )
   )
   or
-  method.(TaintPreservingMethod).transfersTaint(arg, -1)
+  method.(TaintPreservingCallable).transfersTaint(arg, -1)
 }
 
 /** A comparison or equality test with a constant. */
@@ -734,15 +737,27 @@ private class TypeFormatter extends Class {
   TypeFormatter() { this.hasQualifiedName("java.util", "Formatter") }
 }
 
-private class FormatterMethod extends TaintPreservingMethod {
-  FormatterMethod() {
-    getDeclaringType() instanceof TypeFormatter and
-    hasName(["format", "out", "toString"])
+private class FormatterCallable extends TaintPreservingCallable {
+  FormatterCallable() {
+    this.getDeclaringType() instanceof TypeFormatter and
+    (
+      this.hasName(["format", "out", "toString"])
+      or
+      this
+          .(Constructor)
+          .getParameterType(0)
+          .(RefType)
+          .getASourceSupertype*()
+          .hasQualifiedName("java.lang", "Appendable")
+    )
   }
 
-  override predicate returnsTaintFrom(int arg) { arg = [-1 .. getNumberOfParameters()] }
+  override predicate returnsTaintFrom(int arg) {
+    if this instanceof Constructor then arg = 0 else arg = [-1 .. getNumberOfParameters()]
+  }
 
   override predicate transfersTaint(int src, int sink) {
+    this.hasName("format") and
     sink = -1 and
     src = [0 .. getNumberOfParameters()]
   }
diff --git a/java/ql/src/semmle/code/java/frameworks/Guice.qll b/java/ql/src/semmle/code/java/frameworks/Guice.qll
index e79fb494b4de..f7154df4bd20 100644
--- a/java/ql/src/semmle/code/java/frameworks/Guice.qll
+++ b/java/ql/src/semmle/code/java/frameworks/Guice.qll
@@ -35,7 +35,7 @@ class GuiceProvider extends Interface {
   }
 }
 
-private class OverridingGetMethod extends TaintPreservingMethod {
+private class OverridingGetMethod extends TaintPreservingCallable {
   OverridingGetMethod() { this = any(GuiceProvider gp).getAnOverridingGetMethod() }
 
   override predicate returnsTaintFrom(int arg) { arg = -1 }
diff --git a/java/ql/src/semmle/code/java/frameworks/Protobuf.qll b/java/ql/src/semmle/code/java/frameworks/Protobuf.qll
index bb7d0e815c14..7382294f6f9f 100644
--- a/java/ql/src/semmle/code/java/frameworks/Protobuf.qll
+++ b/java/ql/src/semmle/code/java/frameworks/Protobuf.qll
@@ -56,13 +56,13 @@ class ProtobufMessageLite extends Interface {
   }
 }
 
-private class TaintPreservingGetterMethod extends TaintPreservingMethod {
+private class TaintPreservingGetterMethod extends TaintPreservingCallable {
   TaintPreservingGetterMethod() { this = any(ProtobufMessageLite p).getAGetterMethod() }
 
   override predicate returnsTaintFrom(int arg) { arg = -1 }
 }
 
-private class TaintPreservingParseFromMethod extends TaintPreservingMethod {
+private class TaintPreservingParseFromMethod extends TaintPreservingCallable {
   TaintPreservingParseFromMethod() {
     exists(ProtobufParser p | this = p.getAParseFromMethod())
     or
diff --git a/java/ql/src/semmle/code/java/frameworks/android/Intent.qll b/java/ql/src/semmle/code/java/frameworks/android/Intent.qll
index 6bd184a0bb30..c4894a5976c9 100644
--- a/java/ql/src/semmle/code/java/frameworks/android/Intent.qll
+++ b/java/ql/src/semmle/code/java/frameworks/android/Intent.qll
@@ -34,7 +34,7 @@ class ContextStartActivityMethod extends Method {
   }
 }
 
-class IntentGetExtraMethod extends Method, TaintPreservingMethod {
+class IntentGetExtraMethod extends Method, TaintPreservingCallable {
   IntentGetExtraMethod() {
     (getName().regexpMatch("get\\w+Extra") or hasName("getExtras")) and
     getDeclaringType() instanceof TypeIntent
diff --git a/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll b/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll
index cd2232a8683d..7a8defc84d18 100644
--- a/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll
+++ b/java/ql/src/semmle/code/java/frameworks/android/SQLite.qll
@@ -228,7 +228,7 @@ private class ContentProviderUpdateMethod extends SQLiteRunner {
   override int sqlIndex() { result = 2 }
 }
 
-private class QueryBuilderBuildMethod extends TaintPreservingMethod {
+private class QueryBuilderBuildMethod extends TaintPreservingCallable {
   int argument;
 
   QueryBuilderBuildMethod() {
@@ -255,7 +255,7 @@ private class QueryBuilderBuildMethod extends TaintPreservingMethod {
   override predicate returnsTaintFrom(int arg) { argument = arg }
 }
 
-private class QueryBuilderAppendMethod extends TaintPreservingMethod {
+private class QueryBuilderAppendMethod extends TaintPreservingCallable {
   QueryBuilderAppendMethod() {
     this.getDeclaringType().getASourceSupertype*() instanceof TypeSQLiteQueryBuilder and
     // setProjectionMap(Map<String, String> columnMap)
@@ -273,7 +273,7 @@ private class QueryBuilderAppendMethod extends TaintPreservingMethod {
   }
 }
 
-private class UnsafeAppendUtilMethod extends TaintPreservingMethod {
+private class UnsafeAppendUtilMethod extends TaintPreservingCallable {
   UnsafeAppendUtilMethod() {
     this.getDeclaringType() instanceof TypeDatabaseUtils and
     // String[] appendSelectionArgs(String[] originalValues, String[] newValues)
@@ -284,7 +284,7 @@ private class UnsafeAppendUtilMethod extends TaintPreservingMethod {
   override predicate returnsTaintFrom(int arg) { arg = [0 .. getNumberOfParameters()] }
 }
 
-private class TaintPreservingQueryMethod extends TaintPreservingMethod {
+private class TaintPreservingQueryMethod extends TaintPreservingCallable {
   TaintPreservingQueryMethod() {
     (
       this.getDeclaringType() instanceof AndroidContentProvider or
diff --git a/java/ql/src/semmle/code/java/frameworks/jackson/JacksonSerializability.qll b/java/ql/src/semmle/code/java/frameworks/jackson/JacksonSerializability.qll
index ba51ff4c6d76..3356e31d965b 100644
--- a/java/ql/src/semmle/code/java/frameworks/jackson/JacksonSerializability.qll
+++ b/java/ql/src/semmle/code/java/frameworks/jackson/JacksonSerializability.qll
@@ -28,7 +28,7 @@ abstract class JacksonSerializableType extends Type { }
  * A method used for serializing objects using Jackson. The final parameter is the object to be
  * serialized.
  */
-library class JacksonWriteValueMethod extends TaintPreservingMethod {
+library class JacksonWriteValueMethod extends Method, TaintPreservingCallable {
   JacksonWriteValueMethod() {
     (
       getDeclaringType().hasQualifiedName("com.fasterxml.jackson.databind", "ObjectWriter") or
diff --git a/java/ql/test/library-tests/dataflow/taint-format/A.java b/java/ql/test/library-tests/dataflow/taint-format/A.java
index cb4a3395ae4a..57825e24e43b 100644
--- a/java/ql/test/library-tests/dataflow/taint-format/A.java
+++ b/java/ql/test/library-tests/dataflow/taint-format/A.java
@@ -1,7 +1,7 @@
 import java.util.Formatter;
 import java.lang.StringBuilder;
-import java.lang.System;
-import java.io.Console;
+
+
 
 class A {
     public static String taint() { return "tainted"; }
@@ -38,9 +38,10 @@ public static void test3() {
 
     public static void test4() {
         String bad = taint();
-        Console c = System.console();
+        StringBuilder sb = new StringBuilder();
+
+        sb.append(bad);
 
-        c.format(bad);
-        c.readLine("Enter something: %s", bad);
+        new Formatter(sb).format("ok").toString();
     }
 }
\ No newline at end of file
diff --git a/java/ql/test/library-tests/dataflow/taint-format/test.expected b/java/ql/test/library-tests/dataflow/taint-format/test.expected
index 22f8a10ad1ae..ddc0f36d753f 100644
--- a/java/ql/test/library-tests/dataflow/taint-format/test.expected
+++ b/java/ql/test/library-tests/dataflow/taint-format/test.expected
@@ -27,6 +27,10 @@
 | A.java:30:22:30:28 | taint(...) | A.java:36:9:36:10 | sb |
 | A.java:30:22:30:28 | taint(...) | A.java:36:9:36:21 | toString(...) |
 | A.java:40:22:40:28 | taint(...) | A.java:40:22:40:28 | taint(...) |
-| A.java:40:22:40:28 | taint(...) | A.java:43:18:43:20 | bad |
-| A.java:40:22:40:28 | taint(...) | A.java:44:9:44:46 | new ..[] { .. } |
-| A.java:40:22:40:28 | taint(...) | A.java:44:43:44:45 | bad |
+| A.java:40:22:40:28 | taint(...) | A.java:43:9:43:10 | sb [post update] |
+| A.java:40:22:40:28 | taint(...) | A.java:43:9:43:22 | append(...) |
+| A.java:40:22:40:28 | taint(...) | A.java:43:19:43:21 | bad |
+| A.java:40:22:40:28 | taint(...) | A.java:45:9:45:25 | new Formatter(...) |
+| A.java:40:22:40:28 | taint(...) | A.java:45:9:45:38 | format(...) |
+| A.java:40:22:40:28 | taint(...) | A.java:45:9:45:49 | toString(...) |
+| A.java:40:22:40:28 | taint(...) | A.java:45:23:45:24 | sb |

From 3416911ac6942fd3a59c531ce1ec5e38bbdd185c Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Mon, 12 Oct 2020 15:23:01 +0100
Subject: [PATCH 14/16] Java: Refector out StringBuilder and Number taint
 preserving callables

---
 .../semmle/code/java/dataflow/FlowSteps.qll   | 56 +++++++++++++++++
 .../dataflow/internal/TaintTrackingUtil.qll   | 62 -------------------
 2 files changed, 56 insertions(+), 62 deletions(-)

diff --git a/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll b/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
index 802e287bf435..e206703542a4 100644
--- a/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
+++ b/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
@@ -69,3 +69,59 @@ private class StringTaintPreservingCallable extends TaintPreservingCallable {
     this.hasName(["format", "formatted", "join"]) and arg = [0 .. getNumberOfParameters()]
   }
 }
+
+private class NumberTaintPreservingCallable extends TaintPreservingCallable {
+  int argument;
+
+  NumberTaintPreservingCallable() {
+    this.getDeclaringType().getASupertype*().hasQualifiedName("java.lang", "Number") and
+    (
+      this instanceof Constructor and
+      argument = 0
+      or
+      this.getName().matches(["to%String", "toByteArray", "%Value"]) and
+      argument = -1
+      or
+      this.getName().matches(["parse%", "valueOf%", "to%String", "decode"]) and
+      argument = 0
+    )
+  }
+
+  override predicate returnsTaintFrom(int arg) { arg = argument }
+}
+
+private class StringBuilderTaintPreservingCallable extends TaintPreservingCallable {
+  StringBuilderTaintPreservingCallable() {
+    exists(Class c | c = this.getDeclaringType().getASourceSupertype*() |
+      (
+        c.hasQualifiedName("java.lang", "StringBuilder") or
+        c.hasQualifiedName("java.lang", "StringBuffer") or
+        c.hasQualifiedName("java.io", "StringWriter")
+      ) and
+      (
+        this.hasName(["append", "insert", "replace", "toString"])
+        or
+        this.(Constructor).getParameterType(0) instanceof TypeString and
+        c = this.getDeclaringType()
+      )
+    )
+  }
+
+  override predicate returnsTaintFrom(int arg) {
+    arg = -1
+    or
+    this instanceof Constructor and arg = 0
+    or
+    this.hasName("append") and arg = 0
+    or
+    this.hasName("insert") and arg = 1
+    or
+    this.hasName("replace") and arg = 2
+  }
+
+  override predicate transfersTaint(int src, int sink) {
+    returnsTaintFrom(src) and
+    sink = -1 and
+    src != -1
+  }
+}
diff --git a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
index 8643b8e3c19c..76d63bc266c5 100644
--- a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
+++ b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
@@ -192,11 +192,6 @@ private predicate constructorStep(Expr tracked, ConstructorCall sink) {
       or
       s = "java.util.zip.GZIPInputStream" and argi = 0
       or
-      // string builders and buffers
-      s = "java.lang.StringBuilder" and argi = 0
-      or
-      s = "java.lang.StringBuffer" and argi = 0
-      or
       // a cookie with tainted ingredients is tainted
       s = "javax.servlet.http.Cookie" and argi = 0
       or
@@ -220,11 +215,6 @@ private predicate constructorStep(Expr tracked, ConstructorCall sink) {
       s = "java.io.File" and argi = 1
     )
     or
-    exists(RefType t | t.getQualifiedName() = "java.lang.Number" |
-      hasSubtype*(t, sink.getConstructedType())
-    ) and
-    argi = 0
-    or
     // wrappers constructed by extension
     exists(Constructor c, Parameter p, SuperConstructorInvocationStmt sup |
       c = sink.getConstructor() and
@@ -310,13 +300,6 @@ private predicate qualifierToMethodStep(Expr tracked, MethodAccess sink) {
 private predicate taintPreservingQualifierToMethod(Method m) {
   m instanceof CloneMethod
   or
-  exists(Class c | c.getQualifiedName() = "java.lang.Number" | hasSubtype*(c, m.getDeclaringType())) and
-  (
-    m.getName().matches("to%String") or
-    m.getName() = "toByteArray" or
-    m.getName().matches("%Value")
-  )
-  or
   m.getDeclaringType().getASupertype*().hasQualifiedName("java.io", "Reader") and
   (
     m.getName() = "read" and m.getNumberOfParameters() = 0
@@ -340,13 +323,6 @@ private predicate taintPreservingQualifierToMethod(Method m) {
   m.getDeclaringType().hasQualifiedName("java.io", "ObjectInputStream") and
   m.getName().matches("read%")
   or
-  (
-    m.getDeclaringType().hasQualifiedName("java.lang", "StringBuilder") or
-    m.getDeclaringType().hasQualifiedName("java.lang", "StringBuffer") or
-    m.getDeclaringType().hasQualifiedName("java.io", "StringWriter")
-  ) and
-  (m.getName() = "toString" or m.getName() = "append")
-  or
   m.getDeclaringType().hasQualifiedName("javax.xml.transform.sax", "SAXSource") and
   m.hasName("getInputSource")
   or
@@ -432,29 +408,6 @@ private predicate argToMethodStep(Expr tracked, MethodAccess sink) {
  * `arg`th argument is tainted.
  */
 private predicate taintPreservingArgumentToMethod(Method method, int arg) {
-  exists(Class c | c.getQualifiedName() = "java.lang.Number" |
-    hasSubtype*(c, method.getDeclaringType())
-  ) and
-  (
-    method.getName().matches("parse%") and arg = 0
-    or
-    method.getName().matches("valueOf%") and arg = 0
-    or
-    method.getName().matches("to%String") and arg = 0
-  )
-  or
-  (
-    method.getDeclaringType().hasQualifiedName("java.lang", "StringBuilder") or
-    method.getDeclaringType().hasQualifiedName("java.lang", "StringBuffer")
-  ) and
-  (
-    method.getName() = "append" and arg = 0
-    or
-    method.getName() = "insert" and arg = 1
-    or
-    method.getName() = "replace" and arg = 2
-  )
-  or
   (
     method.getDeclaringType().hasQualifiedName("java.util", "Base64$Encoder") or
     method.getDeclaringType().hasQualifiedName("java.util", "Base64$Decoder") or
@@ -518,10 +471,6 @@ private predicate taintPreservingArgumentToMethod(Method method, int arg) {
   method.hasName("sourceToInputSource") and
   arg = 0
   or
-  method.getDeclaringType().hasQualifiedName("java.io", "StringWriter") and
-  method.hasName("append") and
-  arg = 0
-  or
   method.(TaintPreservingCallable).returnsTaintFrom(arg)
 }
 
@@ -602,17 +551,6 @@ private predicate taintPreservingArgumentToQualifier(Method method, int arg) {
     )
   )
   or
-  exists(Method append |
-    method.overrides*(append) and
-    append.hasName("append") and
-    arg = 0 and
-    (
-      append.getDeclaringType().hasQualifiedName("java.lang", "StringBuilder") or
-      append.getDeclaringType().hasQualifiedName("java.lang", "StringBuffer") or
-      append.getDeclaringType().hasQualifiedName("java.io", "StringWriter")
-    )
-  )
-  or
   method.(TaintPreservingCallable).transfersTaint(arg, -1)
 }
 

From aa8bacb72402e061a925ecb919b29c56f0af06d8 Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Mon, 12 Oct 2020 15:36:14 +0100
Subject: [PATCH 15/16] Java: Update test output

---
 .../security/CWE-089/semmle/examples/taintedString.expected    | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/java/ql/test/query-tests/security/CWE-089/semmle/examples/taintedString.expected b/java/ql/test/query-tests/security/CWE-089/semmle/examples/taintedString.expected
index dd2204407074..62c4fdc3c465 100644
--- a/java/ql/test/query-tests/security/CWE-089/semmle/examples/taintedString.expected
+++ b/java/ql/test/query-tests/security/CWE-089/semmle/examples/taintedString.expected
@@ -20,6 +20,9 @@
 | Test.java:29:22:29:28 | tainted | 26 | Test.java:55:22:55:28 | ...[...] |
 | Test.java:29:22:29:28 | tainted | 29 | Test.java:58:4:58:27 | append(...) |
 | Test.java:29:22:29:28 | tainted | 29 | Test.java:58:19:58:26 | category |
+| Test.java:29:22:29:28 | tainted | 30 | Test.java:59:4:59:10 | querySb |
+| Test.java:29:22:29:28 | tainted | 30 | Test.java:59:4:59:37 | append(...) |
+| Test.java:29:22:29:28 | tainted | 31 | Test.java:60:29:60:35 | querySb |
 | Test.java:29:22:29:28 | tainted | 31 | Test.java:60:29:60:46 | toString(...) |
 | Test.java:29:22:29:28 | tainted | 33 | Test.java:62:47:62:61 | querySbToString |
 | Test.java:29:22:29:28 | tainted | 37 | Test.java:66:18:66:21 | args |

From b2a2412f1d9395e92685d415cc570a239372c34e Mon Sep 17 00:00:00 2001
From: Joe Farebrother <joefarebrother@github.com>
Date: Tue, 13 Oct 2020 11:30:02 +0100
Subject: [PATCH 16/16] Java: Clean up the constructor flow steps

---
 .../semmle/code/java/dataflow/FlowSteps.qll   | 50 ++++++++++++-------
 .../dataflow/internal/TaintTrackingUtil.qll   |  6 +--
 2 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll b/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
index e206703542a4..9ed602d86d71 100644
--- a/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
+++ b/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
@@ -51,25 +51,31 @@ abstract class TaintPreservingCallable extends Callable {
   predicate transfersTaint(int src, int sink) { none() }
 }
 
-private class StringTaintPreservingCallable extends TaintPreservingCallable {
-  StringTaintPreservingCallable() {
+private class StringTaintPreservingMethod extends TaintPreservingCallable {
+  StringTaintPreservingMethod() {
     this.getDeclaringType() instanceof TypeString and
     this
         .hasName(["concat", "copyValueOf", "endsWith", "format", "formatted", "getBytes", "indent",
               "intern", "join", "repeat", "split", "strip", "stripIndent", "stripLeading",
               "stripTrailing", "substring", "toCharArray", "toLowerCase", "toString", "toUpperCase",
-              "trim", "String"])
+              "trim"])
   }
 
   override predicate returnsTaintFrom(int arg) {
     arg = -1 and not this.isStatic()
     or
-    this.hasName(["concat", "copyValueOf", "String"]) and arg = 0
+    this.hasName(["concat", "copyValueOf"]) and arg = 0
     or
     this.hasName(["format", "formatted", "join"]) and arg = [0 .. getNumberOfParameters()]
   }
 }
 
+private class StringTaintPreservingConstructor extends Constructor, TaintPreservingCallable {
+  StringTaintPreservingConstructor() { this.getDeclaringType() instanceof TypeString }
+
+  override predicate returnsTaintFrom(int arg) { arg = 0 }
+}
+
 private class NumberTaintPreservingCallable extends TaintPreservingCallable {
   int argument;
 
@@ -90,25 +96,28 @@ private class NumberTaintPreservingCallable extends TaintPreservingCallable {
   override predicate returnsTaintFrom(int arg) { arg = argument }
 }
 
+/** Holds for the types `StringBuilder`, `StringBuffer`, and `StringWriter`. */
+private predicate stringBuilderType(RefType t) {
+  t.hasQualifiedName("java.lang", "StringBuilder") or
+  t.hasQualifiedName("java.lang", "StringBuffer") or
+  t.hasQualifiedName("java.io", "StringWriter")
+}
+
 private class StringBuilderTaintPreservingCallable extends TaintPreservingCallable {
   StringBuilderTaintPreservingCallable() {
-    exists(Class c | c = this.getDeclaringType().getASourceSupertype*() |
-      (
-        c.hasQualifiedName("java.lang", "StringBuilder") or
-        c.hasQualifiedName("java.lang", "StringBuffer") or
-        c.hasQualifiedName("java.io", "StringWriter")
-      ) and
-      (
-        this.hasName(["append", "insert", "replace", "toString"])
-        or
-        this.(Constructor).getParameterType(0) instanceof TypeString and
-        c = this.getDeclaringType()
-      )
+    exists(Method m |
+      this.(Method).overrides*(m) and
+      stringBuilderType(m.getDeclaringType()) and
+      m.hasName(["append", "insert", "replace", "toString", "write"])
     )
+    or
+    this.(Constructor).getParameterType(0) instanceof RefType and
+    stringBuilderType(this.getDeclaringType())
   }
 
   override predicate returnsTaintFrom(int arg) {
-    arg = -1
+    arg = -1 and
+    not this instanceof Constructor
     or
     this instanceof Constructor and arg = 0
     or
@@ -122,6 +131,11 @@ private class StringBuilderTaintPreservingCallable extends TaintPreservingCallab
   override predicate transfersTaint(int src, int sink) {
     returnsTaintFrom(src) and
     sink = -1 and
-    src != -1
+    src != -1 and
+    not this instanceof Constructor
+    or
+    this.hasName("write") and
+    src = 0 and
+    sink = -1
   }
 }
diff --git a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
index 76d63bc266c5..cbf492801496 100644
--- a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
+++ b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll
@@ -544,11 +544,7 @@ private predicate taintPreservingArgumentToQualifier(Method method, int arg) {
     method.overrides*(write) and
     write.hasName("write") and
     arg = 0 and
-    (
-      write.getDeclaringType().hasQualifiedName("java.io", "OutputStream")
-      or
-      write.getDeclaringType().hasQualifiedName("java.io", "StringWriter")
-    )
+    write.getDeclaringType().hasQualifiedName("java.io", "OutputStream")
   )
   or
   method.(TaintPreservingCallable).transfersTaint(arg, -1)