From 02e651a45172e8e2ec27481608c54d8dec0347d7 Mon Sep 17 00:00:00 2001 From: "Santiago M. Mola" Date: Mon, 23 Sep 2024 14:23:23 +0200 Subject: [PATCH] Revert "Add propagation to String strip methods (#7651)" (#7664) This reverts commit 8da3e7aff152d2fe4fee43819ff600f1f0fed27b. --- .../iast/propagation/StringModuleImpl.java | 37 ------------ .../iast/propagation/StringModuleTest.groovy | 56 ------------------- .../java/lang/jdk11/StringCallSite.java | 29 ---------- .../java/lang/jdk11/StringCallSiteTest.groovy | 21 ------- .../java/foo/bar/TestStringJDK11Suite.java | 21 ------- .../api/iast/propagation/StringModule.java | 2 - 6 files changed, 166 deletions(-) diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/propagation/StringModuleImpl.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/propagation/StringModuleImpl.java index be015d69d2e..db1f1ad865d 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/propagation/StringModuleImpl.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/propagation/StringModuleImpl.java @@ -567,43 +567,6 @@ public void onSplit(@Nonnull String self, @Nonnull String[] result) { } } - @Override - @SuppressFBWarnings("ES_COMPARING_PARAMETER_STRING_WITH_EQ") - public void onStringStrip(@Nonnull String self, @Nonnull String result, boolean trailing) { - if (self == result || !canBeTainted(result)) { - return; - } - final IastContext ctx = IastContext.Provider.get(); - if (ctx == null) { - return; - } - final TaintedObjects taintedObjects = ctx.getTaintedObjects(); - final TaintedObject taintedSelf = taintedObjects.get(self); - if (taintedSelf == null) { - return; - } - - final Range[] rangesSelf = taintedSelf.getRanges(); - if (rangesSelf.length == 0) { - return; - } - - int offset = 0; - if (!trailing) { - while ((offset < self.length()) && (Character.isWhitespace(self.charAt(offset)))) { - offset++; - } - } - - int resultLength = result.length(); - - final Range[] newRanges = Ranges.forSubstring(offset, resultLength, rangesSelf); - - if (newRanges != null) { - taintedObjects.taint(result, newRanges); - } - } - /** * Adds the tainted ranges belonging to the current parameter added via placeholder taking care of * an optional tainted placeholder. 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 3fc48d97be7..e8d7ece64e0 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 @@ -984,62 +984,6 @@ class StringModuleTest extends IastModuleImplTestBase { '==>testing<== the ==>test<==' | ' ' | ['==>testing<==', '==>the<==', '==>test<=='] as String[] } - void 'test #method and make sure IastRequestContext is called'() { - given: - final taintedObjects = ctx.getTaintedObjects() - def self = addFromTaintFormat(taintedObjects, testString) - def result = self."$method"() - - when: - module.onStringStrip(self, result, trailing) - def taintedObject = taintedObjects.get(result) - - then: - 1 * tracer.activeSpan() >> span - taintFormat(result, taintedObject.getRanges()) == expected - - where: - method | trailing | testString | expected - "strip" | false | " ==>123<== " | "==>123<==" - "stripLeading" | false | " ==>123<== " | "==>123<== " - "stripTrailing" | true | " ==>123<== " | " ==>123<==" - "strip" | false | " ==> <== ==> <== ==>456<== ==>ABC<== ==> <== ==> <== " | "==>456<== ==>ABC<==" - "stripLeading" | false | " ==> <== ==> <== ==>456<== ==>ABC<== ==> <== ==> <== " | "==>456<== ==>ABC<== ==> <== ==> <== " - "stripTrailing" | true | " ==> <== ==> <== ==>456<== ==>ABC<== ==> <== ==> <== " | " ==> <== ==> <== ==>456<== ==>ABC<==" - "strip" | false | " ==>123<== " | "==>123<==" - "stripLeading" | false | " ==>123<== " | "==>123<== " - "stripTrailing" | true | " ==>123<== " | " ==>123<==" - "strip" | false | "==> 123 <==" | "==>123<==" - "stripLeading" | false | "==> 123 <==" | "==>123 <==" - "stripTrailing" | true | "==> 123 <==" | "==> 123<==" - "strip" | false | " a==> b <==c " | "a==> b <==c" - "stripLeading" | false | " a==> b <==c " | "a==> b <==c " - "stripTrailing" | true | " a==> b <==c " | " a==> b <==c" - } - - void 'test #method for empty string cases'() { - given: - final taintedObjects = ctx.getTaintedObjects() - def self = addFromTaintFormat(taintedObjects, testString) - def result = self."$method"() - - when: - module.onStringStrip(self, result, trailing) - - then: - null == taintedObjects.get(result) - result == expected - - where: - method | trailing | testString | expected - "strip" | false | " ==> <== " | "" - "stripLeading" | false | " ==> <== " | "" - "stripTrailing" | true | " ==> <== " | "" - "strip" | false | "" | "" - "stripLeading" | false | "" | "" - "stripTrailing" | true | "" | "" - } - private static Date date(final String pattern, final String value) { return new SimpleDateFormat(pattern).parse(value) } diff --git a/dd-java-agent/instrumentation/java-lang/java-lang-11/src/main/java/datadog/trace/instrumentation/java/lang/jdk11/StringCallSite.java b/dd-java-agent/instrumentation/java-lang/java-lang-11/src/main/java/datadog/trace/instrumentation/java/lang/jdk11/StringCallSite.java index 2a5319c6b67..c035cb51a2b 100644 --- a/dd-java-agent/instrumentation/java-lang/java-lang-11/src/main/java/datadog/trace/instrumentation/java/lang/jdk11/StringCallSite.java +++ b/dd-java-agent/instrumentation/java-lang/java-lang-11/src/main/java/datadog/trace/instrumentation/java/lang/jdk11/StringCallSite.java @@ -26,33 +26,4 @@ public static String afterRepeat( } return result; } - - @CallSite.After("java.lang.String java.lang.String.strip()") - @CallSite.After("java.lang.String java.lang.String.stripLeading()") - public static String afterStrip( - @CallSite.This final String self, @CallSite.Return final String result) { - final StringModule module = InstrumentationBridge.STRING; - try { - if (module != null) { - module.onStringStrip(self, result, false); - } - } catch (final Throwable e) { - module.onUnexpectedException("afterStrip threw", e); - } - return result; - } - - @CallSite.After("java.lang.String java.lang.String.stripTrailing()") - public static String afterStripTrailing( - @CallSite.This final String self, @CallSite.Return final String result) { - final StringModule module = InstrumentationBridge.STRING; - try { - if (module != null) { - module.onStringStrip(self, result, true); - } - } catch (final Throwable e) { - module.onUnexpectedException("afterStripTrailing threw", e); - } - return result; - } } diff --git a/dd-java-agent/instrumentation/java-lang/java-lang-11/src/test/groovy/datadog/trace/instrumentation/java/lang/jdk11/StringCallSiteTest.groovy b/dd-java-agent/instrumentation/java-lang/java-lang-11/src/test/groovy/datadog/trace/instrumentation/java/lang/jdk11/StringCallSiteTest.groovy index 4a0908a2747..e0b97dbb98d 100644 --- a/dd-java-agent/instrumentation/java-lang/java-lang-11/src/test/groovy/datadog/trace/instrumentation/java/lang/jdk11/StringCallSiteTest.groovy +++ b/dd-java-agent/instrumentation/java-lang/java-lang-11/src/test/groovy/datadog/trace/instrumentation/java/lang/jdk11/StringCallSiteTest.groovy @@ -1,5 +1,3 @@ -package datadog.trace.instrumentation.java.lang.jdk11 - import datadog.trace.agent.test.AgentTestRunner import datadog.trace.api.iast.InstrumentationBridge import datadog.trace.api.iast.propagation.StringModule @@ -33,23 +31,4 @@ class StringCallSiteTest extends AgentTestRunner { 1 * iastModule.onStringRepeat(self, count, expected) 0 * _ } - - def 'test string #method call site'() { - setup: - final module = Mock(StringModule) - InstrumentationBridge.registerIastModule(module) - - when: - final result = TestStringJDK11Suite."$method"(input) - - then: - result == output - 1 * module.onStringStrip(input, output, trailing) - - where: - method | trailing | input | output - "stringStrip" | false | ' hello ' | 'hello' - "stringStripLeading" | false | ' hello ' | 'hello ' - "stringStripTrailing" | true | ' hello ' | ' hello' - } } diff --git a/dd-java-agent/instrumentation/java-lang/java-lang-11/src/test/java/foo/bar/TestStringJDK11Suite.java b/dd-java-agent/instrumentation/java-lang/java-lang-11/src/test/java/foo/bar/TestStringJDK11Suite.java index 25e3d57e769..6da40940227 100644 --- a/dd-java-agent/instrumentation/java-lang/java-lang-11/src/test/java/foo/bar/TestStringJDK11Suite.java +++ b/dd-java-agent/instrumentation/java-lang/java-lang-11/src/test/java/foo/bar/TestStringJDK11Suite.java @@ -15,25 +15,4 @@ public static String stringRepeat(String self, int count) { LOGGER.debug("After string repeat {}", result); return result; } - - public static String stringStrip(final String self) { - LOGGER.debug("Before string strip {}", self); - final String result = self.strip(); - LOGGER.debug("After string strip {}", result); - return result; - } - - public static String stringStripLeading(final String self) { - LOGGER.debug("Before string stripLeading {}", self); - final String result = self.stripLeading(); - LOGGER.debug("After string stripLeading {}", result); - return result; - } - - public static String stringStripTrailing(final String self) { - LOGGER.debug("Before string stripTrailing {}", self); - final String result = self.stripTrailing(); - LOGGER.debug("After string stripTrailing {}", result); - return result; - } } diff --git a/internal-api/src/main/java/datadog/trace/api/iast/propagation/StringModule.java b/internal-api/src/main/java/datadog/trace/api/iast/propagation/StringModule.java index c53b2d03117..4805e1dd6c1 100644 --- a/internal-api/src/main/java/datadog/trace/api/iast/propagation/StringModule.java +++ b/internal-api/src/main/java/datadog/trace/api/iast/propagation/StringModule.java @@ -50,6 +50,4 @@ void onStringFormat( @Nonnull Iterable literals, @Nonnull Object[] params, @Nonnull String result); void onSplit(final @Nonnull String self, final @Nonnull String[] result); - - void onStringStrip(@Nonnull String self, @Nonnull String result, boolean trailing); }