From 679f9e2ece9319cdb6f45b4439b67829e3453b78 Mon Sep 17 00:00:00 2001 From: Sezen Leblay Date: Thu, 16 Jan 2025 16:12:46 +0100 Subject: [PATCH] Propagation of translateEscapes of String class (#8186) --- .../iast/propagation/StringModuleImpl.java | 21 +++++++ .../iast/propagation/StringModuleTest.groovy | 22 +++++++ .../java-lang/java-lang-15/build.gradle | 43 +++++++++++++ .../java/lang/jdk15/StringCallSite.java | 27 +++++++++ .../java/lang/jdk15/StringCallSiteTest.groovy | 40 +++++++++++++ .../java/foo/bar/TestStringJDK15Suite.java | 18 ++++++ .../iast-util/iast-util-17/build.gradle | 35 +++++++++++ .../controller/StringOperationController.java | 17 ++++++ .../AbstractIast17SpringBootTest.groovy | 60 +++++++++++++++++++ .../springboot-java-17/build.gradle | 45 ++++++++++++++ .../springboot/SpringbootApplication.java | 14 +++++ .../springboot/IastSpringBootSmokeTest.groovy | 6 ++ .../api/iast/propagation/StringModule.java | 2 + settings.gradle | 3 + 14 files changed, 353 insertions(+) create mode 100644 dd-java-agent/instrumentation/java-lang/java-lang-15/build.gradle create mode 100644 dd-java-agent/instrumentation/java-lang/java-lang-15/src/main/java/datadog/trace/instrumentation/java/lang/jdk15/StringCallSite.java create mode 100644 dd-java-agent/instrumentation/java-lang/java-lang-15/src/test/groovy/datadog/trace/instrumentation/java/lang/jdk15/StringCallSiteTest.groovy create mode 100644 dd-java-agent/instrumentation/java-lang/java-lang-15/src/test/java/foo/bar/TestStringJDK15Suite.java create mode 100644 dd-smoke-tests/iast-util/iast-util-17/build.gradle create mode 100644 dd-smoke-tests/iast-util/iast-util-17/src/main/java/datadog/smoketest/springboot/controller/StringOperationController.java create mode 100644 dd-smoke-tests/iast-util/iast-util-17/src/testFixtures/groovy/datadog/smoketest/AbstractIast17SpringBootTest.groovy create mode 100644 dd-smoke-tests/springboot-java-17/build.gradle create mode 100644 dd-smoke-tests/springboot-java-17/src/main/java/datadog/smoketest/springboot/SpringbootApplication.java create mode 100644 dd-smoke-tests/springboot-java-17/src/test/groovy/datadog/smoketest/springboot/IastSpringBootSmokeTest.groovy 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 52a9c19f055..69252d40670 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 @@ -293,6 +293,27 @@ public void onStringJoin( } } + @Override + @SuppressFBWarnings("ES_COMPARING_PARAMETER_STRING_WITH_EQ") + public void onStringTranslateEscapes(@Nonnull String self, @Nullable String result) { + if (!canBeTainted(result)) { + return; + } + if (self == result) { // same ref, no change in taint status + 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; // original string is not tainted + } + taintedObjects.taint(result, taintedSelf.getRanges()); // only possibility left + } + @Override @SuppressFBWarnings("ES_COMPARING_PARAMETER_STRING_WITH_EQ") public void onStringRepeat(@Nonnull String self, int count, @Nonnull String result) { 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 cb2d40c20a5..c307127d6db 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 @@ -12,6 +12,7 @@ import datadog.trace.bootstrap.instrumentation.api.AgentSpan import datadog.trace.bootstrap.instrumentation.api.AgentTracer import groovy.transform.CompileDynamic import org.junit.jupiter.api.Assertions +import spock.lang.IgnoreIf import java.text.SimpleDateFormat @@ -1414,6 +1415,27 @@ class StringModuleTest extends IastModuleImplTestBase { taintFormat(result, taintedObject.getRanges()) == "==>my_input<==" } + @IgnoreIf({ System.getProperty('java.specification.version').toBigDecimal() < 15 }) + void 'test translate escapes'() { + given: + final taintedObjects = ctx.getTaintedObjects() + def self = addFromTaintFormat(taintedObjects, testString) + def result = self.translateEscapes() + + when: + module.onStringTranslateEscapes(self, result) + def taintedObject = taintedObjects.get(result) + + then: + taintFormat(result, taintedObject.getRanges()) == expected + + where: + testString | expected + "==>hello world\t<==" | "==>hello world\t<==" + "==>hello world\n<==" | "==>hello world\n<==" + "==>hello worldn<==" | "==>hello worldn<==" + } + void 'test valueOf with special objects and make sure IastRequestContext is called'() { given: final taintedObjects = ctx.getTaintedObjects() diff --git a/dd-java-agent/instrumentation/java-lang/java-lang-15/build.gradle b/dd-java-agent/instrumentation/java-lang/java-lang-15/build.gradle new file mode 100644 index 00000000000..0af4d7d047f --- /dev/null +++ b/dd-java-agent/instrumentation/java-lang/java-lang-15/build.gradle @@ -0,0 +1,43 @@ +plugins { + id 'idea' +} + +ext { + minJavaVersionForTests = JavaVersion.VERSION_15 +} + +apply from: "$rootDir/gradle/java.gradle" +apply plugin: 'call-site-instrumentation' + +muzzle { + pass { + coreJdk() + } +} + +idea { + module { + jdkName = '17' + } +} + +csi { + javaVersion = JavaLanguageVersion.of(17) +} + +addTestSuiteForDir('latestDepTest', 'test') + +dependencies { + testRuntimeOnly project(':dd-java-agent:instrumentation:iast-instrumenter') +} + +project.tasks.withType(AbstractCompile).configureEach { + setJavaVersion(it, 17) + if (it.name != 'compileCsiJava') { + sourceCompatibility = JavaVersion.VERSION_15 + targetCompatibility = JavaVersion.VERSION_15 + if (it instanceof JavaCompile) { + it.options.release.set(15) + } + } +} diff --git a/dd-java-agent/instrumentation/java-lang/java-lang-15/src/main/java/datadog/trace/instrumentation/java/lang/jdk15/StringCallSite.java b/dd-java-agent/instrumentation/java-lang/java-lang-15/src/main/java/datadog/trace/instrumentation/java/lang/jdk15/StringCallSite.java new file mode 100644 index 00000000000..8706339a849 --- /dev/null +++ b/dd-java-agent/instrumentation/java-lang/java-lang-15/src/main/java/datadog/trace/instrumentation/java/lang/jdk15/StringCallSite.java @@ -0,0 +1,27 @@ +package datadog.trace.instrumentation.java.lang.jdk15; + +import datadog.trace.agent.tooling.csi.CallSite; +import datadog.trace.api.iast.IastCallSites; +import datadog.trace.api.iast.InstrumentationBridge; +import datadog.trace.api.iast.Propagation; +import datadog.trace.api.iast.propagation.StringModule; + +@Propagation +@CallSite( + spi = IastCallSites.class, + enabled = {"datadog.trace.api.iast.IastEnabledChecks", "isMajorJavaVersionAtLeast", "15"}) +public class StringCallSite { + @CallSite.After("java.lang.String java.lang.String.translateEscapes()") + public static String afterTranslateEscapes( + @CallSite.This final String self, @CallSite.Return final String result) { + final StringModule module = InstrumentationBridge.STRING; + try { + if (module != null) { + module.onStringTranslateEscapes(self, result); + } + } catch (final Throwable e) { + module.onUnexpectedException("afterTranslateEscapes threw", e); + } + return result; + } +} diff --git a/dd-java-agent/instrumentation/java-lang/java-lang-15/src/test/groovy/datadog/trace/instrumentation/java/lang/jdk15/StringCallSiteTest.groovy b/dd-java-agent/instrumentation/java-lang/java-lang-15/src/test/groovy/datadog/trace/instrumentation/java/lang/jdk15/StringCallSiteTest.groovy new file mode 100644 index 00000000000..bea96f705bd --- /dev/null +++ b/dd-java-agent/instrumentation/java-lang/java-lang-15/src/test/groovy/datadog/trace/instrumentation/java/lang/jdk15/StringCallSiteTest.groovy @@ -0,0 +1,40 @@ +package datadog.trace.instrumentation.java.lang.jdk15 + +import com.github.javaparser.utils.StringEscapeUtils +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.api.iast.InstrumentationBridge +import datadog.trace.api.iast.propagation.StringModule +import foo.bar.TestStringJDK15Suite +import spock.lang.Requires + +@Requires({ + jvm.java15Compatible +}) +class StringCallSiteTest extends AgentTestRunner { + + @Override + protected void configurePreAgent() { + injectSysConfig("dd.iast.enabled", "true") + } + + def 'test string translate escapes call site'() { + setup: + final iastModule = Mock(StringModule) + InstrumentationBridge.registerIastModule(iastModule) + + when: + final result = TestStringJDK15Suite.stringTranslateEscapes(input) + + then: + result == output + 1 * iastModule.onStringTranslateEscapes(input, output) + + where: + input | output + "HelloThisisaline" | "HelloThisisaline" + "Hello\tThis is a line" | "Hello"+ StringEscapeUtils.unescapeJava("\\u0009") +"This is a line" + /Hello\sThis is a line/ | "Hello"+ StringEscapeUtils.unescapeJava("\\u0020") +"This is a line" + /Hello\"This is a line/ | "Hello"+ StringEscapeUtils.unescapeJava("\\u0022") +"This is a line" + /Hello\0This is a line/ | "Hello"+ StringEscapeUtils.unescapeJava("\\u0000") +"This is a line" + } +} diff --git a/dd-java-agent/instrumentation/java-lang/java-lang-15/src/test/java/foo/bar/TestStringJDK15Suite.java b/dd-java-agent/instrumentation/java-lang/java-lang-15/src/test/java/foo/bar/TestStringJDK15Suite.java new file mode 100644 index 00000000000..046978c773e --- /dev/null +++ b/dd-java-agent/instrumentation/java-lang/java-lang-15/src/test/java/foo/bar/TestStringJDK15Suite.java @@ -0,0 +1,18 @@ +package foo.bar; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public abstract class TestStringJDK15Suite { + + private static final Logger LOGGER = LoggerFactory.getLogger(TestStringJDK15Suite.class); + + private TestStringJDK15Suite() {} + + public static String stringTranslateEscapes(String self) { + LOGGER.debug("Before string translate escapes {}", self); + final String result = self.translateEscapes(); + LOGGER.debug("After string translate escapes {}", result); + return result; + } +} diff --git a/dd-smoke-tests/iast-util/iast-util-17/build.gradle b/dd-smoke-tests/iast-util/iast-util-17/build.gradle new file mode 100644 index 00000000000..7cf5228a533 --- /dev/null +++ b/dd-smoke-tests/iast-util/iast-util-17/build.gradle @@ -0,0 +1,35 @@ +plugins { + id 'idea' + id 'java-test-fixtures' +} + + +apply from: "$rootDir/gradle/java.gradle" + +description = 'iast-smoke-tests-utils-java-17' + +idea { + module { + jdkName = '17' + } +} + +dependencies { + api project(':dd-smoke-tests') + compileOnly group: 'org.springframework.boot', name: 'spring-boot-starter-web', version: '2.2.0.RELEASE' + + testFixturesImplementation testFixtures(project(":dd-smoke-tests:iast-util")) +} + +project.tasks.withType(AbstractCompile).configureEach { + setJavaVersion(it, 17) + sourceCompatibility = JavaVersion.VERSION_17 + targetCompatibility = JavaVersion.VERSION_17 + if (it instanceof JavaCompile) { + it.options.release.set(17) + } +} + +forbiddenApisMain { + failOnMissingClasses = false +} diff --git a/dd-smoke-tests/iast-util/iast-util-17/src/main/java/datadog/smoketest/springboot/controller/StringOperationController.java b/dd-smoke-tests/iast-util/iast-util-17/src/main/java/datadog/smoketest/springboot/controller/StringOperationController.java new file mode 100644 index 00000000000..3f819a60f20 --- /dev/null +++ b/dd-smoke-tests/iast-util/iast-util-17/src/main/java/datadog/smoketest/springboot/controller/StringOperationController.java @@ -0,0 +1,17 @@ +package datadog.smoketest.springboot.controller; + +import org.springframework.web.bind.annotation.PostMapping; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestParam; +import org.springframework.web.bind.annotation.RestController; + +@RestController +@RequestMapping("/string") +public class StringOperationController { + + @PostMapping("/translateEscapes") + public String translateEscapes(@RequestParam(value = "parameter") final String parameter) { + parameter.translateEscapes(); + return "ok"; + } +} diff --git a/dd-smoke-tests/iast-util/iast-util-17/src/testFixtures/groovy/datadog/smoketest/AbstractIast17SpringBootTest.groovy b/dd-smoke-tests/iast-util/iast-util-17/src/testFixtures/groovy/datadog/smoketest/AbstractIast17SpringBootTest.groovy new file mode 100644 index 00000000000..4ac43ea4fc3 --- /dev/null +++ b/dd-smoke-tests/iast-util/iast-util-17/src/testFixtures/groovy/datadog/smoketest/AbstractIast17SpringBootTest.groovy @@ -0,0 +1,60 @@ +package datadog.smoketest + +import com.github.javaparser.utils.StringEscapeUtils +import okhttp3.FormBody +import okhttp3.Request + +import static datadog.trace.api.config.IastConfig.IAST_DEBUG_ENABLED +import static datadog.trace.api.config.IastConfig.IAST_DETECTION_MODE +import static datadog.trace.api.config.IastConfig.IAST_ENABLED + +abstract class AbstractIast17SpringBootTest extends AbstractIastServerSmokeTest { + + @Override + ProcessBuilder createProcessBuilder() { + String springBootShadowJar = System.getProperty('datadog.smoketest.springboot.shadowJar.path') + + List command = [] + command.add(javaPath()) + command.addAll(defaultJavaProperties) + command.addAll(iastJvmOpts()) + command.addAll((String[]) ['-jar', springBootShadowJar, "--server.port=${httpPort}"]) + ProcessBuilder processBuilder = new ProcessBuilder(command) + processBuilder.directory(new File(buildDirectory)) + // Spring will print all environment variables to the log, which may pollute it and affect log assertions. + processBuilder.environment().clear() + return processBuilder + } + + protected List iastJvmOpts() { + return [ + withSystemProperty(IAST_ENABLED, true), + withSystemProperty(IAST_DETECTION_MODE, 'FULL'), + withSystemProperty(IAST_DEBUG_ENABLED, true), + ] + } + + void 'test String translateEscapes'() { + setup: + final url = "http://localhost:${httpPort}/string/translateEscapes" + final body = new FormBody.Builder() + .add('parameter', value) + .build() + final request = new Request.Builder().url(url).post(body).build() + + + when: + client.newCall(request).execute() + + then: + hasTainted { tainted -> + tainted.value == expected + } + + where: + value | expected + "withEscape\ttab" | "withEscape" + Character.toString((char)9) + "tab" + "withEscape\nnewline" | "withEscape" + StringEscapeUtils.unescapeJava("\\u000A")+ "newline" + "withEscape\bbackline" | "withEscape" + StringEscapeUtils.unescapeJava("\\u0008")+ "backline" + } +} diff --git a/dd-smoke-tests/springboot-java-17/build.gradle b/dd-smoke-tests/springboot-java-17/build.gradle new file mode 100644 index 00000000000..c6d99c2de64 --- /dev/null +++ b/dd-smoke-tests/springboot-java-17/build.gradle @@ -0,0 +1,45 @@ +plugins { + id 'java' + id 'org.springframework.boot' version '2.7.15' + id 'io.spring.dependency-management' version '1.0.15.RELEASE' + id 'java-test-fixtures' +} + +ext { + minJavaVersionForTests = JavaVersion.VERSION_17 +} + +apply from: "$rootDir/gradle/java.gradle" +description = 'SpringBoot Java 17 Smoke Tests.' + +repositories { + mavenCentral() +} + +dependencies { + implementation group: 'org.springframework.boot', name: 'spring-boot-starter-web', version: '2.2.0.RELEASE' + + testImplementation project(':dd-smoke-tests') + testImplementation testFixtures(project(":dd-smoke-tests:iast-util:iast-util-17")) + testImplementation testFixtures(project(':dd-smoke-tests:iast-util')) + + implementation project(':dd-smoke-tests:iast-util:iast-util-17') +} + +project.tasks.withType(AbstractCompile).configureEach { + setJavaVersion(it, 17) + sourceCompatibility = JavaVersion.VERSION_17 + targetCompatibility = JavaVersion.VERSION_17 + if (it instanceof JavaCompile) { + it.options.release.set(17) + } +} + +forbiddenApisMain { + failOnMissingClasses = false +} + +tasks.withType(Test).configureEach { + dependsOn "bootJar" + jvmArgs "-Ddatadog.smoketest.springboot.shadowJar.path=${tasks.bootJar.archiveFile.get()}" +} diff --git a/dd-smoke-tests/springboot-java-17/src/main/java/datadog/smoketest/springboot/SpringbootApplication.java b/dd-smoke-tests/springboot-java-17/src/main/java/datadog/smoketest/springboot/SpringbootApplication.java new file mode 100644 index 00000000000..03cc9791085 --- /dev/null +++ b/dd-smoke-tests/springboot-java-17/src/main/java/datadog/smoketest/springboot/SpringbootApplication.java @@ -0,0 +1,14 @@ +package datadog.smoketest.springboot; + +import java.lang.management.ManagementFactory; +import org.springframework.boot.SpringApplication; +import org.springframework.boot.autoconfigure.SpringBootApplication; + +@SpringBootApplication +public class SpringbootApplication { + + public static void main(final String[] args) { + SpringApplication.run(SpringbootApplication.class, args); + System.out.println("Started in " + ManagementFactory.getRuntimeMXBean().getUptime() + "ms"); + } +} diff --git a/dd-smoke-tests/springboot-java-17/src/test/groovy/datadog/smoketest/springboot/IastSpringBootSmokeTest.groovy b/dd-smoke-tests/springboot-java-17/src/test/groovy/datadog/smoketest/springboot/IastSpringBootSmokeTest.groovy new file mode 100644 index 00000000000..6435d526654 --- /dev/null +++ b/dd-smoke-tests/springboot-java-17/src/test/groovy/datadog/smoketest/springboot/IastSpringBootSmokeTest.groovy @@ -0,0 +1,6 @@ +package datadog.smoketest.springboot + +import datadog.smoketest.AbstractIast17SpringBootTest + +class IastSpringBootSmokeTest extends AbstractIast17SpringBootTest { +} 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 ebdd0fd7e70..2660b0cd53b 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 @@ -35,6 +35,8 @@ void onStringJoin( void onStringToUpperCase(@Nonnull String self, @Nullable String result); + void onStringTranslateEscapes(@Nonnull String self, @Nullable String result); + void onStringToLowerCase(@Nonnull String self, @Nullable String result); void onStringTrim(@Nonnull String self, @Nullable String result); diff --git a/settings.gradle b/settings.gradle index 9c2f9181340..a6d1c0a2090 100644 --- a/settings.gradle +++ b/settings.gradle @@ -143,6 +143,7 @@ include ':dd-smoke-tests:springboot' include ':dd-smoke-tests:springboot-freemarker' include ':dd-smoke-tests:springboot-grpc' include ':dd-smoke-tests:springboot-java-11' +include ':dd-smoke-tests:springboot-java-17' include ':dd-smoke-tests:springboot-jetty-jsp' include ':dd-smoke-tests:springboot-jpa' include ':dd-smoke-tests:springboot-mongo' @@ -168,6 +169,7 @@ include ':dd-smoke-tests:datastreams:kafkaschemaregistry' include ':dd-smoke-tests:iast-propagation' include ':dd-smoke-tests:iast-util' include ':dd-smoke-tests:iast-util:iast-util-11' +include ':dd-smoke-tests:iast-util:iast-util-17' // TODO this fails too often with a jgit failure, so disable until fixed //include ':dd-smoke-tests:debugger-integration-tests:latest-jdk-app' @@ -291,6 +293,7 @@ include ':dd-java-agent:instrumentation:java-io' include ':dd-java-agent:instrumentation:java-lang' include ':dd-java-agent:instrumentation:java-lang:java-lang-9' include ':dd-java-agent:instrumentation:java-lang:java-lang-11' +include ':dd-java-agent:instrumentation:java-lang:java-lang-15' include ':dd-java-agent:instrumentation:java-lang:java-lang-17' include ':dd-java-agent:instrumentation:java-net' include ':dd-java-agent:instrumentation:java-security'