diff --git a/java/change-notes/2021-03-05-commons-object-utils.md b/java/change-notes/2021-03-05-commons-object-utils.md new file mode 100644 index 000000000000..c219fb8f5bde --- /dev/null +++ b/java/change-notes/2021-03-05-commons-object-utils.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* Added models for `ObjectUtils` methods in the Apache Commons Lang library. This may lead to more results from any dataflow query where traversal of `ObjectUtils` methods means we can now complete a path from a source of tainted data to a corresponding sink. diff --git a/java/ql/src/semmle/code/java/frameworks/apache/Lang.qll b/java/ql/src/semmle/code/java/frameworks/apache/Lang.qll index b2fb782899ca..0b64361c0183 100644 --- a/java/ql/src/semmle/code/java/frameworks/apache/Lang.qll +++ b/java/ql/src/semmle/code/java/frameworks/apache/Lang.qll @@ -396,3 +396,30 @@ private class ApacheRegExUtilsModel extends SummaryModelCsv { ] } } + +/** + * Taint-propagating models for `ObjectUtils`. + */ +private class ApacheObjectUtilsModel extends SummaryModelCsv { + override predicate row(string row) { + row = + [ + // Note all the functions annotated with `taint` flow really should have `value` flow, + // but we don't support value-preserving varargs functions at the moment. + "org.apache.commons.lang3;ObjectUtils;false;clone;;;Argument;ReturnValue;value", + "org.apache.commons.lang3;ObjectUtils;false;cloneIfPossible;;;Argument;ReturnValue;value", + "org.apache.commons.lang3;ObjectUtils;false;CONST;;;Argument;ReturnValue;value", + "org.apache.commons.lang3;ObjectUtils;false;CONST_BYTE;;;Argument;ReturnValue;value", + "org.apache.commons.lang3;ObjectUtils;false;CONST_SHORT;;;Argument;ReturnValue;value", + "org.apache.commons.lang3;ObjectUtils;false;defaultIfNull;;;Argument;ReturnValue;value", + "org.apache.commons.lang3;ObjectUtils;false;firstNonNull;;;Argument;ReturnValue;taint", + "org.apache.commons.lang3;ObjectUtils;false;getIfNull;;;Argument[0];ReturnValue;value", + "org.apache.commons.lang3;ObjectUtils;false;max;;;Argument;ReturnValue;taint", + "org.apache.commons.lang3;ObjectUtils;false;median;;;Argument;ReturnValue;taint", + "org.apache.commons.lang3;ObjectUtils;false;min;;;Argument;ReturnValue;taint", + "org.apache.commons.lang3;ObjectUtils;false;mode;;;Argument;ReturnValue;taint", + "org.apache.commons.lang3;ObjectUtils;false;requireNonEmpty;;;Argument[0];ReturnValue;value", + "org.apache.commons.lang3;ObjectUtils;false;toString;(Object,String);;Argument[1];ReturnValue;value" + ] + } +} diff --git a/java/ql/test/library-tests/frameworks/apache-commons-lang3/ObjectUtilsTest.java b/java/ql/test/library-tests/frameworks/apache-commons-lang3/ObjectUtilsTest.java new file mode 100644 index 000000000000..d586b887290c --- /dev/null +++ b/java/ql/test/library-tests/frameworks/apache-commons-lang3/ObjectUtilsTest.java @@ -0,0 +1,41 @@ +import org.apache.commons.lang3.ObjectUtils; + +public class ObjectUtilsTest { + String taint() { return "tainted"; } + + private static class IntSource { + static int taint() { return 0; } + } + + void sink(Object o) {} + + void test() throws Exception { + sink(ObjectUtils.clone(taint())); // $hasValueFlow + sink(ObjectUtils.cloneIfPossible(taint())); // $hasValueFlow + sink(ObjectUtils.CONST(taint())); // $hasValueFlow + sink(ObjectUtils.CONST_SHORT(IntSource.taint())); // $hasValueFlow + sink(ObjectUtils.CONST_BYTE(IntSource.taint())); // $hasValueFlow + sink(ObjectUtils.defaultIfNull(taint(), null)); // $hasValueFlow + sink(ObjectUtils.defaultIfNull(null, taint())); // $hasValueFlow + sink(ObjectUtils.firstNonNull(taint(), null, null)); // $hasTaintFlow $MISSING:hasValueFlow + sink(ObjectUtils.firstNonNull(null, taint(), null)); // $hasTaintFlow $MISSING:hasValueFlow + sink(ObjectUtils.firstNonNull(null, null, taint())); // $hasTaintFlow $MISSING:hasValueFlow + sink(ObjectUtils.getIfNull(taint(), null)); // $hasValueFlow + sink(ObjectUtils.max(taint(), null, null)); // $hasTaintFlow $MISSING:hasValueFlow + sink(ObjectUtils.max(null, taint(), null)); // $hasTaintFlow $MISSING:hasValueFlow + sink(ObjectUtils.max(null, null, taint())); // $hasTaintFlow $MISSING:hasValueFlow + sink(ObjectUtils.median(taint(), null, null)); // $hasTaintFlow $MISSING:hasValueFlow + sink(ObjectUtils.median((String)null, taint(), null)); // $hasTaintFlow $MISSING:hasValueFlow + sink(ObjectUtils.median((String)null, null, taint())); // $hasTaintFlow $MISSING:hasValueFlow + sink(ObjectUtils.min(taint(), null, null)); // $hasTaintFlow $MISSING:hasValueFlow + sink(ObjectUtils.min(null, taint(), null)); // $hasTaintFlow $MISSING:hasValueFlow + sink(ObjectUtils.min(null, null, taint())); // $hasTaintFlow $MISSING:hasValueFlow + sink(ObjectUtils.mode(taint(), null, null)); // $hasTaintFlow $MISSING:hasValueFlow + sink(ObjectUtils.mode(null, taint(), null)); // $hasTaintFlow $MISSING:hasValueFlow + sink(ObjectUtils.mode(null, null, taint())); // $hasTaintFlow $MISSING:hasValueFlow + sink(ObjectUtils.requireNonEmpty(taint(), "message")); // $hasValueFlow + sink(ObjectUtils.requireNonEmpty("not null", taint())); // GOOD (message doesn't propagate to the return) + sink(ObjectUtils.toString(taint(), "default string")); // GOOD (first argument is stringified) + sink(ObjectUtils.toString(null, taint())); // $hasValueFlow + } +} diff --git a/java/ql/test/library-tests/frameworks/apache-commons-lang3/flow.ql b/java/ql/test/library-tests/frameworks/apache-commons-lang3/flow.ql index be46920c0882..b86d7b2611bc 100644 --- a/java/ql/test/library-tests/frameworks/apache-commons-lang3/flow.ql +++ b/java/ql/test/library-tests/frameworks/apache-commons-lang3/flow.ql @@ -2,8 +2,20 @@ import java import semmle.code.java.dataflow.TaintTracking import TestUtilities.InlineExpectationsTest -class Conf extends TaintTracking::Configuration { - Conf() { this = "qltest:frameworks:apache-commons-lang3" } +class TaintFlowConf extends TaintTracking::Configuration { + TaintFlowConf() { this = "qltest:frameworks:apache-commons-lang3-taint-flow" } + + override predicate isSource(DataFlow::Node n) { + n.asExpr().(MethodAccess).getMethod().hasName("taint") + } + + override predicate isSink(DataFlow::Node n) { + exists(MethodAccess ma | ma.getMethod().hasName("sink") | n.asExpr() = ma.getAnArgument()) + } +} + +class ValueFlowConf extends DataFlow::Configuration { + ValueFlowConf() { this = "qltest:frameworks:apache-commons-lang3-value-flow" } override predicate isSource(DataFlow::Node n) { n.asExpr().(MethodAccess).getMethod().hasName("taint") @@ -17,11 +29,19 @@ class Conf extends TaintTracking::Configuration { class HasFlowTest extends InlineExpectationsTest { HasFlowTest() { this = "HasFlowTest" } - override string getARelevantTag() { result = "hasTaintFlow" } + override string getARelevantTag() { result = ["hasTaintFlow", "hasValueFlow"] } override predicate hasActualResult(Location location, string element, string tag, string value) { tag = "hasTaintFlow" and - exists(DataFlow::Node src, DataFlow::Node sink, Conf conf | conf.hasFlow(src, sink) | + exists(DataFlow::Node src, DataFlow::Node sink, TaintFlowConf conf | conf.hasFlow(src, sink) | + not any(ValueFlowConf vconf).hasFlow(src, sink) and + sink.getLocation() = location and + element = sink.toString() and + value = "" + ) + or + tag = "hasValueFlow" and + exists(DataFlow::Node src, DataFlow::Node sink, ValueFlowConf conf | conf.hasFlow(src, sink) | sink.getLocation() = location and element = sink.toString() and value = "" diff --git a/java/ql/test/stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/ObjectUtils.java b/java/ql/test/stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/ObjectUtils.java new file mode 100644 index 000000000000..fdd2281cfc61 --- /dev/null +++ b/java/ql/test/stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/ObjectUtils.java @@ -0,0 +1,221 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.commons.lang3; + +import java.io.IOException; +import java.io.Serializable; +import java.lang.reflect.Array; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.time.Duration; +import java.util.Collection; +import java.util.Collections; +import java.util.Comparator; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; +import java.util.TreeSet; +import java.util.function.Supplier; + +import org.apache.commons.lang3.text.StrBuilder; + +@SuppressWarnings("deprecation") // deprecated class StrBuilder is imported +// because it is part of the signature of deprecated methods +public class ObjectUtils { + public static class Null implements Serializable { + } + public static final Null NULL = new Null(); + + public static boolean allNotNull(final Object... values) { + return false; + } + + public static boolean allNull(final Object... values) { + return false; + } + + public static boolean anyNotNull(final Object... values) { + return false; + } + + public static boolean anyNull(final Object... values) { + return false; + } + + public static T clone(final T obj) { + return null; + } + + public static T cloneIfPossible(final T obj) { + return null; + } + + public static > int compare(final T c1, final T c2) { + return 0; + } + + public static > int compare(final T c1, final T c2, final boolean nullGreater) { + return 0; + } + + public static boolean CONST(final boolean v) { + return false; + } + + public static byte CONST(final byte v) { + return 0; + } + + public static char CONST(final char v) { + return '\0'; + } + + public static double CONST(final double v) { + return 0; + } + + public static float CONST(final float v) { + return 0; + } + + public static int CONST(final int v) { + return 0; + } + + public static long CONST(final long v) { + return 0; + } + + public static short CONST(final short v) { + return 0; + } + + public static T CONST(final T v) { + return null; + } + + public static byte CONST_BYTE(final int v) { + return 0; + } + + public static short CONST_SHORT(final int v) { + return 0; + } + + public static T defaultIfNull(final T object, final T defaultValue) { + return null; + } + + public static boolean equals(final Object object1, final Object object2) { + return false; + } + + public static T firstNonNull(final T... values) { + return null; + } + + public static T getFirstNonNull(final Supplier... suppliers) { + return null; + } + + public static T getIfNull(final T object, final Supplier defaultSupplier) { + return null; + } + + public static int hashCode(final Object obj) { + return 0; + } + + public static int hashCodeMulti(final Object... objects) { + return 0; + } + + public static void identityToString(final Appendable appendable, final Object object) throws IOException { + } + + public static String identityToString(final Object object) { + return null; + } + + public static void identityToString(final StrBuilder builder, final Object object) { + } + + public static void identityToString(final StringBuffer buffer, final Object object) { + } + + public static void identityToString(final StringBuilder builder, final Object object) { + } + + public static boolean isEmpty(final Object object) { + return false; + } + + public static boolean isNotEmpty(final Object object) { + return false; + } + + public static > T max(final T... values) { + return null; + } + + public static T median(final Comparator comparator, final T... items) { + return null; + } + + public static > T median(final T... items) { + return null; + } + + public static > T min(final T... values) { + return null; + } + + public static T mode(final T... items) { + return null; + } + + public static boolean notEqual(final Object object1, final Object object2) { + return false; + } + + public static T requireNonEmpty(final T obj) { + return null; + } + + public static T requireNonEmpty(final T obj, final String message) { + return null; + } + + public static String toString(final Object obj) { + return null; + } + + public static String toString(final Object obj, final String nullStr) { + return null; + } + + public static String toString(final Object obj, final Supplier supplier) { + return null; + } + + public static void wait(final Object obj, final Duration duration) throws InterruptedException { + } + + public ObjectUtils() { + } + +}