From c23f9bede0f5f7efa617b6d49944fa1dd4958419 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Vidal=20Dom=C3=ADnguez?= Date: Thu, 21 Nov 2024 13:01:06 +0100 Subject: [PATCH 1/2] Add support for StringBuffer substring --- .../iast/propagation/StringModuleTest.groovy | 81 ++++++++++--------- .../java/lang/StringBuilderCallSite.java | 5 ++ .../lang/StringBuilderCallSiteTest.groovy | 14 +++- .../java/foo/bar/TestStringBuilderSuite.java | 14 ++++ 4 files changed, 74 insertions(+), 40 deletions(-) diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/propagation/StringModuleTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/propagation/StringModuleTest.groovy index 63c56202132..47dd95923f3 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/propagation/StringModuleTest.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/propagation/StringModuleTest.groovy @@ -481,42 +481,47 @@ class StringModuleTest extends IastModuleImplTestBase { } where: - self | beginIndex | endIndex | expected - "==>0123<==" | 0 | 4 | "==>0123<==" - "0123==>456<==78" | 0 | 5 | "0123==>4<==" - "01==>234<==5==>678<==90" | 0 | 8 | "01==>234<==5==>67<==" - "==>0123<==" | 0 | 3 | "==>012<==" - "==>0123<==" | 1 | 4 | "==>123<==" - "==>0123<==" | 1 | 3 | "==>12<==" - "0123==>456<==78" | 1 | 8 | "123==>456<==7" - "0123==>456<==78" | 0 | 4 | "0123" - "0123==>456<==78" | 7 | 9 | "78" - "0123==>456<==78" | 1 | 5 | "123==>4<==" - "0123==>456<==78" | 1 | 6 | "123==>45<==" - "0123==>456<==78" | 4 | 7 | "==>456<==" - "0123==>456<==78" | 6 | 8 | "==>6<==7" - "0123==>456<==78" | 5 | 8 | "==>56<==7" - "0123==>456<==78" | 4 | 6 | "==>45<==" - "01==>234<==5==>678<==90" | 1 | 10 | "1==>234<==5==>678<==9" - "01==>234<==5==>678<==90" | 1 | 2 | "1" - "01==>234<==5==>678<==90" | 5 | 6 | "5" - "01==>234<==5==>678<==90" | 9 | 10 | "9" - "01==>234<==5==>678<==90" | 1 | 4 | "1==>23<==" - "01==>234<==5==>678<==90" | 2 | 4 | "==>23<==" - "01==>234<==5==>678<==90" | 2 | 5 | "==>234<==" - "01==>234<==5==>678<==90" | 1 | 8 | "1==>234<==5==>67<==" - "01==>234<==5==>678<==90" | 2 | 8 | "==>234<==5==>67<==" - "01==>234<==5==>678<==90" | 2 | 9 | "==>234<==5==>678<==" - "01==>234<==5==>678<==90" | 5 | 8 | "5==>67<==" - "01==>234<==5==>678<==90" | 6 | 8 | "==>67<==" - "01==>234<==5==>678<==90" | 6 | 9 | "==>678<==" - "01==>234<==5==>678<==90" | 4 | 9 | "==>4<==5==>678<==" - "01==>234<==5==>678<==90" | 4 | 8 | "==>4<==5==>67<==" - sb("==>0123<==") | 0 | 4 | "==>0123<==" - sb("0123==>456<==78") | 0 | 5 | "0123==>4<==" - sb("01==>234<==5==>678<==90") | 0 | 8 | "01==>234<==5==>67<==" - sb("0123==>456<==78") | 4 | 6 | "==>45<==" - sb("01==>234<==5==>678<==90") | 4 | 8 | "==>4<==5==>67<==" + self | beginIndex | endIndex | expected + "==>0123<==" | 0 | 4 | "==>0123<==" + "0123==>456<==78" | 0 | 5 | "0123==>4<==" + "01==>234<==5==>678<==90" | 0 | 8 | "01==>234<==5==>67<==" + "==>0123<==" | 0 | 3 | "==>012<==" + "==>0123<==" | 1 | 4 | "==>123<==" + "==>0123<==" | 1 | 3 | "==>12<==" + "0123==>456<==78" | 1 | 8 | "123==>456<==7" + "0123==>456<==78" | 0 | 4 | "0123" + "0123==>456<==78" | 7 | 9 | "78" + "0123==>456<==78" | 1 | 5 | "123==>4<==" + "0123==>456<==78" | 1 | 6 | "123==>45<==" + "0123==>456<==78" | 4 | 7 | "==>456<==" + "0123==>456<==78" | 6 | 8 | "==>6<==7" + "0123==>456<==78" | 5 | 8 | "==>56<==7" + "0123==>456<==78" | 4 | 6 | "==>45<==" + "01==>234<==5==>678<==90" | 1 | 10 | "1==>234<==5==>678<==9" + "01==>234<==5==>678<==90" | 1 | 2 | "1" + "01==>234<==5==>678<==90" | 5 | 6 | "5" + "01==>234<==5==>678<==90" | 9 | 10 | "9" + "01==>234<==5==>678<==90" | 1 | 4 | "1==>23<==" + "01==>234<==5==>678<==90" | 2 | 4 | "==>23<==" + "01==>234<==5==>678<==90" | 2 | 5 | "==>234<==" + "01==>234<==5==>678<==90" | 1 | 8 | "1==>234<==5==>67<==" + "01==>234<==5==>678<==90" | 2 | 8 | "==>234<==5==>67<==" + "01==>234<==5==>678<==90" | 2 | 9 | "==>234<==5==>678<==" + "01==>234<==5==>678<==90" | 5 | 8 | "5==>67<==" + "01==>234<==5==>678<==90" | 6 | 8 | "==>67<==" + "01==>234<==5==>678<==90" | 6 | 9 | "==>678<==" + "01==>234<==5==>678<==90" | 4 | 9 | "==>4<==5==>678<==" + "01==>234<==5==>678<==90" | 4 | 8 | "==>4<==5==>67<==" + sb("==>0123<==") | 0 | 4 | "==>0123<==" + sb("0123==>456<==78") | 0 | 5 | "0123==>4<==" + sb("01==>234<==5==>678<==90") | 0 | 8 | "01==>234<==5==>67<==" + sb("0123==>456<==78") | 4 | 6 | "==>45<==" + sb("01==>234<==5==>678<==90") | 4 | 8 | "==>4<==5==>67<==" + sbf("==>0123<==") | 0 | 4 | "==>0123<==" + sbf("0123==>456<==78") | 0 | 5 | "0123==>4<==" + sbf("01==>234<==5==>678<==90") | 0 | 8 | "01==>234<==5==>67<==" + sbf("0123==>456<==78") | 4 | 6 | "==>45<==" + sbf("01==>234<==5==>678<==90") | 4 | 8 | "==>4<==5==>67<==" } void 'onStringJoin without null delimiter or elements (#delimiter, #elements)'() { @@ -1263,4 +1268,8 @@ class StringModuleTest extends IastModuleImplTestBase { private static StringBuilder sb(final String string) { return new StringBuilder(string) } + + private static StringBuilder sbf(final String string) { + return new StringBuilder(string) + } } diff --git a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/StringBuilderCallSite.java b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/StringBuilderCallSite.java index 4a85301d4fd..80feb171f2a 100644 --- a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/StringBuilderCallSite.java +++ b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/StringBuilderCallSite.java @@ -10,6 +10,9 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; +/** + * This class provides instrumentation for {@link StringBuilder} and {@link StringBuffer} methods. + */ @Propagation @CallSite(spi = IastCallSites.class) public class StringBuilderCallSite { @@ -103,6 +106,7 @@ public static String afterToString( } @CallSite.After("java.lang.String java.lang.StringBuilder.substring(int)") + @CallSite.After("java.lang.String java.lang.StringBuffer.substring(int)") public static String afterSubstring( @CallSite.This final CharSequence self, @CallSite.Argument final int beginIndex, @@ -119,6 +123,7 @@ public static String afterSubstring( } @CallSite.After("java.lang.String java.lang.StringBuilder.substring(int, int)") + @CallSite.After("java.lang.String java.lang.StringBuffer.substring(int, int)") public static String afterSubstring( @CallSite.This final CharSequence self, @CallSite.Argument final int beginIndex, diff --git a/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/StringBuilderCallSiteTest.groovy b/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/StringBuilderCallSiteTest.groovy index 9c53242b939..8080c140d0a 100644 --- a/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/StringBuilderCallSiteTest.groovy +++ b/dd-java-agent/instrumentation/java-lang/src/test/groovy/datadog/trace/instrumentation/java/lang/StringBuilderCallSiteTest.groovy @@ -188,8 +188,9 @@ class StringBuilderCallSiteTest extends AgentTestRunner { 0 * _ where: - param | beginIndex | expected - sb('012345') | 1 | '12345' + param | beginIndex | expected + sb('012345') | 1 | '12345' + sbf('012345') | 1 | '12345' } def 'test string builder substring with endIndex call site'() { @@ -206,8 +207,9 @@ class StringBuilderCallSiteTest extends AgentTestRunner { 0 * _ where: - param | beginIndex | endIndex | expected - sb('012345') | 1 | 5 | '1234' + param | beginIndex | endIndex | expected + sb('012345') | 1 | 5 | '1234' + sbf('012345') | 1 | 5 | '1234' } private static class BrokenToString { @@ -226,4 +228,8 @@ class StringBuilderCallSiteTest extends AgentTestRunner { private static StringBuilder sb(final String string) { return new StringBuilder(string) } + + private static StringBuffer sbf(final String string) { + return new StringBuffer(string) + } } diff --git a/dd-java-agent/instrumentation/java-lang/src/test/java/foo/bar/TestStringBuilderSuite.java b/dd-java-agent/instrumentation/java-lang/src/test/java/foo/bar/TestStringBuilderSuite.java index 1edd8b1500b..59280cb09ae 100644 --- a/dd-java-agent/instrumentation/java-lang/src/test/java/foo/bar/TestStringBuilderSuite.java +++ b/dd-java-agent/instrumentation/java-lang/src/test/java/foo/bar/TestStringBuilderSuite.java @@ -90,4 +90,18 @@ public static String substring(StringBuilder self, int beginIndex) { LOGGER.debug("After string builder substring {}", result); return result; } + + public static String substring(StringBuffer self, int beginIndex, int endIndex) { + LOGGER.debug("Before string buffer substring {} from {} to {}", self, beginIndex, endIndex); + final String result = self.substring(beginIndex, endIndex); + LOGGER.debug("After string buffer substring {}", result); + return result; + } + + public static String substring(StringBuffer self, int beginIndex) { + LOGGER.debug("Before string buffer substring {} from {}", self, beginIndex); + final String result = self.substring(beginIndex); + LOGGER.debug("After string buffer substring {}", result); + return result; + } } From e1bcfc9ea70c02562d056f7c71449d31756a48c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Vidal=20Dom=C3=ADnguez?= Date: Thu, 21 Nov 2024 18:46:17 +0100 Subject: [PATCH 2/2] Fix StringModuleTest --- .../com/datadog/iast/propagation/StringModuleTest.groovy | 4 ++-- .../test/groovy/com/datadog/iast/taint/TaintUtils.groovy | 9 +++++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/propagation/StringModuleTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/propagation/StringModuleTest.groovy index 47dd95923f3..6a96cdd1603 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/propagation/StringModuleTest.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/propagation/StringModuleTest.groovy @@ -1269,7 +1269,7 @@ class StringModuleTest extends IastModuleImplTestBase { return new StringBuilder(string) } - private static StringBuilder sbf(final String string) { - return new StringBuilder(string) + private static StringBuffer sbf(final String string) { + return new StringBuffer(string) } } diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/TaintUtils.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/TaintUtils.groovy index c064b767b1d..863ec96ce18 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/TaintUtils.groovy +++ b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/taint/TaintUtils.groovy @@ -103,13 +103,18 @@ class TaintUtils { return resultString } - static StringBuilder addFromTaintFormat(final TaintedObjects tos, final StringBuilder sb) { + static Appendable addFromTaintFormat(final TaintedObjects tos, final Appendable sb) { final String s = sb.toString() final ranges = fromTaintFormat(s) if (ranges == null || ranges.length == 0) { return sb } - final result = new StringBuilder(getStringFromTaintFormat(s)) + def result + if (sb instanceof StringBuffer) { + result = new StringBuffer(getStringFromTaintFormat(s)) + } else { + result = new StringBuilder(getStringFromTaintFormat(s)) + } tos.taint(result, ranges) return result }