From 9163893879c280c079d384fa9e89bd1da7e7548b Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Fri, 5 Mar 2021 14:39:16 +0000 Subject: [PATCH 1/4] Add models for Commons-Lang's RegExUtils class --- .../code/java/frameworks/apache/Lang.qll | 20 ++++++ .../apache-commons-lang3/RegExUtilsTest.java | 45 ++++++++++++++ .../org/apache/commons/lang3/RegExUtils.java | 62 +++++++++++++++++++ 3 files changed, 127 insertions(+) create mode 100644 java/ql/test/library-tests/frameworks/apache-commons-lang3/RegExUtilsTest.java create mode 100644 java/ql/test/stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/RegExUtils.java 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 26b52b2716b9..b2fb782899ca 100644 --- a/java/ql/src/semmle/code/java/frameworks/apache/Lang.qll +++ b/java/ql/src/semmle/code/java/frameworks/apache/Lang.qll @@ -376,3 +376,23 @@ private class ApacheStrSubstitutorModel extends SummaryModelCsv { ] } } + +/** + * Taint-propagating models for `RegexUtils`. + */ +private class ApacheRegExUtilsModel extends SummaryModelCsv { + override predicate row(string row) { + row = + [ + "org.apache.commons.lang3;RegExUtils;false;removeAll;;;Argument[0];ReturnValue;taint", + "org.apache.commons.lang3;RegExUtils;false;removeFirst;;;Argument[0];ReturnValue;taint", + "org.apache.commons.lang3;RegExUtils;false;removePattern;;;Argument[0];ReturnValue;taint", + "org.apache.commons.lang3;RegExUtils;false;replaceAll;;;Argument[0];ReturnValue;taint", + "org.apache.commons.lang3;RegExUtils;false;replaceFirst;;;Argument[0];ReturnValue;taint", + "org.apache.commons.lang3;RegExUtils;false;replacePattern;;;Argument[0];ReturnValue;taint", + "org.apache.commons.lang3;RegExUtils;false;replaceAll;;;Argument[2];ReturnValue;taint", + "org.apache.commons.lang3;RegExUtils;false;replaceFirst;;;Argument[2];ReturnValue;taint", + "org.apache.commons.lang3;RegExUtils;false;replacePattern;;;Argument[2];ReturnValue;taint" + ] + } +} diff --git a/java/ql/test/library-tests/frameworks/apache-commons-lang3/RegExUtilsTest.java b/java/ql/test/library-tests/frameworks/apache-commons-lang3/RegExUtilsTest.java new file mode 100644 index 000000000000..ffef863f5b5b --- /dev/null +++ b/java/ql/test/library-tests/frameworks/apache-commons-lang3/RegExUtilsTest.java @@ -0,0 +1,45 @@ +import org.apache.commons.lang3.RegExUtils; +import java.util.regex.Pattern; + +public class RegExUtilsTest { + String taint() { return "tainted"; } + + void sink(Object o) {} + + void test() throws Exception { + Pattern cleanPattern = Pattern.compile("clean"); + Pattern taintedPattern = Pattern.compile(taint()); + + sink(RegExUtils.removeAll(taint(), cleanPattern)); // $hasTaintFlow=y + sink(RegExUtils.removeAll(taint(), "clean")); // $hasTaintFlow=y + sink(RegExUtils.removeFirst(taint(), cleanPattern)); // $hasTaintFlow=y + sink(RegExUtils.removeFirst(taint(), "clean")); // $hasTaintFlow=y + sink(RegExUtils.removePattern(taint(), "clean")); // $hasTaintFlow=y + sink(RegExUtils.replaceAll(taint(), cleanPattern, "replacement")); // $hasTaintFlow=y + sink(RegExUtils.replaceAll(taint(), "clean", "replacement")); // $hasTaintFlow=y + sink(RegExUtils.replaceFirst(taint(), cleanPattern, "replacement")); // $hasTaintFlow=y + sink(RegExUtils.replaceFirst(taint(), "clean", "replacement")); // $hasTaintFlow=y + sink(RegExUtils.replacePattern(taint(), "clean", "replacement")); // $hasTaintFlow=y + sink(RegExUtils.replaceAll("original", cleanPattern, taint())); // $hasTaintFlow=y + sink(RegExUtils.replaceAll("original", "clean", taint())); // $hasTaintFlow=y + sink(RegExUtils.replaceFirst("original", cleanPattern, taint())); // $hasTaintFlow=y + sink(RegExUtils.replaceFirst("original", "clean", taint())); // $hasTaintFlow=y + sink(RegExUtils.replacePattern("original", "clean", taint())); // $hasTaintFlow=y + // Subsequent calls don't propagate taint, as regex search patterns don't propagate to the return value. + sink(RegExUtils.removeAll("original", taintedPattern)); + sink(RegExUtils.removeAll("original", taint())); + sink(RegExUtils.removeFirst("original", taintedPattern)); + sink(RegExUtils.removeFirst("original", taint())); + sink(RegExUtils.removePattern("original", taint())); + sink(RegExUtils.replaceAll("original", taintedPattern, "replacement")); + sink(RegExUtils.replaceAll("original", taint(), "replacement")); + sink(RegExUtils.replaceFirst("original", taintedPattern, "replacement")); + sink(RegExUtils.replaceFirst("original", taint(), "replacement")); + sink(RegExUtils.replacePattern("original", taint(), "replacement")); + sink(RegExUtils.replaceAll("original", taintedPattern, "replacement")); + sink(RegExUtils.replaceAll("original", taint(), "replacement")); + sink(RegExUtils.replaceFirst("original", taintedPattern, "replacement")); + sink(RegExUtils.replaceFirst("original", taint(), "replacement")); + sink(RegExUtils.replacePattern("original", taint(), "replacement")); + } +} \ No newline at end of file diff --git a/java/ql/test/stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/RegExUtils.java b/java/ql/test/stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/RegExUtils.java new file mode 100644 index 000000000000..adf128229b67 --- /dev/null +++ b/java/ql/test/stubs/apache-commons-lang3-3.7/org/apache/commons/lang3/RegExUtils.java @@ -0,0 +1,62 @@ +/* + * 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.util.regex.Pattern; + +public class RegExUtils { + public static String removeAll(final String text, final Pattern regex) { + return null; + } + + public static String removeAll(final String text, final String regex) { + return null; + } + + public static String removeFirst(final String text, final Pattern regex) { + return null; + } + + public static String removeFirst(final String text, final String regex) { + return null; + } + + public static String removePattern(final String text, final String regex) { + return null; + } + + public static String replaceAll(final String text, final Pattern regex, final String replacement) { + return null; + } + + public static String replaceAll(final String text, final String regex, final String replacement) { + return null; + } + + public static String replaceFirst(final String text, final Pattern regex, final String replacement) { + return null; + } + + public static String replaceFirst(final String text, final String regex, final String replacement) { + return null; + } + + public static String replacePattern(final String text, final String regex, final String replacement) { + return null; + } + +} From 074d73e32510b6a02428f69a8cccf820dc5ca4bb Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Fri, 5 Mar 2021 14:43:22 +0000 Subject: [PATCH 2/4] Add change note --- java/change-notes/2021-03-05-regex-utils.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 java/change-notes/2021-03-05-regex-utils.md diff --git a/java/change-notes/2021-03-05-regex-utils.md b/java/change-notes/2021-03-05-regex-utils.md new file mode 100644 index 000000000000..d2a73a2d3e26 --- /dev/null +++ b/java/change-notes/2021-03-05-regex-utils.md @@ -0,0 +1,2 @@ +lgtm,codescanning +* Added models for Apache Commons-Lang's `RegExUtils` class. This means that any query that tracks tainted data may return additional results in cases where a `RegExUtils` transformation is part of the path from source to sink. From e8f81c4f305328bb0877ab088033e5d32c27f131 Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Mon, 8 Mar 2021 11:30:48 +0000 Subject: [PATCH 3/4] Improve change note --- java/change-notes/2021-03-05-regex-utils.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/java/change-notes/2021-03-05-regex-utils.md b/java/change-notes/2021-03-05-regex-utils.md index d2a73a2d3e26..666aa73f29e5 100644 --- a/java/change-notes/2021-03-05-regex-utils.md +++ b/java/change-notes/2021-03-05-regex-utils.md @@ -1,2 +1,2 @@ lgtm,codescanning -* Added models for Apache Commons-Lang's `RegExUtils` class. This means that any query that tracks tainted data may return additional results in cases where a `RegExUtils` transformation is part of the path from source to sink. +* Added models for Apache Commons Lang's `RegExUtils` class. This means that any query that tracks tainted data may return additional results in cases where a `RegExUtils` transformation is part of the path from source to sink. From 189b2215c52f85472863e83a8a92d0fd095ea71d Mon Sep 17 00:00:00 2001 From: Chris Smowton Date: Tue, 9 Mar 2021 15:11:39 +0000 Subject: [PATCH 4/4] Remove useless value from inline test expectations --- .../apache-commons-lang3/RegExUtilsTest.java | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/java/ql/test/library-tests/frameworks/apache-commons-lang3/RegExUtilsTest.java b/java/ql/test/library-tests/frameworks/apache-commons-lang3/RegExUtilsTest.java index ffef863f5b5b..59db473d886c 100644 --- a/java/ql/test/library-tests/frameworks/apache-commons-lang3/RegExUtilsTest.java +++ b/java/ql/test/library-tests/frameworks/apache-commons-lang3/RegExUtilsTest.java @@ -10,21 +10,21 @@ void test() throws Exception { Pattern cleanPattern = Pattern.compile("clean"); Pattern taintedPattern = Pattern.compile(taint()); - sink(RegExUtils.removeAll(taint(), cleanPattern)); // $hasTaintFlow=y - sink(RegExUtils.removeAll(taint(), "clean")); // $hasTaintFlow=y - sink(RegExUtils.removeFirst(taint(), cleanPattern)); // $hasTaintFlow=y - sink(RegExUtils.removeFirst(taint(), "clean")); // $hasTaintFlow=y - sink(RegExUtils.removePattern(taint(), "clean")); // $hasTaintFlow=y - sink(RegExUtils.replaceAll(taint(), cleanPattern, "replacement")); // $hasTaintFlow=y - sink(RegExUtils.replaceAll(taint(), "clean", "replacement")); // $hasTaintFlow=y - sink(RegExUtils.replaceFirst(taint(), cleanPattern, "replacement")); // $hasTaintFlow=y - sink(RegExUtils.replaceFirst(taint(), "clean", "replacement")); // $hasTaintFlow=y - sink(RegExUtils.replacePattern(taint(), "clean", "replacement")); // $hasTaintFlow=y - sink(RegExUtils.replaceAll("original", cleanPattern, taint())); // $hasTaintFlow=y - sink(RegExUtils.replaceAll("original", "clean", taint())); // $hasTaintFlow=y - sink(RegExUtils.replaceFirst("original", cleanPattern, taint())); // $hasTaintFlow=y - sink(RegExUtils.replaceFirst("original", "clean", taint())); // $hasTaintFlow=y - sink(RegExUtils.replacePattern("original", "clean", taint())); // $hasTaintFlow=y + sink(RegExUtils.removeAll(taint(), cleanPattern)); // $hasTaintFlow + sink(RegExUtils.removeAll(taint(), "clean")); // $hasTaintFlow + sink(RegExUtils.removeFirst(taint(), cleanPattern)); // $hasTaintFlow + sink(RegExUtils.removeFirst(taint(), "clean")); // $hasTaintFlow + sink(RegExUtils.removePattern(taint(), "clean")); // $hasTaintFlow + sink(RegExUtils.replaceAll(taint(), cleanPattern, "replacement")); // $hasTaintFlow + sink(RegExUtils.replaceAll(taint(), "clean", "replacement")); // $hasTaintFlow + sink(RegExUtils.replaceFirst(taint(), cleanPattern, "replacement")); // $hasTaintFlow + sink(RegExUtils.replaceFirst(taint(), "clean", "replacement")); // $hasTaintFlow + sink(RegExUtils.replacePattern(taint(), "clean", "replacement")); // $hasTaintFlow + sink(RegExUtils.replaceAll("original", cleanPattern, taint())); // $hasTaintFlow + sink(RegExUtils.replaceAll("original", "clean", taint())); // $hasTaintFlow + sink(RegExUtils.replaceFirst("original", cleanPattern, taint())); // $hasTaintFlow + sink(RegExUtils.replaceFirst("original", "clean", taint())); // $hasTaintFlow + sink(RegExUtils.replacePattern("original", "clean", taint())); // $hasTaintFlow // Subsequent calls don't propagate taint, as regex search patterns don't propagate to the return value. sink(RegExUtils.removeAll("original", taintedPattern)); sink(RegExUtils.removeAll("original", taint()));