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..9ed602d86d71 --- /dev/null +++ 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 + } +} 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..cbf492801496 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,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 @@ -383,10 +338,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 @@ -402,19 +353,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 columnsPresentInTable, int computedColumnsOffset, String typeDiscriminatorValue, String selection, String[] selectionArgs, String groupBy, String having) - // buildUnionSubQuery(String typeDiscriminatorColumn, String[] unionColumns, Set 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 ( @@ -423,6 +365,8 @@ private class StringReplaceMethod extends Method { hasName("replaceFirst") ) } + + override predicate returnsTaintFrom(int arg) { arg = 1 } } private predicate unsafeEscape(MethodAccess ma) { @@ -438,16 +382,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 @@ -465,63 +403,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 @@ -585,41 +471,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 columnsPresentInTable, int computedColumnsOffset, String typeDiscriminatorValue, String selection, String[] selectionArgs, String groupBy, String having) - // buildUnionSubQuery(String typeDiscriminatorColumn, String[] unionColumns, Set 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) } /** @@ -628,7 +480,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 @@ -667,17 +519,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) } /** @@ -686,25 +528,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") } /** @@ -716,27 +544,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 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. */ @@ -860,6 +671,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 { diff --git a/java/ql/src/semmle/code/java/frameworks/Guice.qll b/java/ql/src/semmle/code/java/frameworks/Guice.qll index ad1735c8f618..f7154df4bd20 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 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 8af99d7f6086..7382294f6f9f 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 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 } +} 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..c4894a5976c9 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, TaintPreservingCallable { IntentGetExtraMethod() { (getName().regexpMatch("get\\w+Extra") or hasName("getExtras")) and getDeclaringType() instanceof TypeIntent } + + 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 e0b41027232e..7a8defc84d18 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 +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 columnsPresentInTable, int computedColumnsOffset, String typeDiscriminatorValue, String selection, String[] selectionArgs, String groupBy, String having) + // buildUnionSubQuery(String typeDiscriminatorColumn, String[] unionColumns, Set 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 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 } +} 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..3356e31d965b 100644 --- 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. */ 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..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"; } @@ -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() { @@ -37,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 c52918fadc5e..ddc0f36d753f 100644 --- 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 | 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 |