diff --git a/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll b/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll index 9ed602d86d7..287a81d7f64 100644 --- a/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll +++ b/java/ql/src/semmle/code/java/dataflow/FlowSteps.qll @@ -15,6 +15,7 @@ module Frameworks { 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.guava.Guava } /** 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 62d96409b0e..e7764d90d51 100644 --- a/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll +++ b/java/ql/src/semmle/code/java/dataflow/internal/TaintTrackingUtil.qll @@ -7,14 +7,9 @@ private import semmle.code.java.security.SecurityTests private import semmle.code.java.security.Validation private import semmle.code.java.Maps private import semmle.code.java.dataflow.internal.ContainerFlow -<<<<<<< HEAD private import semmle.code.java.frameworks.spring.SpringController private import semmle.code.java.frameworks.spring.SpringHttp import semmle.code.java.dataflow.FlowSteps -======= -private import semmle.code.java.frameworks.jackson.JacksonSerializability -private import semmle.code.java.frameworks.guava.Guava ->>>>>>> 61c00e344... Java: Add modelling for Guava `Strings`, `Splitter`, and `Joiner` /** * Holds if taint can flow from `src` to `sink` in zero or more @@ -290,11 +285,7 @@ private predicate taintPreservingQualifierToArgument(Method m, int arg) { m.hasName("read") and arg = 0 or -<<<<<<< HEAD m.(TaintPreservingCallable).transfersTaint(-1, arg) -======= - m.(GuavaTaintPropagationMethod).propagatesTaint(-1, arg) ->>>>>>> 61c00e344... Java: Add modelling for Guava `Strings`, `Splitter`, and `Joiner` } /** Access to a method that passes taint from the qualifier. */ @@ -368,11 +359,7 @@ private predicate taintPreservingQualifierToMethod(Method m) { ) ) or -<<<<<<< HEAD m.(TaintPreservingCallable).returnsTaintFrom(-1) -======= - m.(GuavaTaintPropagationMethod).propagatesTaint(-1, -2) ->>>>>>> 61c00e344... Java: Add modelling for Guava `Strings`, `Splitter`, and `Joiner` } private class StringReplaceMethod extends TaintPreservingCallable { @@ -490,26 +477,7 @@ private predicate taintPreservingArgumentToMethod(Method method, int arg) { method.hasName("sourceToInputSource") and arg = 0 or -<<<<<<< HEAD method.(TaintPreservingCallable).returnsTaintFrom(arg) -======= - 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.(GuavaTaintPropagationMethod).propagatesTaint(arg, -2) ->>>>>>> 61c00e344... Java: Add modelling for Guava `Strings`, `Splitter`, and `Joiner` } /** @@ -557,17 +525,7 @@ private predicate taintPreservingArgToArg(Method method, int input, int output) input = 0 and output = 2 or -<<<<<<< HEAD method.(TaintPreservingCallable).transfersTaint(input, output) -======= - // 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.(GuavaTaintPropagationMethod).propagatesTaint(input, output) ->>>>>>> 61c00e344... Java: Add modelling for Guava `Strings`, `Splitter`, and `Joiner` } /** @@ -595,18 +553,7 @@ private predicate taintPreservingArgumentToQualifier(Method method, int arg) { write.getDeclaringType().hasQualifiedName("java.io", "OutputStream") ) or -<<<<<<< HEAD method.(TaintPreservingCallable).transfersTaint(arg, -1) -======= - exists(Method append | - method.overrides*(append) and - append.hasName("append") and - arg = 0 and - append.getDeclaringType().hasQualifiedName("java.io", "StringWriter") - ) - or - method.(GuavaTaintPropagationMethod).propagatesTaint(arg, -1) ->>>>>>> 61c00e344... Java: Add modelling for Guava `Strings`, `Splitter`, and `Joiner` } /** A comparison or equality test with a constant. */ diff --git a/java/ql/src/semmle/code/java/frameworks/guava/Guava.qll b/java/ql/src/semmle/code/java/frameworks/guava/Guava.qll index 65bc9e929ef..a4fd500cd87 100644 --- a/java/ql/src/semmle/code/java/frameworks/guava/Guava.qll +++ b/java/ql/src/semmle/code/java/frameworks/guava/Guava.qll @@ -6,29 +6,3 @@ import java private import Strings private import Splitter private import Joiner - -/** - * A method in the guava framework that propegates taint. - */ -abstract class GuavaTaintPropagationMethod extends Method { - /** - * Holds if this method propagates taint between the given source and sink. - * `src` and `sink` are indicies of arguments to this method, or -1 to represent the qualifier. - * `sink` can also be -2 to represent the return value. - */ - abstract predicate propagatesTaint(int src, int sink); -} - -/** - * A method in the guava framework that returns tainted data when a specific input - * (either an argument or the qualifier) is tainted. - */ -abstract class GuavaTaintPropagationMethodToReturn extends GuavaTaintPropagationMethod { - /** - * Holds if this method returns tainted data when the given source is tainted. - * `src` is an argument index, or -1 to indicate the qualifier. - */ - abstract predicate propagatesTaint(int src); - - override predicate propagatesTaint(int src, int sink) { propagatesTaint(src) and sink = -2 } -} diff --git a/java/ql/src/semmle/code/java/frameworks/guava/Joiner.qll b/java/ql/src/semmle/code/java/frameworks/guava/Joiner.qll index 7b9f7cb10b1..cb1ccef2d16 100644 --- a/java/ql/src/semmle/code/java/frameworks/guava/Joiner.qll +++ b/java/ql/src/semmle/code/java/frameworks/guava/Joiner.qll @@ -3,7 +3,7 @@ */ import java -import Guava +import semmle.code.java.dataflow.FlowSteps /** * The class `com.google.common.base.Joiner`. @@ -35,8 +35,7 @@ private class GuavaJoinerMethod extends Method { /** * A method that builds a `Joiner` or `MapJoiner`. */ -private class GuavaJoinerBuilderMethod extends GuavaJoinerMethod, - GuavaTaintPropagationMethodToReturn { +private class GuavaJoinerBuilderMethod extends GuavaJoinerMethod, TaintPreservingCallable { GuavaJoinerBuilderMethod() { // static Joiner on(char separator) // static Joiner on(String separator) @@ -47,13 +46,17 @@ private class GuavaJoinerBuilderMethod extends GuavaJoinerMethod, this.hasName(["on", "skipNulls", "useForNull", "withKeyValueSeparator"]) } - override predicate propagatesTaint(int src) { src = [-1, 0] } + override predicate returnsTaintFrom(int src) { + src = 0 + or + src = -1 and not isStatic() + } } /** * An `appendTo` method on `Joiner` or `MapJoiner` */ -private class GuavaJoinerAppendToMethod extends GuavaJoinerMethod, GuavaTaintPropagationMethod { +private class GuavaJoinerAppendToMethod extends GuavaJoinerMethod, TaintPreservingCallable { GuavaJoinerAppendToMethod() { // A appendTo(A appendable, Iterable parts) // A appendTo(A appendable, Iterator parts) @@ -72,17 +75,19 @@ private class GuavaJoinerAppendToMethod extends GuavaJoinerMethod, GuavaTaintPro this.hasName("appendTo") } - override predicate propagatesTaint(int src, int sink) { + override predicate transfersTaint(int src, int sink) { src = [-1 .. getNumberOfParameters()] and src != sink and - sink = [-2, 0] + sink = 0 } + + override predicate returnsTaintFrom(int src) { src = [-1 .. getNumberOfParameters()] } } /** * A `join` method on `Joiner` or `MapJoiner` */ -private class GuavaJoinMethod extends GuavaJoinerMethod, GuavaTaintPropagationMethodToReturn { +private class GuavaJoinMethod extends GuavaJoinerMethod, TaintPreservingCallable { GuavaJoinMethod() { // String join(Iterable parts) // String join(Iterator parts) @@ -94,5 +99,5 @@ private class GuavaJoinMethod extends GuavaJoinerMethod, GuavaTaintPropagationMe this.hasName("join") } - override predicate propagatesTaint(int src) { src = [-1 .. getNumberOfParameters()] } + override predicate returnsTaintFrom(int src) { src = [-1 .. getNumberOfParameters()] } } diff --git a/java/ql/src/semmle/code/java/frameworks/guava/Splitter.qll b/java/ql/src/semmle/code/java/frameworks/guava/Splitter.qll index 1682e81a525..a6d58d35698 100644 --- a/java/ql/src/semmle/code/java/frameworks/guava/Splitter.qll +++ b/java/ql/src/semmle/code/java/frameworks/guava/Splitter.qll @@ -3,7 +3,7 @@ */ import java -import Guava +import semmle.code.java.dataflow.FlowSteps /** * The class `com.google.common.base.Splitter`. @@ -25,7 +25,7 @@ class TypeGuavaMapSplitter extends NestedClass { /** * A method of `Splitter` or `MapSplitter` that splits its input string. */ -private class GuavaSplitMethod extends GuavaTaintPropagationMethodToReturn { +private class GuavaSplitMethod extends TaintPreservingCallable { GuavaSplitMethod() { ( this.getDeclaringType() instanceof TypeGuavaSplitter @@ -39,5 +39,5 @@ private class GuavaSplitMethod extends GuavaTaintPropagationMethodToReturn { this.hasName(["split", "splitToList", "splitToStream"]) } - override predicate propagatesTaint(int src) { src = 0 } + override predicate returnsTaintFrom(int src) { src = 0 } } diff --git a/java/ql/src/semmle/code/java/frameworks/guava/Strings.qll b/java/ql/src/semmle/code/java/frameworks/guava/Strings.qll index b996642bf83..f2df0984899 100644 --- a/java/ql/src/semmle/code/java/frameworks/guava/Strings.qll +++ b/java/ql/src/semmle/code/java/frameworks/guava/Strings.qll @@ -3,7 +3,7 @@ */ import java -import Guava +import semmle.code.java.dataflow.FlowSteps /** * The class `com.google.common.base.Strings`. @@ -13,31 +13,24 @@ class TypeGuavaStrings extends Class { } /** - * A Guava string utility method that preserves taint from its first argument. + * A Guava string utility method that preserves taint. */ -private class GuavaStringsTaintPropagationMethod extends GuavaTaintPropagationMethodToReturn { - GuavaStringsTaintPropagationMethod() { +private class GuavaStringsTaintPreservingMethod extends TaintPreservingCallable { + GuavaStringsTaintPreservingMethod() { this.getDeclaringType() instanceof TypeGuavaStrings and // static String emptyToNull(String string) // static String emptyToNull(String string) // static String padEnd(String string, int minLength, char padChar) // static String padStart(String string, int minLength, char padChar) // static String repeat(String string, int count) - this.hasName(["emptyToNull", "nullToEmpty", "padStart", "padEnd", "repeat"]) - } - - override predicate propagatesTaint(int src) { src = 0 } -} - -/** - * The method `Strings.lenientFormat`. - */ -private class GuavaStringsFormatMethod extends GuavaTaintPropagationMethodToReturn { - GuavaStringsFormatMethod() { - this.getDeclaringType() instanceof TypeGuavaStrings and // static String lenientFormat(String template, Object ... args) - this.hasName("lenientFormat") + this.hasName(["emptyToNull", "nullToEmpty", "padStart", "padEnd", "repeat", "lenientFormat"]) } - override predicate propagatesTaint(int src) { src in [0 .. getNumberOfParameters()] } + override predicate returnsTaintFrom(int src) { + src = 0 + or + this.hasName("lenientFormat") and + src = [0 .. getNumberOfParameters()] + } }