Merge pull request github#5329 from tamasvajk/feature/csv-taint-step

Java: migrate taint steps to CSV

Author: aschackmull

java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll

@@ -182,7 +182,91 @@ private predicate sourceModelCsv(string row) {
 
 private predicate sinkModelCsv(string row) { none() }
 
-private predicate summaryModelCsv(string row) { none() }
+private predicate summaryModelCsv(string row) {
+  row =
+    [
+      // qualifier to arg
+      "java.io;InputStream;true;read;(byte[]);;Argument[-1];Argument[0];taint",
+      "java.io;InputStream;true;read;(byte[],int,int);;Argument[-1];Argument[0];taint",
+      "java.io;ByteArrayOutputStream;false;writeTo;;;Argument[-1];Argument[0];taint",
+      "java.io;Reader;true;read;;;Argument[-1];Argument[0];taint",
+      // qualifier to return
+      "java.io;ByteArrayOutputStream;false;toByteArray;;;Argument[-1];ReturnValue;taint",
+      "java.io;ByteArrayOutputStream;false;toString;;;Argument[-1];ReturnValue;taint",
+      "java.util;StringTokenizer;false;nextElement;();;Argument[-1];ReturnValue;taint",
+      "java.util;StringTokenizer;false;nextToken;;;Argument[-1];ReturnValue;taint",
+      "javax.xml.transform.sax;SAXSource;false;getInputSource;;;Argument[-1];ReturnValue;taint",
+      "javax.xml.transform.stream;StreamSource;false;getInputStream;;;Argument[-1];ReturnValue;taint",
+      "java.nio;ByteBuffer;false;get;;;Argument[-1];ReturnValue;taint",
+      "java.net;URI;false;toURL;;;Argument[-1];ReturnValue;taint",
+      "java.io;File;false;toURI;;;Argument[-1];ReturnValue;taint",
+      "java.io;File;false;toPath;;;Argument[-1];ReturnValue;taint",
+      "java.nio.file;Path;false;toFile;;;Argument[-1];ReturnValue;taint",
+      "java.io;Reader;true;readLine;;;Argument[-1];ReturnValue;taint",
+      "java.io;Reader;true;read;();;Argument[-1];ReturnValue;taint",
+      // arg to return
+      "java.util;Base64$Encoder;false;encode;(byte[]);;Argument[0];ReturnValue;taint",
+      "java.util;Base64$Encoder;false;encode;(ByteBuffer);;Argument[0];ReturnValue;taint",
+      "java.util;Base64$Encoder;false;encodeToString;(byte[]);;Argument[0];ReturnValue;taint",
+      "java.util;Base64$Encoder;false;wrap;(OutputStream);;Argument[0];ReturnValue;taint",
+      "java.util;Base64$Decoder;false;decode;(byte[]);;Argument[0];ReturnValue;taint",
+      "java.util;Base64$Decoder;false;decode;(ByteBuffer);;Argument[0];ReturnValue;taint",
+      "java.util;Base64$Decoder;false;decode;(String);;Argument[0];ReturnValue;taint",
+      "java.util;Base64$Decoder;false;wrap;(InputStream);;Argument[0];ReturnValue;taint",
+      "org.apache.commons.codec;Encoder;true;encode;;;Argument[0];ReturnValue;taint",
+      "org.apache.commons.codec;Decoder;true;decode;;;Argument[0];ReturnValue;taint",
+      "org.apache.commons.io;IOUtils;false;buffer;;;Argument[0];ReturnValue;taint",
+      "org.apache.commons.io;IOUtils;false;readLines;;;Argument[0];ReturnValue;taint",
+      "org.apache.commons.io;IOUtils;false;readFully;(InputStream,int);;Argument[0];ReturnValue;taint",
+      "org.apache.commons.io;IOUtils;false;toBufferedInputStream;;;Argument[0];ReturnValue;taint",
+      "org.apache.commons.io;IOUtils;false;toBufferedReader;;;Argument[0];ReturnValue;taint",
+      "org.apache.commons.io;IOUtils;false;toByteArray;;;Argument[0];ReturnValue;taint",
+      "org.apache.commons.io;IOUtils;false;toCharArray;;;Argument[0];ReturnValue;taint",
+      "org.apache.commons.io;IOUtils;false;toInputStream;;;Argument[0];ReturnValue;taint",
+      "org.apache.commons.io;IOUtils;false;toString;;;Argument[0];ReturnValue;taint",
+      "java.net;URLDecoder;false;decode;;;Argument[0];ReturnValue;taint",
+      "java.net;URI;false;create;;;Argument[0];ReturnValue;taint",
+      "javax.xml.transform.sax;SAXSource;false;sourceToInputSource;;;Argument[0];ReturnValue;taint",
+      // arg to arg
+      "java.lang;System;false;arraycopy;;;Argument[0];Argument[2];taint",
+      "org.apache.commons.io;IOUtils;false;copy;;;Argument[0];Argument[1];taint",
+      "org.apache.commons.io;IOUtils;false;copyLarge;;;Argument[0];Argument[1];taint",
+      "org.apache.commons.io;IOUtils;false;read;;;Argument[0];Argument[1];taint",
+      "org.apache.commons.io;IOUtils;false;readFully;(InputStream,byte[]);;Argument[0];Argument[1];taint",
+      "org.apache.commons.io;IOUtils;false;readFully;(InputStream,byte[],int,int);;Argument[0];Argument[1];taint",
+      "org.apache.commons.io;IOUtils;false;readFully;(InputStream,ByteBuffer);;Argument[0];Argument[1];taint",
+      "org.apache.commons.io;IOUtils;false;readFully;(ReadableByteChannel,ByteBuffer);;Argument[0];Argument[1];taint",
+      "org.apache.commons.io;IOUtils;false;readFully;(Reader,char[]);;Argument[0];Argument[1];taint",
+      "org.apache.commons.io;IOUtils;false;readFully;(Reader,char[],int,int);;Argument[0];Argument[1];taint",
+      "org.apache.commons.io;IOUtils;false;write;;;Argument[0];Argument[1];taint",
+      "org.apache.commons.io;IOUtils;false;writeChunked;;;Argument[0];Argument[1];taint",
+      "org.apache.commons.io;IOUtils;false;writeLines;;;Argument[0];Argument[2];taint",
+      "org.apache.commons.io;IOUtils;false;writeLines;;;Argument[1];Argument[2];taint",
+      // constructor flow
+      "java.io;File;false;File;;;Argument[0];Argument[-1];taint",
+      "java.io;File;false;File;;;Argument[1];Argument[-1];taint",
+      "java.net;URI;false;URI;(String);;Argument[0];Argument[-1];taint",
+      "javax.xml.transform.stream;StreamSource;false;StreamSource;;;Argument[0];Argument[-1];taint",
+      "javax.xml.transform.sax;SAXSource;false;SAXSource;(InputSource);;Argument[0];Argument[-1];taint",
+      "javax.xml.transform.sax;SAXSource;false;SAXSource;(XMLReader,InputSource);;Argument[1];Argument[-1];taint",
+      "org.xml.sax;InputSource;false;InputSource;;;Argument[0];Argument[-1];taint",
+      "javax.servlet.http;Cookie;false;Cookie;;;Argument[0];Argument[-1];taint",
+      "javax.servlet.http;Cookie;false;Cookie;;;Argument[1];Argument[-1];taint",
+      "java.util.zip;ZipInputStream;false;ZipInputStream;;;Argument[0];Argument[-1];taint",
+      "java.util.zip;GZIPInputStream;false;GZIPInputStream;;;Argument[0];Argument[-1];taint",
+      "java.util;StringTokenizer;false;StringTokenizer;;;Argument[0];Argument[-1];taint",
+      "java.beans;XMLDecoder;false;XMLDecoder;;;Argument[0];Argument[-1];taint",
+      "com.esotericsoftware.kryo.io;Input;false;Input;;;Argument[0];Argument[-1];taint",
+      "java.io;BufferedInputStream;false;BufferedInputStream;;;Argument[0];Argument[-1];taint",
+      "java.io;DataInputStream;false;DataInputStream;;;Argument[0];Argument[-1];taint",
+      "java.io;ByteArrayInputStream;false;ByteArrayInputStream;;;Argument[0];Argument[-1];taint",
+      "java.io;ObjectInputStream;false;ObjectInputStream;;;Argument[0];Argument[-1];taint",
+      "java.io;StringReader;false;StringReader;;;Argument[0];Argument[-1];taint",
+      "java.io;CharArrayReader;false;CharArrayReader;;;Argument[0];Argument[-1];taint",
+      "java.io;BufferedReader;false;BufferedReader;;;Argument[0];Argument[-1];taint",
+      "java.io;InputStreamReader;false;InputStreamReader;;;Argument[0];Argument[-1];taint"
+    ]
+}
 
 /**
  * A unit class for adding additional source model rows.

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

@@ -166,60 +166,6 @@ private predicate inputStreamWrapper(Constructor c, int argi) {
 /** An object construction that preserves the data flow status of any of its arguments. */
 private predicate constructorStep(Expr tracked, ConstructorCall sink) {
   exists(int argi | sink.getArgument(argi) = tracked |
-    exists(string s | sink.getConstructedType().getQualifiedName() = s |
-      // some readers preserve the content of streams
-      s = "java.io.InputStreamReader" and argi = 0
-      or
-      s = "java.io.BufferedReader" and argi = 0
-      or
-      s = "java.io.CharArrayReader" and argi = 0
-      or
-      s = "java.io.StringReader" and argi = 0
-      or
-      // data preserved through streams
-      s = "java.io.ObjectInputStream" and argi = 0
-      or
-      s = "java.io.ByteArrayInputStream" and argi = 0
-      or
-      s = "java.io.DataInputStream" and argi = 0
-      or
-      s = "java.io.BufferedInputStream" and argi = 0
-      or
-      s = "com.esotericsoftware.kryo.io.Input" and argi = 0
-      or
-      s = "java.beans.XMLDecoder" and argi = 0
-      or
-      // a tokenizer preserves the content of a string
-      s = "java.util.StringTokenizer" and argi = 0
-      or
-      // unzipping the stream preserves content
-      s = "java.util.zip.ZipInputStream" and argi = 0
-      or
-      s = "java.util.zip.GZIPInputStream" and argi = 0
-      or
-      // a cookie with tainted ingredients is tainted
-      s = "javax.servlet.http.Cookie" and argi = 0
-      or
-      s = "javax.servlet.http.Cookie" and argi = 1
-      or
-      // various xml stream source constructors.
-      s = "org.xml.sax.InputSource" and argi = 0
-      or
-      s = "javax.xml.transform.sax.SAXSource" and argi = 0 and sink.getNumArgument() = 1
-      or
-      s = "javax.xml.transform.sax.SAXSource" and argi = 1 and sink.getNumArgument() = 2
-      or
-      s = "javax.xml.transform.stream.StreamSource" and argi = 0
-      or
-      //a URI constructed from a tainted string is tainted.
-      s = "java.net.URI" and argi = 0 and sink.getNumArgument() = 1
-      or
-      //a File constructed from a tainted string is tainted.
-      s = "java.io.File" and argi = 0
-      or
-      s = "java.io.File" and argi = 1
-    )
-    or
     // wrappers constructed by extension
     exists(Constructor c, Parameter p, SuperConstructorInvocationStmt sup |
       c = sink.getConstructor() and
@@ -267,32 +213,12 @@ private int argToParam(Call call, int 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(), argToParam(ma, arg)) and
+    ma.getMethod().(TaintPreservingCallable).transfersTaint(-1, argToParam(ma, arg)) and
     tracked = ma.getQualifier() and
     sink = ma.getArgument(arg)
   )
 }
 
-/** Methods that passes tainted data from qualifier to argument. */
-private predicate taintPreservingQualifierToArgument(Method m, int arg) {
-  m.getDeclaringType().hasQualifiedName("java.io", "ByteArrayOutputStream") and
-  m.hasName("writeTo") and
-  arg = 0
-  or
-  exists(Method read |
-    m.overrides*(read) and
-    read.getDeclaringType().hasQualifiedName("java.io", "InputStream") and
-    read.hasName("read") and
-    arg = 0
-  )
-  or
-  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. */
 private predicate qualifierToMethodStep(Expr tracked, MethodAccess sink) {
   (taintPreservingQualifierToMethod(sink.getMethod()) or unsafeEscape(sink)) and
@@ -305,50 +231,16 @@ private predicate qualifierToMethodStep(Expr tracked, MethodAccess sink) {
 private predicate taintPreservingQualifierToMethod(Method m) {
   m instanceof CloneMethod
   or
-  m.getDeclaringType().getASupertype*().hasQualifiedName("java.io", "Reader") and
-  (
-    m.getName() = "read" and m.getNumberOfParameters() = 0
-    or
-    m.getName() = "readLine"
-  )
-  or
   m.getDeclaringType().getQualifiedName().matches("%StringWriter") and
   (
     m.getName() = "getBuffer"
     or
     m.getName() = "toString"
   )
   or
-  m.getDeclaringType().hasQualifiedName("java.util", "StringTokenizer") and
-  m.getName().matches("next%")
-  or
-  m.getDeclaringType().hasQualifiedName("java.io", "ByteArrayOutputStream") and
-  (m.getName() = "toByteArray" or m.getName() = "toString")
-  or
   m.getDeclaringType().hasQualifiedName("java.io", "ObjectInputStream") and
   m.getName().matches("read%")
   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.getDeclaringType().hasQualifiedName("java.nio", "ByteBuffer") and
-  m.hasName("get")
-  or
-  m.getDeclaringType() instanceof TypeFile and
-  m.hasName("toPath")
-  or
-  m.getDeclaringType() instanceof TypePath and
-  m.hasName("toFile")
-  or
-  m.getDeclaringType() instanceof TypeFile and
-  m.hasName("toURI")
-  or
-  m.getDeclaringType() instanceof TypeUri and
-  m.hasName("toURL")
-  or
   m instanceof GetterMethod and
   m.getDeclaringType().getASubtype*() instanceof SpringUntrustedDataType and
   not m.getDeclaringType() instanceof TypeObject
@@ -421,69 +313,13 @@ private predicate argToMethodStep(Expr tracked, MethodAccess sink) {
  * `arg`th argument is tainted.
  */
 private predicate taintPreservingArgumentToMethod(Method method, int arg) {
-  (
-    method.getDeclaringType().hasQualifiedName("java.util", "Base64$Encoder") or
-    method.getDeclaringType().hasQualifiedName("java.util", "Base64$Decoder") or
-    method
-        .getDeclaringType()
-        .getASupertype*()
-        .hasQualifiedName("org.apache.commons.codec", "Encoder") or
-    method
-        .getDeclaringType()
-        .getASupertype*()
-        .hasQualifiedName("org.apache.commons.codec", "Decoder")
-  ) and
-  (
-    method.getName() = "encode" and arg = 0 and method.getNumberOfParameters() = 1
-    or
-    method.getName() = "decode" and arg = 0 and method.getNumberOfParameters() = 1
-    or
-    method.getName() = "encodeToString" and arg = 0
-    or
-    method.getName() = "wrap" and arg = 0
-  )
-  or
   method.getDeclaringType().hasQualifiedName("org.apache.commons.codec.binary", "Base64") and
   (
     method.getName() = "decodeBase64" and arg = 0
     or
     method.getName().matches("encodeBase64%") and arg = 0
   )
   or
-  method.getDeclaringType().hasQualifiedName("org.apache.commons.io", "IOUtils") and
-  (
-    method.getName() = "buffer" and arg = 0
-    or
-    method.getName() = "readLines" and arg = 0
-    or
-    method.getName() = "readFully" and arg = 0 and method.getParameterType(1).hasName("int")
-    or
-    method.getName() = "toBufferedInputStream" and arg = 0
-    or
-    method.getName() = "toBufferedReader" and arg = 0
-    or
-    method.getName() = "toByteArray" and arg = 0
-    or
-    method.getName() = "toCharArray" and arg = 0
-    or
-    method.getName() = "toInputStream" and arg = 0
-    or
-    method.getName() = "toString" and arg = 0
-  )
-  or
-  method.getDeclaringType().hasQualifiedName("java.net", "URLDecoder") and
-  method.hasName("decode") and
-  arg = 0
-  or
-  // A URI created from a tainted string is still tainted.
-  method.getDeclaringType() instanceof TypeUri and
-  method.hasName("create") and
-  arg = 0
-  or
-  method.getDeclaringType().hasQualifiedName("javax.xml.transform.sax", "SAXSource") and
-  method.hasName("sourceToInputSource") and
-  arg = 0
-  or
   method.(TaintPreservingCallable).returnsTaintFrom(arg)
 }
 
@@ -493,48 +329,13 @@ 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, argToParam(ma, input), argToParam(ma, output)) and
+    method.(TaintPreservingCallable).transfersTaint(argToParam(ma, input), argToParam(ma, output)) and
     ma.getMethod() = method and
     ma.getArgument(input) = tracked and
     ma.getArgument(output) = sink
   )
 }
 
-/**
- * Holds if `method` is a library method that writes tainted data to the
- * `output`th argument if the `input`th argument is tainted.
- */
-private predicate taintPreservingArgToArg(Method method, int input, int output) {
-  method.getDeclaringType().hasQualifiedName("org.apache.commons.io", "IOUtils") and
-  (
-    method.hasName("copy") and input = 0 and output = 1
-    or
-    method.hasName("copyLarge") and input = 0 and output = 1
-    or
-    method.hasName("read") and input = 0 and output = 1
-    or
-    method.hasName("readFully") and
-    input = 0 and
-    output = 1 and
-    not method.getParameterType(1).hasName("int")
-    or
-    method.hasName("write") and input = 0 and output = 1
-    or
-    method.hasName("writeChunked") and input = 0 and output = 1
-    or
-    method.hasName("writeLines") and input = 0 and output = 2
-    or
-    method.hasName("writeLines") and input = 1 and output = 2
-  )
-  or
-  method.getDeclaringType().hasQualifiedName("java.lang", "System") and
-  method.hasName("arraycopy") and
-  input = 0 and
-  output = 2
-  or
-  method.(TaintPreservingCallable).transfersTaint(input, output)
-}
-
 /**
  * Holds if `tracked` is the argument of a method that transfers taint
  * from the argument to the qualifier and `sink` is the qualifier.