Merge pull request #4430 from joefarebrother/tainttrackingutils-refactor

Java: Refactor part of TaintTrackingUtil.qll
2026-04-29 18:55:14 +02:00 · 2020-10-15 16:05:38 +01:00
parent b05cc2eafd b2a2412f1d
commit 388f60f818
10 changed files with 348 additions and 247 deletions
--- a/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
+++ b/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll
@@ -0,0 +1,141 @@
+/**
+ * Provides classes representing various flow steps for taint tracking.
+ */
+
+private import java
+private import semmle.code.java.dataflow.DataFlow
+
+/**
+ * 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 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 TaintPreservingCallable extends Callable {
+  /**
+   * 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 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 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"])
+  }
+
+  override predicate returnsTaintFrom(int arg) {
+    arg = -1 and not this.isStatic()
+    or
+    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;
+
+  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 }
+}
+
+/** 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(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 and
+    not this instanceof Constructor
+    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 and
+    not this instanceof Constructor
+    or
+    this.hasName("write") and
+    src = 0 and
+    sink = -1
+  }
+}
--- 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,11 @@ 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.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
@@ -55,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.
@@ -180,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
@@ -213,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
@@ -241,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
@@ -272,13 +241,28 @@ private predicate constructorStep(Expr tracked, ConstructorCall sink) {
      argi = 0 and
      tracked.getType() instanceof TypeString
    )
+    or
+    sink.getConstructor().(TaintPreservingCallable).returnsTaintFrom(argToParam(sink, argi))
+  )
+}
+
+/**
+ * Converts an argument index to a formal parameter index.
+ * This is relevant for varadic methods.
+ */
+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
  )
 }

 /** 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)
  )
@@ -300,6 +284,8 @@ private predicate taintPreservingQualifierToArgument(Method m, int arg) {
  m.getDeclaringType().getASupertype*().hasQualifiedName("java.io", "Reader") and
  m.hasName("read") and
  arg = 0
+  or
+  m.(TaintPreservingCallable).transfersTaint(-1, arg)
 }

 /** Access to a method that passes taint from the qualifier. */
@@ -314,28 +300,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
-    m.getName() = "toByteArray" or
-    m.getName().matches("%Value")
-  )
-  or
  m.getDeclaringType().getASupertype*().hasQualifiedName("java.io", "Reader") and
  (
    m.getName() = "read" and m.getNumberOfParameters() = 0
@@ -359,21 +323,12 @@ 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
  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
@@ -389,10 +344,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
@@ -408,19 +359,10 @@ 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)
-  // 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"])
+  m.(TaintPreservingCallable).returnsTaintFrom(-1)
 }

-private class StringReplaceMethod extends Method {
+private class StringReplaceMethod extends TaintPreservingCallable {
  StringReplaceMethod() {
    getDeclaringType() instanceof TypeString and
    (
@@ -429,6 +371,8 @@ private class StringReplaceMethod extends Method {
      hasName("replaceFirst")
    )
  }
+
+  override predicate returnsTaintFrom(int arg) { arg = 1 }
 }

 private predicate unsafeEscape(MethodAccess ma) {
@@ -444,16 +388,10 @@ 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
-  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
@@ -471,63 +409,11 @@ 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 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)
-  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.
 */
 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
-  (
-    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() instanceof TypeString and
-  method.getName() = "concat" 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
@@ -591,41 +477,7 @@ 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
-  // 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
-  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
-  ) 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
+  method.(TaintPreservingCallable).returnsTaintFrom(arg)
 }

 /**
@@ -634,7 +486,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
@@ -673,17 +525,7 @@ 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
-  input = 1 and
-  output = 0
+  method.(TaintPreservingCallable).transfersTaint(input, output)
 }

 /**
@@ -692,25 +534,11 @@ 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()
  )
-  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")
 }

 /**
@@ -722,27 +550,10 @@ 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
-  exists(Method append |
-    method.overrides*(append) and
-    append.hasName("append") and
-    arg = 0 and
-    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
+  method.(TaintPreservingCallable).transfersTaint(arg, -1)
 }

 /** A comparison or equality test with a constant. */
@@ -866,6 +677,32 @@ private class TypeFormatter extends Class {
  TypeFormatter() { this.hasQualifiedName("java.util", "Formatter") }
 }

+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) {
+    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()]
+  }
+}
+
 private import StringBuilderVarModule

 module StringBuilderVarModule {
--- 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 TaintPreservingCallable {
+  OverridingGetMethod() { this = any(GuiceProvider gp).getAnOverridingGetMethod() }
+
+  override predicate returnsTaintFrom(int arg) { arg = -1 }
+}
--- 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 TaintPreservingCallable {
+  TaintPreservingGetterMethod() { this = any(ProtobufMessageLite p).getAGetterMethod() }
+
+  override predicate returnsTaintFrom(int arg) { arg = -1 }
+}
+
+private class TaintPreservingParseFromMethod extends TaintPreservingCallable {
+  TaintPreservingParseFromMethod() {
+    exists(ProtobufParser p | this = p.getAParseFromMethod())
+    or
+    exists(ProtobufMessageLite m | this = m.getAParseFromMethod())
+  }
+
+  override predicate returnsTaintFrom(int arg) { arg = 0 }
+}
--- 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, TaintPreservingCallable {
  IntentGetExtraMethod() {
    (getName().regexpMatch("get\\w+Extra") or hasName("getExtras")) and
    getDeclaringType() instanceof TypeIntent
  }
+
+  override predicate returnsTaintFrom(int arg) { arg = -1 }
 }
--- 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
+import semmle.code.java.dataflow.FlowSteps

 /**
 * The class `android.database.sqlite.SQLiteDatabase`.
@@ -226,3 +227,73 @@ private class ContentProviderUpdateMethod extends SQLiteRunner {

  override int sqlIndex() { result = 2 }
 }
+
+private class QueryBuilderBuildMethod extends TaintPreservingCallable {
+  int argument;
+
+  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)
+      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 }
+}
+
+private class QueryBuilderAppendMethod extends TaintPreservingCallable {
+  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 TaintPreservingCallable {
+  UnsafeAppendUtilMethod() {
+    this.getDeclaringType() instanceof TypeDatabaseUtils and
+    // String[] appendSelectionArgs(String[] originalValues, String[] newValues)
+    // String concatenateWhere(String a, String b)
+    this.hasName(["appendSelectionArgs", "concatenateWhere"])
+  }
+
+  override predicate returnsTaintFrom(int arg) { arg = [0 .. getNumberOfParameters()] }
+}
+
+private class TaintPreservingQueryMethod extends TaintPreservingCallable {
+  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 returnsTaintFrom(int arg) { arg = 0 }
+}
--- a/java/ql/src/semmle/code/java/frameworks/jackson/JacksonSerializability.qll
+++ b/java/ql/src/semmle/code/java/frameworks/jackson/JacksonSerializability.qll
@@ -8,6 +8,7 @@ import semmle.code.java.Serializability
 import semmle.code.java.Reflection
 import semmle.code.java.dataflow.DataFlow
 import semmle.code.java.dataflow.DataFlow5
+import semmle.code.java.dataflow.FlowSteps

 /**
 * A `@com.fasterxml.jackson.annotation.JsonIgnore` annoation.
@@ -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 Method, TaintPreservingCallable {
  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 returnsTaintFrom(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. */
--- 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"; }
@@ -14,6 +14,7 @@ class A {
        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() {
@@ -37,9 +38,10 @@ class A {

    public static void test4() {
        String bad = taint();
-        Console c = System.console();
+        StringBuilder sb = new StringBuilder();

-        c.format(bad);
-        c.readLine("Enter something: %s", bad);
+        sb.append(bad);
+
+        new Formatter(sb).format("ok").toString();
    }
 }
--- a/java/ql/test/library-tests/dataflow/taint-format/test.expected
+++ b/java/ql/test/library-tests/dataflow/taint-format/test.expected
@@ -7,22 +7,30 @@
 | 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: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: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 |
--- 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 |