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 142e1c2d4f2..f635a4463b4 100644 --- a/java/ql/src/semmle/code/java/frameworks/apache/Lang.qll +++ b/java/ql/src/semmle/code/java/frameworks/apache/Lang.qll @@ -123,8 +123,8 @@ private class ApacheStringUtilsTaintPreservingMethod extends TaintPreservingCall * A method declared on Apache Commons Lang's `StrBuilder`, or the same class or its * renamed version `TextStringBuilder` in Commons Text. */ -class ApacheStrBuilderMethod extends Method { - ApacheStrBuilderMethod() { +class ApacheStrBuilderCallable extends Callable { + ApacheStrBuilderCallable() { this.getDeclaringType().hasQualifiedName("org.apache.commons.lang3.text", "StrBuilder") or this.getDeclaringType() .hasQualifiedName("org.apache.commons.text", ["StrBuilder", "TextStringBuilder"]) @@ -134,8 +134,11 @@ class ApacheStrBuilderMethod extends Method { /** * An Apache Commons Lang StrBuilder method that adds taint to the StrBuilder. */ -private class ApacheStrBuilderTaintingMethod extends ApacheStrBuilderMethod, TaintPreservingCallable { +private class ApacheStrBuilderTaintingMethod extends ApacheStrBuilderCallable, + TaintPreservingCallable { ApacheStrBuilderTaintingMethod() { + this instanceof Constructor + or this.hasName([ "append", "appendAll", "appendFixedWidthPadLeft", "appendFixedWidthPadRight", "appendln", "appendSeparator", "appendWithSeparators", "insert", "readFrom", "replace", "replaceAll", @@ -170,12 +173,14 @@ private class ApacheStrBuilderTaintingMethod extends ApacheStrBuilderMethod, Tai this.consumesTaintFromAllArgs() and fromArg in [0 .. this.getNumberOfParameters() - 1] ) } + + override predicate returnsTaintFrom(int arg) { this instanceof Constructor and arg = 0 } } /** * An Apache Commons Lang StrBuilder method that returns taint from the StrBuilder. */ -private class ApacheStrBuilderTaintGetter extends ApacheStrBuilderMethod, TaintPreservingCallable { +private class ApacheStrBuilderTaintGetter extends ApacheStrBuilderCallable, TaintPreservingCallable { ApacheStrBuilderTaintGetter() { // Taint getters: this.hasName([ @@ -193,7 +198,7 @@ private class ApacheStrBuilderTaintGetter extends ApacheStrBuilderMethod, TaintP /** * An Apache Commons Lang StrBuilder method that writes taint from the StrBuilder to some parameter. */ -private class ApacheStrBuilderTaintWriter extends ApacheStrBuilderMethod, TaintPreservingCallable { +private class ApacheStrBuilderTaintWriter extends ApacheStrBuilderCallable, TaintPreservingCallable { ApacheStrBuilderTaintWriter() { this.hasName(["appendTo", "getChars"]) } override predicate transfersTaint(int fromArg, int toArg) { diff --git a/java/ql/test/library-tests/frameworks/apache-commons-lang3/StrBuilderTest.java b/java/ql/test/library-tests/frameworks/apache-commons-lang3/StrBuilderTest.java index 27d0fa9f636..c24fe65b4c7 100644 --- a/java/ql/test/library-tests/frameworks/apache-commons-lang3/StrBuilderTest.java +++ b/java/ql/test/library-tests/frameworks/apache-commons-lang3/StrBuilderTest.java @@ -14,6 +14,8 @@ class StrBuilderTest { void test() throws Exception { + StrBuilder cons1 = new StrBuilder(taint()); sink(cons1.toString()); // $hasTaintFlow=y + StrBuilder sb1 = new StrBuilder(); sb1.append(taint().toCharArray()); sink(sb1.toString()); // $hasTaintFlow=y StrBuilder sb2 = new StrBuilder(); sb2.append(taint().toCharArray(), 0, 0); sink(sb2.toString()); // $hasTaintFlow=y StrBuilder sb3 = new StrBuilder(); sb3.append(CharBuffer.wrap(taint().toCharArray())); sink(sb3.toString()); // BAD (but not detected because we don't model CharBuffer yet) diff --git a/java/ql/test/library-tests/frameworks/apache-commons-lang3/StrBuilderTextTest.java b/java/ql/test/library-tests/frameworks/apache-commons-lang3/StrBuilderTextTest.java index 0177cf8d4cd..e9e902552a0 100644 --- a/java/ql/test/library-tests/frameworks/apache-commons-lang3/StrBuilderTextTest.java +++ b/java/ql/test/library-tests/frameworks/apache-commons-lang3/StrBuilderTextTest.java @@ -14,6 +14,8 @@ class StrBuilderTextTest { void test() throws Exception { + StrBuilder cons1 = new StrBuilder(taint()); sink(cons1.toString()); // $hasTaintFlow=y + StrBuilder sb1 = new StrBuilder(); sb1.append(taint().toCharArray()); sink(sb1.toString()); // $hasTaintFlow=y StrBuilder sb2 = new StrBuilder(); sb2.append(taint().toCharArray(), 0, 0); sink(sb2.toString()); // $hasTaintFlow=y StrBuilder sb3 = new StrBuilder(); sb3.append(CharBuffer.wrap(taint().toCharArray())); sink(sb3.toString()); // BAD (but not detected because we don't model CharBuffer yet) diff --git a/java/ql/test/library-tests/frameworks/apache-commons-lang3/TextStringBuilderTest.java b/java/ql/test/library-tests/frameworks/apache-commons-lang3/TextStringBuilderTest.java index e98cf5d8191..ebedb2283be 100644 --- a/java/ql/test/library-tests/frameworks/apache-commons-lang3/TextStringBuilderTest.java +++ b/java/ql/test/library-tests/frameworks/apache-commons-lang3/TextStringBuilderTest.java @@ -14,6 +14,9 @@ class TextStringBuilderTest { void test() throws Exception { + TextStringBuilder cons1 = new TextStringBuilder(taint()); sink(cons1.toString()); // $hasTaintFlow=y + TextStringBuilder cons2 = new TextStringBuilder((CharSequence)taint()); sink(cons2.toString()); // $hasTaintFlow=y + TextStringBuilder sb1 = new TextStringBuilder(); sb1.append(taint().toCharArray()); sink(sb1.toString()); // $hasTaintFlow=y TextStringBuilder sb2 = new TextStringBuilder(); sb2.append(taint().toCharArray(), 0, 0); sink(sb2.toString()); // $hasTaintFlow=y TextStringBuilder sb3 = new TextStringBuilder(); sb3.append(CharBuffer.wrap(taint().toCharArray())); sink(sb3.toString()); // BAD (but not detected because we don't model CharBuffer yet)