From 9f1f33f5dedd4062fa7c5fefc9a5acfbaa94025e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20=C3=81lvarez=20=C3=81lvarez?= Date: Tue, 11 Jun 2024 16:28:24 +0200 Subject: [PATCH] Removed scala converter lambdas and ensure they are added as helpers (#7146) Removed scala converter lambdas and ensure they are added as helpers (cherry picked from commit 5294facf9556c4a3e4d6b7b854b358d19144fce6) --- .../scala/ScalaJavaConverters.java | 48 ++++++++++---- .../scala/StringContextCallSite.java | 8 ++- .../scala/StringOpsCallSite.java | 8 ++- dd-smoke-tests/iast-propagation/build.gradle | 38 ++++++++++++ .../src/main/groovy/GroovyPropagation.groovy | 18 ++++++ .../src/main/java/JavaPropagation.java | 19 ++++++ .../springboot/PropagationController.java | 55 ++++++++++++++++ .../springboot/SpringbootApplication.java | 12 ++++ .../src/main/kotlin/KotlinPropagation.kt | 10 +++ .../src/main/scala/ScalaPropagation.scala | 16 +++++ .../smoketest/IastPropagationSmokeTest.groovy | 62 +++++++++++++++++++ settings.gradle | 1 + 12 files changed, 280 insertions(+), 15 deletions(-) create mode 100644 dd-smoke-tests/iast-propagation/build.gradle create mode 100644 dd-smoke-tests/iast-propagation/src/main/groovy/GroovyPropagation.groovy create mode 100644 dd-smoke-tests/iast-propagation/src/main/java/JavaPropagation.java create mode 100644 dd-smoke-tests/iast-propagation/src/main/java/datadog/smoketest/springboot/PropagationController.java create mode 100644 dd-smoke-tests/iast-propagation/src/main/java/datadog/smoketest/springboot/SpringbootApplication.java create mode 100644 dd-smoke-tests/iast-propagation/src/main/kotlin/KotlinPropagation.kt create mode 100644 dd-smoke-tests/iast-propagation/src/main/scala/ScalaPropagation.scala create mode 100644 dd-smoke-tests/iast-propagation/src/test/groovy/datadog/smoketest/IastPropagationSmokeTest.groovy diff --git a/dd-java-agent/instrumentation/scala/src/main/java/datadog/trace/instrumentation/scala/ScalaJavaConverters.java b/dd-java-agent/instrumentation/scala/src/main/java/datadog/trace/instrumentation/scala/ScalaJavaConverters.java index 8946a2b9f8b..4d2113a7c03 100644 --- a/dd-java-agent/instrumentation/scala/src/main/java/datadog/trace/instrumentation/scala/ScalaJavaConverters.java +++ b/dd-java-agent/instrumentation/scala/src/main/java/datadog/trace/instrumentation/scala/ScalaJavaConverters.java @@ -7,19 +7,7 @@ public class ScalaJavaConverters { public static Iterable toIterable(@Nonnull final scala.collection.Iterable iterable) { - scala.collection.Iterator iterator = iterable.iterator(); - return () -> - new Iterator() { - @Override - public boolean hasNext() { - return iterator.hasNext(); - } - - @Override - public E next() { - return iterator.next(); - } - }; + return new JavaIterable<>(iterable); } public static Object[] toArray(@Nullable final scala.collection.Iterable iterable) { @@ -34,4 +22,38 @@ public static Object[] toArray(@Nullable final scala.collection.Iterable } return array; } + + public static class JavaIterable implements Iterable { + + private final scala.collection.Iterable iterable; + + private JavaIterable(final scala.collection.Iterable iterable) { + this.iterable = iterable; + } + + @Override + @Nonnull + public Iterator iterator() { + return new JavaIterator<>(iterable.iterator()); + } + } + + public static class JavaIterator implements Iterator { + + private final scala.collection.Iterator iterator; + + private JavaIterator(final scala.collection.Iterator iterator) { + this.iterator = iterator; + } + + @Override + public boolean hasNext() { + return iterator.hasNext(); + } + + @Override + public E next() { + return iterator.next(); + } + } } diff --git a/dd-java-agent/instrumentation/scala/src/main/java/datadog/trace/instrumentation/scala/StringContextCallSite.java b/dd-java-agent/instrumentation/scala/src/main/java/datadog/trace/instrumentation/scala/StringContextCallSite.java index a116b098314..8718021e02d 100644 --- a/dd-java-agent/instrumentation/scala/src/main/java/datadog/trace/instrumentation/scala/StringContextCallSite.java +++ b/dd-java-agent/instrumentation/scala/src/main/java/datadog/trace/instrumentation/scala/StringContextCallSite.java @@ -10,7 +10,13 @@ import scala.collection.Seq; @Propagation -@CallSite(spi = IastCallSites.class, helpers = ScalaJavaConverters.class) +@CallSite( + spi = IastCallSites.class, + helpers = { + ScalaJavaConverters.class, + ScalaJavaConverters.JavaIterable.class, + ScalaJavaConverters.JavaIterator.class + }) public class StringContextCallSite { @CallSite.After("java.lang.String scala.StringContext.s(scala.collection.Seq)") diff --git a/dd-java-agent/instrumentation/scala/src/main/java/datadog/trace/instrumentation/scala/StringOpsCallSite.java b/dd-java-agent/instrumentation/scala/src/main/java/datadog/trace/instrumentation/scala/StringOpsCallSite.java index 91ea3402e56..74322d9319d 100644 --- a/dd-java-agent/instrumentation/scala/src/main/java/datadog/trace/instrumentation/scala/StringOpsCallSite.java +++ b/dd-java-agent/instrumentation/scala/src/main/java/datadog/trace/instrumentation/scala/StringOpsCallSite.java @@ -9,7 +9,13 @@ import scala.collection.immutable.StringOps; @Propagation -@CallSite(spi = IastCallSites.class, helpers = ScalaJavaConverters.class) +@CallSite( + spi = IastCallSites.class, + helpers = { + ScalaJavaConverters.class, + ScalaJavaConverters.JavaIterable.class, + ScalaJavaConverters.JavaIterator.class + }) public class StringOpsCallSite { @CallSite.After( diff --git a/dd-smoke-tests/iast-propagation/build.gradle b/dd-smoke-tests/iast-propagation/build.gradle new file mode 100644 index 00000000000..b5ae2c23ff1 --- /dev/null +++ b/dd-smoke-tests/iast-propagation/build.gradle @@ -0,0 +1,38 @@ +plugins { + id 'com.github.johnrengelman.shadow' + id 'java' + id 'org.jetbrains.kotlin.jvm' version '2.0.0' + id 'scala' + id 'groovy' +} + +apply from: "$rootDir/gradle/java.gradle" +description = 'IAST propagation Smoke Tests.' + +// The standard spring-boot plugin doesn't play nice with our project +// so we'll build a fat jar instead +jar { + manifest { + attributes('Main-Class': 'datadog.smoketest.springboot.SpringbootApplication') + } +} + +shadowJar { + configurations = [project.configurations.runtimeClasspath] +} + +dependencies { + implementation project(':dd-trace-api') + implementation group: 'org.springframework.boot', name: 'spring-boot-starter-web', version: '2.5.4' + implementation deps.scala + implementation deps.groovy + implementation deps.kotlin + + testImplementation project(':dd-smoke-tests') + testImplementation(testFixtures(project(":dd-smoke-tests:iast-util"))) +} + +tasks.withType(Test).configureEach { + dependsOn "shadowJar" + jvmArgs "-Ddatadog.smoketest.springboot.shadowJar.path=${tasks.shadowJar.archiveFile.get()}" +} diff --git a/dd-smoke-tests/iast-propagation/src/main/groovy/GroovyPropagation.groovy b/dd-smoke-tests/iast-propagation/src/main/groovy/GroovyPropagation.groovy new file mode 100644 index 00000000000..c26b1ab6b88 --- /dev/null +++ b/dd-smoke-tests/iast-propagation/src/main/groovy/GroovyPropagation.groovy @@ -0,0 +1,18 @@ +import java.util.function.Supplier + +class GroovyPropagation implements Supplier> { + + @Override + List get() { + return ["plus", "interpolation"] + } + + String plus(String tainted) { + return tainted + tainted + } + + + String interpolation(String tainted) { + return "interpolation: $tainted" + } +} diff --git a/dd-smoke-tests/iast-propagation/src/main/java/JavaPropagation.java b/dd-smoke-tests/iast-propagation/src/main/java/JavaPropagation.java new file mode 100644 index 00000000000..e80eb1df605 --- /dev/null +++ b/dd-smoke-tests/iast-propagation/src/main/java/JavaPropagation.java @@ -0,0 +1,19 @@ +import java.util.Arrays; +import java.util.List; +import java.util.function.Supplier; + +public class JavaPropagation implements Supplier> { + + @Override + public List get() { + return Arrays.asList("plus", "concat"); + } + + public String plus(String tainted) { + return tainted + tainted; + } + + public String concat(String tainted) { + return tainted.concat(tainted); + } +} diff --git a/dd-smoke-tests/iast-propagation/src/main/java/datadog/smoketest/springboot/PropagationController.java b/dd-smoke-tests/iast-propagation/src/main/java/datadog/smoketest/springboot/PropagationController.java new file mode 100644 index 00000000000..3302b4916c5 --- /dev/null +++ b/dd-smoke-tests/iast-propagation/src/main/java/datadog/smoketest/springboot/PropagationController.java @@ -0,0 +1,55 @@ +package datadog.smoketest.springboot; + +import java.lang.reflect.Method; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Supplier; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.PathVariable; +import org.springframework.web.bind.annotation.RequestParam; +import org.springframework.web.bind.annotation.RestController; + +@RestController +public class PropagationController { + + private final Map>> suppliers = new ConcurrentHashMap<>(); + + @GetMapping("/{language}") + public List propagation(@PathVariable("language") final String language) { + return supplierForLanguage(language).get(); + } + + @GetMapping("/{language}/{method}") + public String propagation( + @PathVariable("language") final String language, + @PathVariable("method") final String method, + @RequestParam("param") final String param) { + return invokeMethod(supplierForLanguage(language), method, param); + } + + @SuppressWarnings("unchecked") + private Supplier> supplierForLanguage(final String language) { + return suppliers.computeIfAbsent( + language, + l -> { + try { + String name = Character.toUpperCase(l.charAt(0)) + l.substring(1) + "Propagation"; + Class instance = Thread.currentThread().getContextClassLoader().loadClass(name); + return (Supplier>) instance.newInstance(); + } catch (final Exception e) { + throw new RuntimeException(e); + } + }); + } + + private String invokeMethod( + final Supplier> supplier, final String name, final String param) { + try { + final Method method = supplier.getClass().getDeclaredMethod(name, String.class); + return (String) method.invoke(supplier, param); + } catch (Exception e) { + throw new RuntimeException(e); + } + } +} diff --git a/dd-smoke-tests/iast-propagation/src/main/java/datadog/smoketest/springboot/SpringbootApplication.java b/dd-smoke-tests/iast-propagation/src/main/java/datadog/smoketest/springboot/SpringbootApplication.java new file mode 100644 index 00000000000..c80e4cc2aa5 --- /dev/null +++ b/dd-smoke-tests/iast-propagation/src/main/java/datadog/smoketest/springboot/SpringbootApplication.java @@ -0,0 +1,12 @@ +package datadog.smoketest.springboot; + +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); + } +} diff --git a/dd-smoke-tests/iast-propagation/src/main/kotlin/KotlinPropagation.kt b/dd-smoke-tests/iast-propagation/src/main/kotlin/KotlinPropagation.kt new file mode 100644 index 00000000000..5b682833eab --- /dev/null +++ b/dd-smoke-tests/iast-propagation/src/main/kotlin/KotlinPropagation.kt @@ -0,0 +1,10 @@ +import java.util.function.Supplier + +class KotlinPropagation : Supplier> { + + override fun get(): List = listOf("plus", "interpolation") + + fun plus(param: String): String = param + param + + fun interpolation(param: String): String = "Interpolation $param" +} diff --git a/dd-smoke-tests/iast-propagation/src/main/scala/ScalaPropagation.scala b/dd-smoke-tests/iast-propagation/src/main/scala/ScalaPropagation.scala new file mode 100644 index 00000000000..07674d00325 --- /dev/null +++ b/dd-smoke-tests/iast-propagation/src/main/scala/ScalaPropagation.scala @@ -0,0 +1,16 @@ +import java.util +import java.util.function.Supplier +import scala.collection.JavaConverters._ + +class ScalaPropagation extends Supplier[util.List[String]] { + + override def get(): util.List[String] = List("plus", "s", "f", "raw").asJava + + def plus(param: String): String = param + param + + def s(param: String): String = s"s interpolation: $param" + + def f(param: String): String = f"f interpolation: $param" + + def raw(param: String): String = raw"raw interpolation: $param" +} diff --git a/dd-smoke-tests/iast-propagation/src/test/groovy/datadog/smoketest/IastPropagationSmokeTest.groovy b/dd-smoke-tests/iast-propagation/src/test/groovy/datadog/smoketest/IastPropagationSmokeTest.groovy new file mode 100644 index 00000000000..8d0f9e348e0 --- /dev/null +++ b/dd-smoke-tests/iast-propagation/src/test/groovy/datadog/smoketest/IastPropagationSmokeTest.groovy @@ -0,0 +1,62 @@ +package datadog.smoketest + +import groovy.json.JsonSlurper +import okhttp3.Request +import org.junit.Assume + +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 + +class IastPropagationSmokeTest extends AbstractIastServerSmokeTest { + + @Override + ProcessBuilder createProcessBuilder() { + final jarPath = System.getProperty('datadog.smoketest.springboot.shadowJar.path') + List command = [] + command.add(javaPath()) + command.addAll(defaultJavaProperties) + command.addAll((String[]) [ + withSystemProperty(IAST_ENABLED, true), + withSystemProperty(IAST_DETECTION_MODE, 'FULL'), + withSystemProperty(IAST_DEBUG_ENABLED, true), + '-jar', + jarPath, + "--server.port=${httpPort}" + ]) + final processBuilder = new ProcessBuilder(command) + processBuilder.directory(new File(buildDirectory)) + return processBuilder + } + + void 'test propagation language=#language, method=#method'() { + setup: + // TODO fix when we have groovy default string propagation + Assume.assumeTrue(language != 'groovy') + String param = "${language}_${method}" + String url = "http://localhost:${httpPort}/${language}/${method}?param=$param" + + when: + def request = new Request.Builder().url(url).get().build() + def response = client.newCall(request).execute() + + then: + def responseBodyStr = response.body().string() + response.code() == 200 + hasTainted { tainted -> + tainted.value == responseBodyStr && + tainted.ranges[0].source.origin == 'http.request.parameter' + } + + where: + [language, method] << methods() + } + + private List> methods() { + final languages = ['java', 'scala', 'groovy', 'kotlin'] + return languages.collectMany { language -> + final methods = new JsonSlurper().parse(new URL("http://localhost:${httpPort}/${language}")) as List + return methods.collect { method -> [language, method] } + } + } +} diff --git a/settings.gradle b/settings.gradle index e086eb13f38..86942f79a2f 100644 --- a/settings.gradle +++ b/settings.gradle @@ -154,6 +154,7 @@ include ':dd-smoke-tests:appsec:springboot-graphql' include ':dd-smoke-tests:appsec:springboot-security' include ':dd-smoke-tests:debugger-integration-tests' include ':dd-smoke-tests:datastreams:kafkaschemaregistry' +include ':dd-smoke-tests:iast-propagation' include ':dd-smoke-tests:iast-util' // TODO this fails too often with a jgit failure, so disable until fixed //include ':dd-smoke-tests:debugger-integration-tests:latest-jdk-app'