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 69252d40670..36efc48205a 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 @@ -749,8 +749,8 @@ public String onStringReplace( return StringUtils.replaceAndTaint( taintedObjects, self, - Pattern.compile((String) oldCharSeq), - (String) newCharSeq, + Pattern.compile(oldCharSeq.toString(), Pattern.LITERAL), + newCharSeq.toString(), rangesSelf, rangesInput, Integer.MAX_VALUE); diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/util/StringUtils.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/util/StringUtils.java index 86aa33ea576..fd02d0dc78e 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/util/StringUtils.java +++ b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/util/StringUtils.java @@ -87,6 +87,11 @@ public static String replaceAndTaint( int firstRange = 0; int newLength = replacement.length(); + // In case there is a '\' or '$' in the replacement string we need to make a + // quoteReplacement + // If there is no '\' or '$' it will return the same string. + String finalReplacement = Matcher.quoteReplacement(replacement); + boolean canAddRange = true; StringBuffer sb = new StringBuffer(); do { @@ -165,7 +170,7 @@ public static String replaceAndTaint( canAddRange = newRanges.add(rangesInput, start + offset); } - matcher.appendReplacement(sb, replacement); + matcher.appendReplacement(sb, finalReplacement); offset = diffLength; numOfReplacements--; 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 c307127d6db..77e406ffa15 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 @@ -1252,6 +1252,7 @@ class StringModuleTest extends IastModuleImplTestBase { given: final taintedObjects = ctx.getTaintedObjects() def self = addFromTaintFormat(taintedObjects, testString) + def originalReplace = self.replace(oldCharSeq, newCharSeq) when: def result = module.onStringReplace(self, oldCharSeq, newCharSeq) @@ -1259,6 +1260,7 @@ class StringModuleTest extends IastModuleImplTestBase { then: 1 * tracer.activeSpan() >> span + originalReplace == result taintFormat(result, taintedObject.getRanges()) == expected where: @@ -1277,6 +1279,9 @@ class StringModuleTest extends IastModuleImplTestBase { "==>my_o<==u==>tput<==" | 'out' | 'in' | "==>my_<==in==>put<==" "==>my_o<==u==>tput<====>my_o<==u==>tput<==" | 'out' | 'in' | "==>my_<==in==>put<====>my_<==in==>put<==" "==>my_o<==u==>tp<==ut" | 'output' | 'input' | "==>my_<==input" + "==>my_input<==" | '_' | '/\\,.*+' | "==>my<==/\\,.*+==>input<==" + "==>my_input<==" | '_' | '!?^&$#' | "==>my<==!?^&\$#==>input<==" + "==>my_input<==" | '_' | ')(][}{' | "==>my<==)(][}{==>input<==" } void 'test replace with a char sequence (tainted) and make sure IastRequestContext is called'() { @@ -1284,6 +1289,7 @@ class StringModuleTest extends IastModuleImplTestBase { final taintedObjects = ctx.getTaintedObjects() def self = addFromTaintFormat(taintedObjects, testString) def inputTainted = addFromTaintFormat(taintedObjects, newCharSeq) + def originalReplace = self.replace(oldCharSeq, inputTainted) when: def result = module.onStringReplace(self, oldCharSeq, inputTainted) @@ -1291,31 +1297,41 @@ class StringModuleTest extends IastModuleImplTestBase { then: 1 * tracer.activeSpan() >> span + originalReplace == result taintFormat(result, taintedObject.getRanges()) == expected where: - testString | oldCharSeq | newCharSeq | expected - "==>masquita<==" | 'as' | '==>os<==' | "==>m<====>os<====>quita<==" - "==>masquita<==" | 'os' | '==>as<==' | "==>masquita<==" - "masquita" | 'as' | '==>os<==' | "m==>os<==quita" - "==>m<==as==>qu<==i==>ta<==" | 'as' | '==>os<==' | "==>m<====>os<====>qu<==i==>ta<==" - "==>my_input<==" | 'in' | '==>out<==' | "==>my_<====>out<====>put<==" - "==>my_output<==" | 'out' | '==>in<==' | "==>my_<====>in<====>put<==" - "==>my_input<==" | '_' | '==>-<==' | "==>my<====>-<====>input<==" - "==>my<==_==>input<==" | 'in' | '==>out<==' | "==>my<==_==>out<====>put<==" - "==>my_in<==p==>ut<==" | 'in' | '==>out<==' | "==>my_<====>out<==p==>ut<==" - "==>my_<==in==>put<==" | 'in' | '==>out<==' | "==>my_<====>out<====>put<==" - "==>my_i<==n==>put<==" | 'in' | '==>out<==' | "==>my_<====>out<====>put<==" - "==>my_<==i==>nput<==" | 'in' | '==>out<==' | "==>my_<====>out<====>put<==" - "==>my_o<==u==>tput<==" | 'out' | '==>in<==' | "==>my_<====>in<====>put<==" - "==>my_o<==u==>tput<====>my_o<==u==>tput<==" | 'out' | '==>in<==' | "==>my_<====>in<====>put<====>my_<====>in<====>put<==" - "==>my_o<==u==>tp<==ut" | 'output' | '==>input<==' | "==>my_<====>input<==" + testString | oldCharSeq | newCharSeq | expected + "==>masquita<==" | 'as' | '==>os<==' | "==>m<====>os<====>quita<==" + "==>masquita<==" | 'os' | '==>as<==' | "==>masquita<==" + "masquita" | 'as' | '==>os<==' | "m==>os<==quita" + "==>m<==as==>qu<==i==>ta<==" | 'as' | '==>os<==' | "==>m<====>os<====>qu<==i==>ta<==" + "==>my_input<==" | 'in' | '==>out<==' | "==>my_<====>out<====>put<==" + "==>my_output<==" | 'out' | '==>in<==' | "==>my_<====>in<====>put<==" + "==>my_input<==" | '_' | '==>-<==' | "==>my<====>-<====>input<==" + "==>my<==_==>input<==" | 'in' | '==>out<==' | "==>my<==_==>out<====>put<==" + "==>my_in<==p==>ut<==" | 'in' | '==>out<==' | "==>my_<====>out<==p==>ut<==" + "==>my_<==in==>put<==" | 'in' | '==>out<==' | "==>my_<====>out<====>put<==" + "==>my_i<==n==>put<==" | 'in' | '==>out<==' | "==>my_<====>out<====>put<==" + "==>my_<==i==>nput<==" | 'in' | '==>out<==' | "==>my_<====>out<====>put<==" + "==>my_o<==u==>tput<==" | 'out' | '==>in<==' | "==>my_<====>in<====>put<==" + "==>my_o<==u==>tput<====>my_o<==u==>tput<==" | 'out' | '==>in<==' | "==>my_<====>in<====>put<====>my_<====>in<====>put<==" + "==>my_o<==u==>tp<==ut" | 'output' | '==>input<==' | "==>my_<====>input<==" + "==>my_input<==" | '_' | '==>/\\,.*+<==' | "==>my<====>/\\,.*+<====>input<==" + "==>my_input<==" | '_' | '==>!?^&$#<==' | "==>my<====>!?^&\$#<====>input<==" + "==>my_input<==" | '_' | '==>)(][}{<==' | "==>my<====>)(][}{<====>input<==" } void 'test replace with a regex and replacement (not tainted) and make sure IastRequestContext is called'() { given: final taintedObjects = ctx.getTaintedObjects() def self = addFromTaintFormat(taintedObjects, testString) + def originalReplace + if (numReplacements > 1) { + originalReplace = self.replaceAll(regex, replacement) + } else { + originalReplace = self.replaceFirst(regex, replacement) + } when: def result = module.onStringReplace(self, regex, replacement, numReplacements) @@ -1323,6 +1339,9 @@ class StringModuleTest extends IastModuleImplTestBase { then: 1 * tracer.activeSpan() >> span + if (numReplacements > 0) { + originalReplace == result + } taintFormat(result, taintedObject.getRanges()) == expected where: @@ -1341,6 +1360,9 @@ class StringModuleTest extends IastModuleImplTestBase { "==>my_o<==u==>tput<==" | 'out' | 'in' | Integer.MAX_VALUE | "==>my_<==in==>put<==" "==>my_o<==u==>tput<====>my_o<==u==>tput<==" | 'out' | 'in' | Integer.MAX_VALUE | "==>my_<==in==>put<====>my_<==in==>put<==" "==>my_o<==u==>tp<==ut" | 'output' | 'input' | Integer.MAX_VALUE | "==>my_<==input" + "==>my_input<==" | '_' | '/\\,.*+' | Integer.MAX_VALUE | "==>my<==/\\,.*+==>input<==" + "==>my_input<==" | '_' | '!?^&#' | Integer.MAX_VALUE | "==>my<==!?^&#==>input<==" + "==>my_input<==" | '_' | ')(][}{' | Integer.MAX_VALUE | "==>my<==)(][}{==>input<==" } void 'test replace with a regex and replacement (tainted) and make sure IastRequestContext is called'() { @@ -1348,6 +1370,12 @@ class StringModuleTest extends IastModuleImplTestBase { final taintedObjects = ctx.getTaintedObjects() def self = addFromTaintFormat(taintedObjects, testString) def inputTainted = addFromTaintFormat(taintedObjects, replacement) + def originalReplace + if (numReplacements > 1) { + originalReplace = self.replaceAll(regex, inputTainted) + } else { + originalReplace = self.replaceFirst(regex, inputTainted) + } when: def result = module.onStringReplace(self, regex, inputTainted, numReplacements) @@ -1355,27 +1383,33 @@ class StringModuleTest extends IastModuleImplTestBase { then: 1 * tracer.activeSpan() >> span + if (numReplacements > 0) { + originalReplace == result + } taintFormat(result, taintedObject.getRanges()) == expected where: - testString | regex | replacement | numReplacements | expected - "==>masquita<==" | 'as' | '==>os<==' | Integer.MAX_VALUE | "==>m<====>os<====>quita<==" - "==>masquita<==" | 'os' | '==>as<==' | Integer.MAX_VALUE | "==>masquita<==" - "masquita" | 'as' | '==>os<==' | Integer.MAX_VALUE | "m==>os<==quita" - "==>m<==as==>qu<==i==>ta<==" | 'as' | '==>os<==' | Integer.MAX_VALUE | "==>m<====>os<====>qu<==i==>ta<==" - "==>my_input<==" | 'in' | '==>out<==' | Integer.MAX_VALUE | "==>my_<====>out<====>put<==" - "==>my_output<==" | 'out' | '==>in<==' | Integer.MAX_VALUE | "==>my_<====>in<====>put<==" - "==>my_input<==" | '_' | '==>-<==' | Integer.MAX_VALUE | "==>my<====>-<====>input<==" - "==>my<==_==>input<==" | 'in' | '==>out<==' | Integer.MAX_VALUE | "==>my<==_==>out<====>put<==" - "==>my_in<==p==>ut<==" | 'in' | '==>out<==' | Integer.MAX_VALUE | "==>my_<====>out<==p==>ut<==" - "==>my_<==in==>put<==" | 'in' | '==>out<==' | Integer.MAX_VALUE | "==>my_<====>out<====>put<==" - "==>my_i<==n==>put<==" | 'in' | '==>out<==' | Integer.MAX_VALUE | "==>my_<====>out<====>put<==" - "==>my_<==i==>nput<==" | 'in' | '==>out<==' | Integer.MAX_VALUE | "==>my_<====>out<====>put<==" - "==>my_o<==u==>tput<==" | 'out' | '==>in<==' | Integer.MAX_VALUE | "==>my_<====>in<====>put<==" - "==>my_o<==u==>tput<====>my_o<==u==>tput<==" | 'out' | '==>in<==' | Integer.MAX_VALUE | "==>my_<====>in<====>put<====>my_<====>in<====>put<==" - "==>my_o<==u==>tp<==ut" | 'output' | '==>input<==' | Integer.MAX_VALUE | "==>my_<====>input<==" - "==>my_o<==u==>tput<====>my_o<==u==>tput<==" | 'out' | '==>in<==' | 1 | "==>my_<====>in<====>put<====>my_o<==u==>tput<==" - "==>my_o<==u==>tput<====>my_o<==u==>tput<==" | 'out' | '==>in<==' | 0 | "==>my_o<==u==>tput<====>my_o<==u==>tput<==" + testString | regex | replacement | numReplacements | expected + "==>masquita<==" | 'as' | '==>os<==' | Integer.MAX_VALUE | "==>m<====>os<====>quita<==" + "==>masquita<==" | 'os' | '==>as<==' | Integer.MAX_VALUE | "==>masquita<==" + "masquita" | 'as' | '==>os<==' | Integer.MAX_VALUE | "m==>os<==quita" + "==>m<==as==>qu<==i==>ta<==" | 'as' | '==>os<==' | Integer.MAX_VALUE | "==>m<====>os<====>qu<==i==>ta<==" + "==>my_input<==" | 'in' | '==>out<==' | Integer.MAX_VALUE | "==>my_<====>out<====>put<==" + "==>my_output<==" | 'out' | '==>in<==' | Integer.MAX_VALUE | "==>my_<====>in<====>put<==" + "==>my_input<==" | '_' | '==>-<==' | Integer.MAX_VALUE | "==>my<====>-<====>input<==" + "==>my<==_==>input<==" | 'in' | '==>out<==' | Integer.MAX_VALUE | "==>my<==_==>out<====>put<==" + "==>my_in<==p==>ut<==" | 'in' | '==>out<==' | Integer.MAX_VALUE | "==>my_<====>out<==p==>ut<==" + "==>my_<==in==>put<==" | 'in' | '==>out<==' | Integer.MAX_VALUE | "==>my_<====>out<====>put<==" + "==>my_i<==n==>put<==" | 'in' | '==>out<==' | Integer.MAX_VALUE | "==>my_<====>out<====>put<==" + "==>my_<==i==>nput<==" | 'in' | '==>out<==' | Integer.MAX_VALUE | "==>my_<====>out<====>put<==" + "==>my_o<==u==>tput<==" | 'out' | '==>in<==' | Integer.MAX_VALUE | "==>my_<====>in<====>put<==" + "==>my_o<==u==>tput<====>my_o<==u==>tput<==" | 'out' | '==>in<==' | Integer.MAX_VALUE | "==>my_<====>in<====>put<====>my_<====>in<====>put<==" + "==>my_o<==u==>tp<==ut" | 'output' | '==>input<==' | Integer.MAX_VALUE | "==>my_<====>input<==" + "==>my_o<==u==>tput<====>my_o<==u==>tput<==" | 'out' | '==>in<==' | 1 | "==>my_<====>in<====>put<====>my_o<==u==>tput<==" + "==>my_o<==u==>tput<====>my_o<==u==>tput<==" | 'out' | '==>in<==' | 0 | "==>my_o<==u==>tput<====>my_o<==u==>tput<==" + "==>my_input<==" | '_' | '==>/\\,.*+<==' | Integer.MAX_VALUE | "==>my<====>/\\,.*+<====>input<==" + "==>my_input<==" | '_' | '==>!?^&#<==' | Integer.MAX_VALUE | "==>my<====>!?^&#<====>input<==" + "==>my_input<==" | '_' | '==>)(][}{<==' | Integer.MAX_VALUE | "==>my<====>)(][}{<====>input<==" } void 'test valueOf with (#param) and make sure IastRequestContext is called'() { diff --git a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/StringExperimentalCallSite.java b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/StringExperimentalCallSite.java index 762b6eaa84c..977d9ea4066 100644 --- a/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/StringExperimentalCallSite.java +++ b/dd-java-agent/instrumentation/java-lang/src/main/java/datadog/trace/instrumentation/java/lang/StringExperimentalCallSite.java @@ -38,16 +38,20 @@ public static String afterReplaceCharSeq( module.onUnexpectedException("afterReplaceCharSeq threw", e); } } + + IastMetricCollector.add(IastMetric.EXPERIMENTAL_PROPAGATION, 1); + if (!result.equals(newReplaced)) { LOGGER.debug( SEND_TELEMETRY, "afterReplaceCharSeq failed due to a different result between original replace and new replace, originalLength: {}, newLength: {}", result.length(), newReplaced != null ? newReplaced.length() : 0); + + return result; } - IastMetricCollector.add(IastMetric.EXPERIMENTAL_PROPAGATION, 1); - return result; + return newReplaced; } @CallSite.After( @@ -67,16 +71,20 @@ public static String afterReplaceAll( module.onUnexpectedException("afterReplaceAll threw", e); } } + + IastMetricCollector.add(IastMetric.EXPERIMENTAL_PROPAGATION, 1); + if (!result.equals(newReplaced)) { LOGGER.debug( SEND_TELEMETRY, "afterReplaceAll failed due to a different result between original replace and new replace, originalLength: {}, newLength: {}", result.length(), newReplaced != null ? newReplaced.length() : 0); + + return result; } - IastMetricCollector.add(IastMetric.EXPERIMENTAL_PROPAGATION, 1); - return result; + return newReplaced; } @CallSite.After( @@ -96,15 +104,19 @@ public static String afterReplaceFirst( module.onUnexpectedException("afterReplaceFirst threw", e); } } + + IastMetricCollector.add(IastMetric.EXPERIMENTAL_PROPAGATION, 1); + if (!result.equals(newReplaced)) { LOGGER.debug( SEND_TELEMETRY, "afterReplaceFirst failed due to a different result between original replace and new replace, originalLength: {}, newLength: {}", result.length(), newReplaced != null ? newReplaced.length() : 0); + + return result; } - IastMetricCollector.add(IastMetric.EXPERIMENTAL_PROPAGATION, 1); - return result; + return newReplaced; } }