Java: Refector out StringBuilder and Number taint preserving callables

Author: joefarebrother

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
+  }
+}

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)
 }