From 40126563ef80ec3e8f9f21eecb107c7258a268c7 Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Thu, 4 Mar 2021 15:40:32 +0100 Subject: [PATCH 1/7] Java: migrate 'qualifier to arg' taint steps to CSV --- .../code/java/dataflow/ExternalFlow.qll | 11 +++++++++- .../dataflow/internal/TaintTrackingUtil.qll | 22 +------------------ 2 files changed, 11 insertions(+), 22 deletions(-) diff --git a/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll b/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll index 9fd7365f63ca..de774d1884fc 100644 --- a/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll +++ b/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll @@ -182,7 +182,16 @@ 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" + ] +} /** * A unit class for adding additional source model rows. 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 aadb42469ad0..29244618a46d 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll @@ -267,32 +267,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 From 5cdbde268632778a04318f7052bd80d772027d0a Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Thu, 4 Mar 2021 15:59:52 +0100 Subject: [PATCH 2/7] Java: migrate 'qualifier to return' taint steps to CSV --- .../code/java/dataflow/ExternalFlow.qll | 16 ++++++++- .../dataflow/internal/TaintTrackingUtil.qll | 34 ------------------- 2 files changed, 15 insertions(+), 35 deletions(-) diff --git a/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll b/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll index de774d1884fc..9cd79e37c52c 100644 --- a/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll +++ b/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll @@ -189,7 +189,21 @@ private predicate summaryModelCsv(string row) { "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" + "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" ] } 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 29244618a46d..9f5054f7c803 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll @@ -285,13 +285,6 @@ 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" @@ -299,36 +292,9 @@ private predicate taintPreservingQualifierToMethod(Method m) { 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 From 7e1534a6cdb42dd6a7f52d6cbc84b524c783ee5b Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Thu, 4 Mar 2021 16:16:55 +0100 Subject: [PATCH 3/7] Java: migrate 'arg to return' taint steps to CSV --- .../code/java/dataflow/ExternalFlow.qll | 25 ++++++++- .../dataflow/internal/TaintTrackingUtil.qll | 56 ------------------- 2 files changed, 24 insertions(+), 57 deletions(-) diff --git a/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll b/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll index 9cd79e37c52c..d2e8734feae8 100644 --- a/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll +++ b/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll @@ -203,7 +203,30 @@ private predicate summaryModelCsv(string row) { "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" + "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" ] } 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 9f5054f7c803..791436908239 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll @@ -367,28 +367,6 @@ 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 @@ -396,40 +374,6 @@ private predicate taintPreservingArgumentToMethod(Method method, int arg) { 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) } From f9a207dd9f8acff113e11c01981be3d3225d647a Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Thu, 4 Mar 2021 16:30:25 +0100 Subject: [PATCH 4/7] Java: migrate 'arg to arg' taint steps to CSV --- .../code/java/dataflow/ExternalFlow.qll | 17 ++++++++- .../dataflow/internal/TaintTrackingUtil.qll | 37 +------------------ 2 files changed, 17 insertions(+), 37 deletions(-) diff --git a/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll b/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll index d2e8734feae8..d1ef5c28cbc3 100644 --- a/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll +++ b/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll @@ -226,7 +226,22 @@ private predicate summaryModelCsv(string row) { "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" + "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" ] } 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 791436908239..67c5f8891248 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll @@ -383,48 +383,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. From af0dff8c6fa945cfe9f8dfc9d4697fca8c7f3c41 Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Fri, 5 Mar 2021 10:15:13 +0100 Subject: [PATCH 5/7] Java: migrate constructor flow taint steps to CSV --- .../code/java/dataflow/ExternalFlow.qll | 25 ++++++++- .../dataflow/internal/TaintTrackingUtil.qll | 54 ------------------- 2 files changed, 24 insertions(+), 55 deletions(-) diff --git a/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll b/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll index d1ef5c28cbc3..ec45f6f8103c 100644 --- a/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll +++ b/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll @@ -241,7 +241,30 @@ private predicate summaryModelCsv(string row) { "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" + "org.apache.commons.io;IOUtils;false;writeLines;;;Argument[1];Argument[2];taint", + // constructor flow + "java.io;File;false;File;;;Argument[0];ReturnValue;taint", + "java.io;File;false;File;;;Argument[1];ReturnValue;taint", + "java.net;URI;false;URI;(String);;Argument[0];ReturnValue;taint", + "javax.xml.transform.stream;StreamSource;false;StreamSource;;;Argument[0];ReturnValue;taint", + "javax.xml.transform.sax;SAXSource;false;SAXSource;(InputSource);;Argument[0];ReturnValue;taint", + "javax.xml.transform.sax;SAXSource;false;SAXSource;(XMLReader,InputSource);;Argument[1];ReturnValue;taint", + "org.xml.sax;InputSource;false;InputSource;;;Argument[0];ReturnValue;taint", + "javax.servlet.http;Cookie;false;Cookie;;;Argument[0];ReturnValue;taint", + "javax.servlet.http;Cookie;false;Cookie;;;Argument[1];ReturnValue;taint", + "java.util.zip;ZipInputStream;false;ZipInputStream;;;Argument[0];ReturnValue;taint", + "java.util.zip;GZIPInputStream;false;GZIPInputStream;;;Argument[0];ReturnValue;taint", + "java.util;StringTokenizer;false;StringTokenizer;;;Argument[0];ReturnValue;taint", + "java.beans;XMLDecoder;false;XMLDecoder;;;Argument[0];ReturnValue;taint", + "com.esotericsoftware.kryo.io;Input;false;Input;;;Argument[0];ReturnValue;taint", + "java.io;BufferedInputStream;false;BufferedInputStream;;;Argument[0];ReturnValue;taint", + "java.io;DataInputStream;false;DataInputStream;;;Argument[0];ReturnValue;taint", + "java.io;ByteArrayInputStream;false;ByteArrayInputStream;;;Argument[0];ReturnValue;taint", + "java.io;ObjectInputStream;false;ObjectInputStream;;;Argument[0];ReturnValue;taint", + "java.io;StringReader;false;StringReader;;;Argument[0];ReturnValue;taint", + "java.io;CharArrayReader;false;CharArrayReader;;;Argument[0];ReturnValue;taint", + "java.io;BufferedReader;false;BufferedReader;;;Argument[0];ReturnValue;taint", + "java.io;InputStreamReader;false;InputStreamReader;;;Argument[0];ReturnValue;taint" ] } 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 67c5f8891248..05f601231113 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll +++ b/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 From e3534d16355b7f3cc798738149a226db20e278a4 Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Fri, 5 Mar 2021 10:40:37 +0100 Subject: [PATCH 6/7] Java: cover wrapped constructor taint flow --- .../src/semmle/code/java/dataflow/ExternalFlow.qll | 12 ++++++++++++ .../java/dataflow/internal/TaintTrackingUtil.qll | 3 +++ 2 files changed, 15 insertions(+) diff --git a/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll b/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll index ec45f6f8103c..27875a212734 100644 --- a/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll +++ b/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll @@ -697,3 +697,15 @@ predicate summaryStep(Node node1, Node node2, string kind) { interpretOutput(output, 0, ref, TNode(node2)) ) } + +/** + * Holds if `node1` to `node2` is specified as a flow step with the given kind, input and output + * in a CSV flow model. + */ +predicate summaryStep(Node node1, Node node2, string kind, string input, string output) { + exists(Top ref | + summaryElementRef(ref, input, output, kind) and + interpretInput(input, 0, ref, TNode(node1)) and + interpretOutput(output, 0, ref, TNode(node2)) + ) +} 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 05f601231113..21d52dba7749 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll @@ -166,6 +166,9 @@ 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 | + summaryStep(any(DataFlow::Node n | n.asExpr() = tracked), + any(DataFlow::Node n | n.asExpr() = sink), "taint", "Argument(" + argi + ")", "ReturnValue") + or // wrappers constructed by extension exists(Constructor c, Parameter p, SuperConstructorInvocationStmt sup | c = sink.getConstructor() and From d02fba8c37216e01e39f185778060efa41ccbcce Mon Sep 17 00:00:00 2001 From: Tamas Vajk Date: Tue, 16 Mar 2021 10:06:20 +0100 Subject: [PATCH 7/7] Java: adjust wrapped constructor calls --- .../code/java/dataflow/ExternalFlow.qll | 56 ++++++++----------- .../dataflow/internal/TaintTrackingUtil.qll | 3 - 2 files changed, 22 insertions(+), 37 deletions(-) diff --git a/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll b/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll index 27875a212734..28808bc722d5 100644 --- a/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll +++ b/java/ql/src/semmle/code/java/dataflow/ExternalFlow.qll @@ -243,28 +243,28 @@ private predicate summaryModelCsv(string row) { "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];ReturnValue;taint", - "java.io;File;false;File;;;Argument[1];ReturnValue;taint", - "java.net;URI;false;URI;(String);;Argument[0];ReturnValue;taint", - "javax.xml.transform.stream;StreamSource;false;StreamSource;;;Argument[0];ReturnValue;taint", - "javax.xml.transform.sax;SAXSource;false;SAXSource;(InputSource);;Argument[0];ReturnValue;taint", - "javax.xml.transform.sax;SAXSource;false;SAXSource;(XMLReader,InputSource);;Argument[1];ReturnValue;taint", - "org.xml.sax;InputSource;false;InputSource;;;Argument[0];ReturnValue;taint", - "javax.servlet.http;Cookie;false;Cookie;;;Argument[0];ReturnValue;taint", - "javax.servlet.http;Cookie;false;Cookie;;;Argument[1];ReturnValue;taint", - "java.util.zip;ZipInputStream;false;ZipInputStream;;;Argument[0];ReturnValue;taint", - "java.util.zip;GZIPInputStream;false;GZIPInputStream;;;Argument[0];ReturnValue;taint", - "java.util;StringTokenizer;false;StringTokenizer;;;Argument[0];ReturnValue;taint", - "java.beans;XMLDecoder;false;XMLDecoder;;;Argument[0];ReturnValue;taint", - "com.esotericsoftware.kryo.io;Input;false;Input;;;Argument[0];ReturnValue;taint", - "java.io;BufferedInputStream;false;BufferedInputStream;;;Argument[0];ReturnValue;taint", - "java.io;DataInputStream;false;DataInputStream;;;Argument[0];ReturnValue;taint", - "java.io;ByteArrayInputStream;false;ByteArrayInputStream;;;Argument[0];ReturnValue;taint", - "java.io;ObjectInputStream;false;ObjectInputStream;;;Argument[0];ReturnValue;taint", - "java.io;StringReader;false;StringReader;;;Argument[0];ReturnValue;taint", - "java.io;CharArrayReader;false;CharArrayReader;;;Argument[0];ReturnValue;taint", - "java.io;BufferedReader;false;BufferedReader;;;Argument[0];ReturnValue;taint", - "java.io;InputStreamReader;false;InputStreamReader;;;Argument[0];ReturnValue;taint" + "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" ] } @@ -697,15 +697,3 @@ predicate summaryStep(Node node1, Node node2, string kind) { interpretOutput(output, 0, ref, TNode(node2)) ) } - -/** - * Holds if `node1` to `node2` is specified as a flow step with the given kind, input and output - * in a CSV flow model. - */ -predicate summaryStep(Node node1, Node node2, string kind, string input, string output) { - exists(Top ref | - summaryElementRef(ref, input, output, kind) and - interpretInput(input, 0, ref, TNode(node1)) and - interpretOutput(output, 0, ref, TNode(node2)) - ) -} 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 21d52dba7749..05f601231113 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll @@ -166,9 +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 | - summaryStep(any(DataFlow::Node n | n.asExpr() = tracked), - any(DataFlow::Node n | n.asExpr() = sink), "taint", "Argument(" + argi + ")", "ReturnValue") - or // wrappers constructed by extension exists(Constructor c, Parameter p, SuperConstructorInvocationStmt sup | c = sink.getConstructor() and