From fe010b36a77e1119cca6325cdabfe257a51d29fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20=C3=81lvarez=20=C3=81lvarez?= Date: Fri, 13 Sep 2024 13:41:39 +0200 Subject: [PATCH] Remove dependency with thread locals in jersey IAST instrumentation --- .../instrumentation/jersey/build.gradle | 3 + .../jersey2/JettyServer.groovy | 2 + .../jersey2/iast/IastJersey2JettyTest.groovy | 290 ++++++++++++++++++ .../jersey2/iast/IastResource.groovy | 107 +++++++ .../jersey3/JettyServer.groovy | 2 + .../jersey3/iast/IastJersey3JettyTest.groovy | 290 ++++++++++++++++++ .../jersey3/iast/IastResource.groovy | 107 +++++++ ...actParamValueExtractorInstrumentation.java | 62 ---- .../jersey/AbstractStringReaderAdvice.java | 28 -- .../AbstractStringReaderInstrumentation.java | 32 +- ...a => ContainerRequestInstrumentation.java} | 43 ++- ...ookieParamValueFactoryInstrumentation.java | 46 --- .../jersey/FormInstrumentation.java | 77 +++++ ...eaderParamValueFactoryInstrumentation.java | 46 --- .../InboundMessageContextInstrumentation.java | 105 +++++-- .../jersey/JerseyTaintHelper.java | 27 ++ ...ValueFactoryWithSourceInstrumentation.java | 67 ---- .../PathParamValueFactoryInstrumentation.java | 45 --- .../jersey/ThreadLocalSourceType.java | 16 - .../UriRoutingContextInstrumentation.java | 110 +++++++ .../jersey/AbstractStringReaderTest.groovy | 13 - .../smoketest/AbstractJerseySmokeTest.groovy | 2 +- 22 files changed, 1156 insertions(+), 364 deletions(-) create mode 100644 dd-java-agent/instrumentation/jersey/src/jersey2JettyTest/groovy/datadog/trace/instrumentation/jersey2/iast/IastJersey2JettyTest.groovy create mode 100644 dd-java-agent/instrumentation/jersey/src/jersey2JettyTest/groovy/datadog/trace/instrumentation/jersey2/iast/IastResource.groovy create mode 100644 dd-java-agent/instrumentation/jersey/src/jersey3JettyTest/groovy/datadog/trace/instrumentation/jersey3/iast/IastJersey3JettyTest.groovy create mode 100644 dd-java-agent/instrumentation/jersey/src/jersey3JettyTest/groovy/datadog/trace/instrumentation/jersey3/iast/IastResource.groovy delete mode 100644 dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/AbstractParamValueExtractorInstrumentation.java delete mode 100644 dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/AbstractStringReaderAdvice.java rename dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/{AbstractFormProviderInstrumentation.java => ContainerRequestInstrumentation.java} (62%) delete mode 100644 dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/CookieParamValueFactoryInstrumentation.java create mode 100644 dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/FormInstrumentation.java delete mode 100644 dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/HeaderParamValueFactoryInstrumentation.java create mode 100644 dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/JerseyTaintHelper.java delete mode 100644 dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/ParamValueFactoryWithSourceInstrumentation.java delete mode 100644 dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/PathParamValueFactoryInstrumentation.java delete mode 100644 dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/ThreadLocalSourceType.java create mode 100644 dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/UriRoutingContextInstrumentation.java delete mode 100644 dd-java-agent/instrumentation/jersey/src/test/groovy/datadog/trace/instrumentation/jersey/AbstractStringReaderTest.groovy diff --git a/dd-java-agent/instrumentation/jersey/build.gradle b/dd-java-agent/instrumentation/jersey/build.gradle index ef366ec044c..a52d4039b3f 100644 --- a/dd-java-agent/instrumentation/jersey/build.gradle +++ b/dd-java-agent/instrumentation/jersey/build.gradle @@ -39,6 +39,7 @@ def jersey2Version = '2.18' def jersey3Version = '3.1.2' dependencies { compileOnly group: 'org.glassfish.jersey.core', name: 'jersey-common', version: '2.0' + compileOnly group: 'org.glassfish.jersey.core', name: 'jersey-server', version: '2.0' testImplementation group: 'jakarta.ws.rs', name: 'jakarta.ws.rs-api', version: '3.0.0' testImplementation group: 'org.glassfish.jersey.core', name: 'jersey-common', version: jersey3Version @@ -47,6 +48,7 @@ dependencies { exclude group: 'org.eclipse.jetty', module: 'jetty-server' } jersey2JettyTestImplementation testFixtures(project(':dd-java-agent:appsec')) + jersey2JettyTestImplementation testFixtures(project(':dd-java-agent:agent-iast')) jersey2JettyTestImplementation group: 'org.glassfish.jersey.containers', name: 'jersey-container-jetty-http', version : jersey2Version jersey2JettyTestImplementation group: 'org.glassfish.jersey.media', name: 'jersey-media-multipart', version: jersey2Version jersey2JettyTestImplementation group: 'org.glassfish.jersey.media', name: 'jersey-media-json-jackson', version: jersey2Version @@ -59,6 +61,7 @@ dependencies { exclude group: 'org.eclipse.jetty', module: 'jetty-server' } jersey3JettyTestImplementation testFixtures(project(':dd-java-agent:appsec')) + jersey3JettyTestImplementation testFixtures(project(':dd-java-agent:agent-iast')) jersey3JettyTestImplementation group: 'org.glassfish.jersey.containers', name: 'jersey-container-jetty-http', version : jersey3Version jersey3JettyTestImplementation group: 'org.glassfish.jersey.media', name: 'jersey-media-multipart', version: jersey3Version jersey3JettyTestImplementation group: 'org.glassfish.jersey.media', name: 'jersey-media-json-jackson', version: jersey3Version diff --git a/dd-java-agent/instrumentation/jersey/src/jersey2JettyTest/groovy/datadog/trace/instrumentation/jersey2/JettyServer.groovy b/dd-java-agent/instrumentation/jersey/src/jersey2JettyTest/groovy/datadog/trace/instrumentation/jersey2/JettyServer.groovy index 78a87697e4f..bcf91675eec 100644 --- a/dd-java-agent/instrumentation/jersey/src/jersey2JettyTest/groovy/datadog/trace/instrumentation/jersey2/JettyServer.groovy +++ b/dd-java-agent/instrumentation/jersey/src/jersey2JettyTest/groovy/datadog/trace/instrumentation/jersey2/JettyServer.groovy @@ -1,6 +1,7 @@ package datadog.trace.instrumentation.jersey2 import datadog.trace.agent.test.base.HttpServer +import datadog.trace.instrumentation.jersey2.iast.IastResource import org.eclipse.jetty.server.Server import org.glassfish.jersey.jackson.JacksonFeature import org.glassfish.jersey.jetty.JettyHttpContainerFactory @@ -20,6 +21,7 @@ class JettyServer implements HttpServer { rc.register(ResponseServerFilter) rc.register(MultiPartFeature) rc.register(JacksonFeature) + rc.register(IastResource) server = JettyHttpContainerFactory.createServer(new URI("http://localhost:0"), rc, false) } diff --git a/dd-java-agent/instrumentation/jersey/src/jersey2JettyTest/groovy/datadog/trace/instrumentation/jersey2/iast/IastJersey2JettyTest.groovy b/dd-java-agent/instrumentation/jersey/src/jersey2JettyTest/groovy/datadog/trace/instrumentation/jersey2/iast/IastJersey2JettyTest.groovy new file mode 100644 index 00000000000..970d17c9273 --- /dev/null +++ b/dd-java-agent/instrumentation/jersey/src/jersey2JettyTest/groovy/datadog/trace/instrumentation/jersey2/iast/IastJersey2JettyTest.groovy @@ -0,0 +1,290 @@ +package datadog.trace.instrumentation.jersey2.iast + +import com.datadog.iast.test.IastRequestTestRunner +import datadog.trace.agent.test.base.HttpServer +import datadog.trace.api.iast.SourceTypes +import okhttp3.FormBody +import okhttp3.Request +import spock.lang.Shared + +import static org.hamcrest.Matchers.greaterThan + +class IastJersey2JettyTest extends IastRequestTestRunner { + + @Shared + HttpServer server + + void setupSpec() { + server = getClass().classLoader + .loadClass("datadog.trace.instrumentation.jersey2.JettyServer") + .newInstance([] as Object[]) as HttpServer + server.start() + } + + void cleanupSpec() { + server.stop() + } + + protected String buildUrl(String path) { + "${server.address()}$path" + } + + void 'path variable'() { + when: + String url = buildUrl 'iast/path/myValue' + def request = new Request.Builder().url(url).get().build() + def response = client.newCall(request).execute() + + then: + response.code() == 200 + response.body().string() == 'IAST: myValue (tainted)' + + when: + def toc = finReqTaintedObjects + + then: + toc.hasTaintedObject { + value 'myValue' + range 0, 7, source(SourceTypes.REQUEST_PATH_PARAMETER, 'name', 'myValue') + } + } + + void 'all path variables'() { + when: + String url = buildUrl 'iast/all_path/myValue' + def request = new Request.Builder().url(url).get().build() + def response = client.newCall(request).execute() + + then: + response.code() == 200 + response.body().string() == 'IAST: [[name:[myValue (tainted)]]]' + + when: + def toc = finReqTaintedObjects + + then: + toc.hasTaintedObject { + value 'myValue' + range 0, 7, source(SourceTypes.REQUEST_PATH_PARAMETER, 'name', 'myValue') + } + } + + void 'query param'() { + when: + String url = buildUrl 'iast/query?var=bar' + def request = new Request.Builder().url(url).get().build() + def response = client.newCall(request).execute() + + then: + response.code() == 200 + response.body().string() == 'IAST: bar (tainted)' + + when: + def toc = finReqTaintedObjects + + then: + toc.hasTaintedObject { + value 'bar' + range 0, 3, source(SourceTypes.REQUEST_PARAMETER_VALUE, 'var', 'bar') + } + } + + void 'all query params'() { + when: + String url = buildUrl 'iast/all_query?var1=foo&var1=bar&var2=a+b+c' + def request = new Request.Builder().url(url).get().build() + def response = client.newCall(request).execute() + + then: + response.code() == 200 + response.body().string() == 'IAST: [[var1 (tainted):[foo (tainted), bar (tainted)]], [var2 (tainted):[a b c (tainted)]]]' + + when: + def toc = finReqTaintedObjects + + then: + toc.hasTaintedObject { + value 'var1' + range 0, 4, source(SourceTypes.REQUEST_PARAMETER_NAME, 'var1', 'var1') + } + toc.hasTaintedObject { + value 'var2' + range 0, 4, source(SourceTypes.REQUEST_PARAMETER_NAME, 'var2', 'var2') + } + toc.hasTaintedObject { + value 'foo' + range 0, 3, source(SourceTypes.REQUEST_PARAMETER_VALUE, 'var1', 'foo') + } + toc.hasTaintedObject { + value 'bar' + range 0, 3, source(SourceTypes.REQUEST_PARAMETER_VALUE, 'var1', 'bar') + } + toc.hasTaintedObject { + value 'a b c' + range 0, 5, source(SourceTypes.REQUEST_PARAMETER_VALUE, 'var2', 'a b c') + } + } + + void 'form param'() { + when: + String url = buildUrl 'iast/form' + def body = new FormBody.Builder() + .add('var', 'bar') + .build() + def request = new Request.Builder().url(url).post(body).build() + def response = client.newCall(request).execute() + + then: + response.code() == 200 + response.body().string() == 'IAST: bar (tainted)' + + when: + def toc = finReqTaintedObjects + + then: + toc.hasTaintedObject { + value 'bar' + range 0, 3, source(SourceTypes.REQUEST_PARAMETER_VALUE, 'var', 'bar') + } + } + + void 'all form params'() { + when: + String url = buildUrl 'iast/all_form' + def body = new FormBody.Builder() + .add('var1', 'foo') + .add('var1', 'bar') + .add('var2', 'a b c') + .build() + def request = new Request.Builder().url(url).post(body).build() + def response = client.newCall(request).execute() + + then: + response.code() == 200 + response.body().string() == 'IAST: [[var1 (tainted):[foo (tainted), bar (tainted)]], [var2 (tainted):[a b c (tainted)]]]' + + when: + def toc = finReqTaintedObjects + + then: + toc.hasTaintedObject { + value 'var1' + range 0, 4, source(SourceTypes.REQUEST_PARAMETER_NAME, 'var1', 'var1') + } + toc.hasTaintedObject { + value 'var2' + range 0, 4, source(SourceTypes.REQUEST_PARAMETER_NAME, 'var2', 'var2') + } + toc.hasTaintedObject { + value 'foo' + range 0, 3, source(SourceTypes.REQUEST_PARAMETER_VALUE, 'var1', 'foo') + } + toc.hasTaintedObject { + value 'bar' + range 0, 3, source(SourceTypes.REQUEST_PARAMETER_VALUE, 'var1', 'bar') + } + toc.hasTaintedObject { + value 'a b c' + range 0, 5, source(SourceTypes.REQUEST_PARAMETER_VALUE, 'var2', 'a b c') + } + } + + void 'cookie'() { + when: + String url = buildUrl "iast/cookie" + def request = new Request.Builder() + .url(url) + .addHeader('Cookie', 'var1=bar') + .get().build() + def response = client.newCall(request).execute() + + then: + response.code() == 200 + response.body().string() == 'IAST: bar (tainted)' + + when: + def toc = finReqTaintedObjects + + then: + toc.hasTaintedObject { + value 'bar' + range 0, 3, source(SourceTypes.REQUEST_COOKIE_VALUE, 'var1', 'bar') + } + } + + void 'all cookies'() { + when: + String url = buildUrl "iast/all_cookies" + def request = new Request.Builder() + .url(url) + .addHeader('Cookie', 'var1=foo') + .get().build() + def response = client.newCall(request).execute() + + then: + response.code() == 200 + // cookie names are not tainted + response.body().string() == 'IAST: [var1 (tainted):foo (tainted)]' + + when: + def toc = finReqTaintedObjects + + then: + toc.hasTaintedObject { + value 'var1' + range 0, 4, source(SourceTypes.REQUEST_COOKIE_NAME, 'var1', 'var1') + } + toc.hasTaintedObject { + value 'foo' + range 0, 3, source(SourceTypes.REQUEST_COOKIE_VALUE, 'var1', 'foo') + } + } + + void 'header'() { + when: + String url = buildUrl 'iast/header' + def request = new Request.Builder() + .url(url) + .addHeader('X-My-Header', 'bar') + .get().build() + def response = client.newCall(request).execute() + + then: + response.code() == 200 + response.body().string() == 'IAST: bar (tainted)' + + when: + def toc = finReqTaintedObjects + + then: + toc.hasTaintedObject { + value 'bar' + range 0, 3, source(SourceTypes.REQUEST_HEADER_VALUE, 'X-My-Header', 'bar') + } + } + + void 'all headers'() { + when: + String url = buildUrl "iast/all_headers" + def request = new Request.Builder().url(url).get().build() + def response = client.newCall(request).execute() + String body = response.body().string() + + then: + response.code() == 200 + body.matches(/.*User-Agent \(tainted\):\[okhttp\/[\d.]+ \(tainted\)].*/) + + when: + def toc = finReqTaintedObjects + + then: + toc.hasTaintedObject { + value 'User-Agent' + range 0, 10, source(SourceTypes.REQUEST_HEADER_NAME, 'User-Agent', 'User-Agent') + } + toc.hasTaintedObject { + value ~/okhttp\/[\d.]+/ + range 0, greaterThan(7), source(SourceTypes.REQUEST_HEADER_VALUE, 'User-Agent', ~/okhttp\/[\d.]+/) + } + } +} diff --git a/dd-java-agent/instrumentation/jersey/src/jersey2JettyTest/groovy/datadog/trace/instrumentation/jersey2/iast/IastResource.groovy b/dd-java-agent/instrumentation/jersey/src/jersey2JettyTest/groovy/datadog/trace/instrumentation/jersey2/iast/IastResource.groovy new file mode 100644 index 00000000000..281a3b027c3 --- /dev/null +++ b/dd-java-agent/instrumentation/jersey/src/jersey2JettyTest/groovy/datadog/trace/instrumentation/jersey2/iast/IastResource.groovy @@ -0,0 +1,107 @@ +package datadog.trace.instrumentation.jersey2.iast + +import javax.ws.rs.CookieParam +import javax.ws.rs.FormParam +import javax.ws.rs.GET +import javax.ws.rs.HeaderParam +import javax.ws.rs.POST +import javax.ws.rs.Path +import javax.ws.rs.PathParam +import javax.ws.rs.Produces +import javax.ws.rs.QueryParam +import javax.ws.rs.core.Context +import javax.ws.rs.core.Form +import javax.ws.rs.core.HttpHeaders +import javax.ws.rs.core.MediaType +import javax.ws.rs.core.MultivaluedMap +import javax.ws.rs.core.UriInfo + +import static com.datadog.iast.test.TaintMarkerHelpers.t + +@Path("/iast") +class IastResource { + + @Path("/path/{name}") + @GET + @Produces(MediaType.TEXT_PLAIN) + String path(@PathParam("name") String value) { + return "IAST: ${t(value)}" + } + + @Path("/all_path/{name}") + @GET + @Produces(MediaType.TEXT_PLAIN) + String allPath(@Context UriInfo uri) { + def pairList = collectMultiMap(uri.pathParameters) + return "IAST: ${t(pairList)}" + } + + @Path("/query") + @GET + @Produces(MediaType.TEXT_PLAIN) + String query(@QueryParam("var") String value) { + return "IAST: ${t(value)}" + } + + @Path("/all_query") + @GET + @Produces(MediaType.TEXT_PLAIN) + String allQuery(@Context UriInfo uri) { + def pairList = collectMultiMap(uri.queryParameters) + return "IAST: ${t(pairList)}" + } + + @Path("/header") + @GET + @Produces(MediaType.TEXT_PLAIN) + String header(@HeaderParam("x-my-header") String value) { + return "IAST: ${t(value)}" + } + + @Path("/all_headers") + @GET + @Produces(MediaType.TEXT_PLAIN) + String allHeaders(@Context final HttpHeaders headers) { + def pairList = collectMultiMap(headers.requestHeaders) + return "IAST: ${pairList}" + } + + @Path("/cookie") + @GET + @Produces(MediaType.TEXT_PLAIN) + String cookie(@CookieParam("var1") String value) { + return "IAST: ${t(value)}" + } + + @Path("/all_cookies") + @GET + @Produces(MediaType.TEXT_PLAIN) + String allCookies(@Context final HttpHeaders headers) { + def pairList = headers.cookies.values().collectEntries { cookie -> + [(t(cookie.name)): t(cookie.value)] + } + return "IAST: ${pairList}" + } + + @Path("/form") + @POST + @Produces(MediaType.TEXT_PLAIN) + String form(@FormParam("var") String value) { + return "IAST: ${t(value)}" + } + + @Path("/all_form") + @POST + @Produces(MediaType.TEXT_PLAIN) + String form(final Form form) { + def pairList = collectMultiMap(form.asMap()) + return "IAST: ${pairList}" + } + + private static collectMultiMap(final MultivaluedMap map) { + return map.keySet().sort().collect {key -> + final values = map[key] + return [(t(key)): values.collect { t(it) }] + } + } +} diff --git a/dd-java-agent/instrumentation/jersey/src/jersey3JettyTest/groovy/datadog/trace/instrumentation/jersey3/JettyServer.groovy b/dd-java-agent/instrumentation/jersey/src/jersey3JettyTest/groovy/datadog/trace/instrumentation/jersey3/JettyServer.groovy index f3b6c6dbb6f..9ccbe059499 100644 --- a/dd-java-agent/instrumentation/jersey/src/jersey3JettyTest/groovy/datadog/trace/instrumentation/jersey3/JettyServer.groovy +++ b/dd-java-agent/instrumentation/jersey/src/jersey3JettyTest/groovy/datadog/trace/instrumentation/jersey3/JettyServer.groovy @@ -1,6 +1,7 @@ package datadog.trace.instrumentation.jersey3 import datadog.trace.agent.test.base.HttpServer +import datadog.trace.instrumentation.jersey3.iast.IastResource import org.eclipse.jetty.server.Server import org.glassfish.jersey.jackson.JacksonFeature import org.glassfish.jersey.jetty.JettyHttpContainerFactory @@ -20,6 +21,7 @@ class JettyServer implements HttpServer { rc.register(ResponseServerFilter) rc.register(MultiPartFeature) rc.register(JacksonFeature) + rc.register(IastResource) server = JettyHttpContainerFactory.createServer(new URI("http://localhost:0"), rc, false) } diff --git a/dd-java-agent/instrumentation/jersey/src/jersey3JettyTest/groovy/datadog/trace/instrumentation/jersey3/iast/IastJersey3JettyTest.groovy b/dd-java-agent/instrumentation/jersey/src/jersey3JettyTest/groovy/datadog/trace/instrumentation/jersey3/iast/IastJersey3JettyTest.groovy new file mode 100644 index 00000000000..44ee3881e06 --- /dev/null +++ b/dd-java-agent/instrumentation/jersey/src/jersey3JettyTest/groovy/datadog/trace/instrumentation/jersey3/iast/IastJersey3JettyTest.groovy @@ -0,0 +1,290 @@ +package datadog.trace.instrumentation.jersey3.iast + +import com.datadog.iast.test.IastRequestTestRunner +import datadog.trace.agent.test.base.HttpServer +import datadog.trace.api.iast.SourceTypes +import okhttp3.FormBody +import okhttp3.Request +import spock.lang.Shared + +import static org.hamcrest.Matchers.greaterThan + +class IastJersey3JettyTest extends IastRequestTestRunner { + + @Shared + HttpServer server + + void setupSpec() { + server = getClass().classLoader + .loadClass("datadog.trace.instrumentation.jersey3.JettyServer") + .newInstance([] as Object[]) as HttpServer + server.start() + } + + void cleanupSpec() { + server.stop() + } + + protected String buildUrl(String path) { + "${server.address()}$path" + } + + void 'path variable'() { + when: + String url = buildUrl 'iast/path/myValue' + def request = new Request.Builder().url(url).get().build() + def response = client.newCall(request).execute() + + then: + response.code() == 200 + response.body().string() == 'IAST: myValue (tainted)' + + when: + def toc = finReqTaintedObjects + + then: + toc.hasTaintedObject { + value 'myValue' + range 0, 7, source(SourceTypes.REQUEST_PATH_PARAMETER, 'name', 'myValue') + } + } + + void 'all path variables'() { + when: + String url = buildUrl 'iast/all_path/myValue' + def request = new Request.Builder().url(url).get().build() + def response = client.newCall(request).execute() + + then: + response.code() == 200 + response.body().string() == 'IAST: [[name:[myValue (tainted)]]]' + + when: + def toc = finReqTaintedObjects + + then: + toc.hasTaintedObject { + value 'myValue' + range 0, 7, source(SourceTypes.REQUEST_PATH_PARAMETER, 'name', 'myValue') + } + } + + void 'query param'() { + when: + String url = buildUrl 'iast/query?var=bar' + def request = new Request.Builder().url(url).get().build() + def response = client.newCall(request).execute() + + then: + response.code() == 200 + response.body().string() == 'IAST: bar (tainted)' + + when: + def toc = finReqTaintedObjects + + then: + toc.hasTaintedObject { + value 'bar' + range 0, 3, source(SourceTypes.REQUEST_PARAMETER_VALUE, 'var', 'bar') + } + } + + void 'all query params'() { + when: + String url = buildUrl 'iast/all_query?var1=foo&var1=bar&var2=a+b+c' + def request = new Request.Builder().url(url).get().build() + def response = client.newCall(request).execute() + + then: + response.code() == 200 + response.body().string() == 'IAST: [[var1 (tainted):[foo (tainted), bar (tainted)]], [var2 (tainted):[a b c (tainted)]]]' + + when: + def toc = finReqTaintedObjects + + then: + toc.hasTaintedObject { + value 'var1' + range 0, 4, source(SourceTypes.REQUEST_PARAMETER_NAME, 'var1', 'var1') + } + toc.hasTaintedObject { + value 'var2' + range 0, 4, source(SourceTypes.REQUEST_PARAMETER_NAME, 'var2', 'var2') + } + toc.hasTaintedObject { + value 'foo' + range 0, 3, source(SourceTypes.REQUEST_PARAMETER_VALUE, 'var1', 'foo') + } + toc.hasTaintedObject { + value 'bar' + range 0, 3, source(SourceTypes.REQUEST_PARAMETER_VALUE, 'var1', 'bar') + } + toc.hasTaintedObject { + value 'a b c' + range 0, 5, source(SourceTypes.REQUEST_PARAMETER_VALUE, 'var2', 'a b c') + } + } + + void 'form param'() { + when: + String url = buildUrl 'iast/form' + def body = new FormBody.Builder() + .add('var', 'bar') + .build() + def request = new Request.Builder().url(url).post(body).build() + def response = client.newCall(request).execute() + + then: + response.code() == 200 + response.body().string() == 'IAST: bar (tainted)' + + when: + def toc = finReqTaintedObjects + + then: + toc.hasTaintedObject { + value 'bar' + range 0, 3, source(SourceTypes.REQUEST_PARAMETER_VALUE, 'var', 'bar') + } + } + + void 'all form params'() { + when: + String url = buildUrl 'iast/all_form' + def body = new FormBody.Builder() + .add('var1', 'foo') + .add('var1', 'bar') + .add('var2', 'a b c') + .build() + def request = new Request.Builder().url(url).post(body).build() + def response = client.newCall(request).execute() + + then: + response.code() == 200 + response.body().string() == 'IAST: [[var1 (tainted):[foo (tainted), bar (tainted)]], [var2 (tainted):[a b c (tainted)]]]' + + when: + def toc = finReqTaintedObjects + + then: + toc.hasTaintedObject { + value 'var1' + range 0, 4, source(SourceTypes.REQUEST_PARAMETER_NAME, 'var1', 'var1') + } + toc.hasTaintedObject { + value 'var2' + range 0, 4, source(SourceTypes.REQUEST_PARAMETER_NAME, 'var2', 'var2') + } + toc.hasTaintedObject { + value 'foo' + range 0, 3, source(SourceTypes.REQUEST_PARAMETER_VALUE, 'var1', 'foo') + } + toc.hasTaintedObject { + value 'bar' + range 0, 3, source(SourceTypes.REQUEST_PARAMETER_VALUE, 'var1', 'bar') + } + toc.hasTaintedObject { + value 'a b c' + range 0, 5, source(SourceTypes.REQUEST_PARAMETER_VALUE, 'var2', 'a b c') + } + } + + void 'cookie'() { + when: + String url = buildUrl "iast/cookie" + def request = new Request.Builder() + .url(url) + .addHeader('Cookie', 'var1=bar') + .get().build() + def response = client.newCall(request).execute() + + then: + response.code() == 200 + response.body().string() == 'IAST: bar (tainted)' + + when: + def toc = finReqTaintedObjects + + then: + toc.hasTaintedObject { + value 'bar' + range 0, 3, source(SourceTypes.REQUEST_COOKIE_VALUE, 'var1', 'bar') + } + } + + void 'all cookies'() { + when: + String url = buildUrl "iast/all_cookies" + def request = new Request.Builder() + .url(url) + .addHeader('Cookie', 'var1=foo') + .get().build() + def response = client.newCall(request).execute() + + then: + response.code() == 200 + // cookie names are not tainted + response.body().string() == 'IAST: [var1 (tainted):foo (tainted)]' + + when: + def toc = finReqTaintedObjects + + then: + toc.hasTaintedObject { + value 'var1' + range 0, 4, source(SourceTypes.REQUEST_COOKIE_NAME, 'var1', 'var1') + } + toc.hasTaintedObject { + value 'foo' + range 0, 3, source(SourceTypes.REQUEST_COOKIE_VALUE, 'var1', 'foo') + } + } + + void 'header'() { + when: + String url = buildUrl 'iast/header' + def request = new Request.Builder() + .url(url) + .addHeader('X-My-Header', 'bar') + .get().build() + def response = client.newCall(request).execute() + + then: + response.code() == 200 + response.body().string() == 'IAST: bar (tainted)' + + when: + def toc = finReqTaintedObjects + + then: + toc.hasTaintedObject { + value 'bar' + range 0, 3, source(SourceTypes.REQUEST_HEADER_VALUE, 'X-My-Header', 'bar') + } + } + + void 'all headers'() { + when: + String url = buildUrl "iast/all_headers" + def request = new Request.Builder().url(url).get().build() + def response = client.newCall(request).execute() + String body = response.body().string() + + then: + response.code() == 200 + body.matches(/.*User-Agent \(tainted\):\[okhttp\/[\d.]+ \(tainted\)].*/) + + when: + def toc = finReqTaintedObjects + + then: + toc.hasTaintedObject { + value 'User-Agent' + range 0, 10, source(SourceTypes.REQUEST_HEADER_NAME, 'User-Agent', 'User-Agent') + } + toc.hasTaintedObject { + value ~/okhttp\/[\d.]+/ + range 0, greaterThan(7), source(SourceTypes.REQUEST_HEADER_VALUE, 'User-Agent', ~/okhttp\/[\d.]+/) + } + } +} diff --git a/dd-java-agent/instrumentation/jersey/src/jersey3JettyTest/groovy/datadog/trace/instrumentation/jersey3/iast/IastResource.groovy b/dd-java-agent/instrumentation/jersey/src/jersey3JettyTest/groovy/datadog/trace/instrumentation/jersey3/iast/IastResource.groovy new file mode 100644 index 00000000000..8e29ffe72be --- /dev/null +++ b/dd-java-agent/instrumentation/jersey/src/jersey3JettyTest/groovy/datadog/trace/instrumentation/jersey3/iast/IastResource.groovy @@ -0,0 +1,107 @@ +package datadog.trace.instrumentation.jersey3.iast + +import jakarta.ws.rs.CookieParam +import jakarta.ws.rs.FormParam +import jakarta.ws.rs.GET +import jakarta.ws.rs.HeaderParam +import jakarta.ws.rs.POST +import jakarta.ws.rs.Path +import jakarta.ws.rs.PathParam +import jakarta.ws.rs.Produces +import jakarta.ws.rs.QueryParam +import jakarta.ws.rs.core.Context +import jakarta.ws.rs.core.Form +import jakarta.ws.rs.core.HttpHeaders +import jakarta.ws.rs.core.MediaType +import jakarta.ws.rs.core.MultivaluedMap +import jakarta.ws.rs.core.UriInfo + +import static com.datadog.iast.test.TaintMarkerHelpers.t + +@Path("/iast") +class IastResource { + + @Path("/path/{name}") + @GET + @Produces(MediaType.TEXT_PLAIN) + String path(@PathParam("name") String value) { + return "IAST: ${t(value)}" + } + + @Path("/all_path/{name}") + @GET + @Produces(MediaType.TEXT_PLAIN) + String allPath(@Context UriInfo uri) { + def pairList = collectMultiMap(uri.pathParameters) + return "IAST: ${t(pairList)}" + } + + @Path("/query") + @GET + @Produces(MediaType.TEXT_PLAIN) + String query(@QueryParam("var") String value) { + return "IAST: ${t(value)}" + } + + @Path("/all_query") + @GET + @Produces(MediaType.TEXT_PLAIN) + String allQuery(@Context UriInfo uri) { + def pairList = collectMultiMap(uri.queryParameters) + return "IAST: ${t(pairList)}" + } + + @Path("/header") + @GET + @Produces(MediaType.TEXT_PLAIN) + String header(@HeaderParam("x-my-header") String value) { + return "IAST: ${t(value)}" + } + + @Path("/all_headers") + @GET + @Produces(MediaType.TEXT_PLAIN) + String allHeaders(@Context final HttpHeaders headers) { + def pairList = collectMultiMap(headers.requestHeaders) + return "IAST: ${pairList}" + } + + @Path("/cookie") + @GET + @Produces(MediaType.TEXT_PLAIN) + String cookie(@CookieParam("var1") String value) { + return "IAST: ${t(value)}" + } + + @Path("/all_cookies") + @GET + @Produces(MediaType.TEXT_PLAIN) + String allCookies(@Context final HttpHeaders headers) { + def pairList = headers.cookies.values().collectEntries { cookie -> + [(t(cookie.name)): t(cookie.value)] + } + return "IAST: ${pairList}" + } + + @Path("/form") + @POST + @Produces(MediaType.TEXT_PLAIN) + String form(@FormParam("var") String value) { + return "IAST: ${t(value)}" + } + + @Path("/all_form") + @POST + @Produces(MediaType.TEXT_PLAIN) + String form(final Form form) { + def pairList = collectMultiMap(form.asMap()) + return "IAST: ${pairList}" + } + + private static collectMultiMap(final MultivaluedMap map) { + return map.keySet().sort().collect {key -> + final values = map[key] + return [(t(key)): values.collect { t(it) }] + } + } +} diff --git a/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/AbstractParamValueExtractorInstrumentation.java b/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/AbstractParamValueExtractorInstrumentation.java deleted file mode 100644 index c98a917c554..00000000000 --- a/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/AbstractParamValueExtractorInstrumentation.java +++ /dev/null @@ -1,62 +0,0 @@ -package datadog.trace.instrumentation.jersey; - -import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; -import static net.bytebuddy.matcher.ElementMatchers.isProtected; -import static net.bytebuddy.matcher.ElementMatchers.takesArguments; - -import com.google.auto.service.AutoService; -import datadog.trace.advice.ActiveRequestContext; -import datadog.trace.advice.RequiresRequestContext; -import datadog.trace.agent.tooling.Instrumenter; -import datadog.trace.agent.tooling.InstrumenterModule; -import datadog.trace.api.gateway.RequestContext; -import datadog.trace.api.gateway.RequestContextSlot; -import datadog.trace.api.iast.IastContext; -import datadog.trace.api.iast.InstrumentationBridge; -import datadog.trace.api.iast.Propagation; -import datadog.trace.api.iast.propagation.PropagationModule; -import net.bytebuddy.asm.Advice; - -@AutoService(InstrumenterModule.class) -public class AbstractParamValueExtractorInstrumentation extends InstrumenterModule.Iast - implements Instrumenter.ForSingleType { - - public AbstractParamValueExtractorInstrumentation() { - super("jersey"); - } - - @Override - public void methodAdvice(MethodTransformer transformer) { - transformer.applyAdvice( - named("fromString").and(isProtected().and(takesArguments(String.class))), - AbstractParamValueExtractorInstrumentation.class.getName() + "$InstrumenterAdvice"); - } - - @Override - public String[] helperClassNames() { - return new String[] {packageName + ".ThreadLocalSourceType"}; - } - - @Override - public String instrumentedType() { - return "org.glassfish.jersey.server.internal.inject.AbstractParamValueExtractor"; - } - - @RequiresRequestContext(RequestContextSlot.IAST) - public static class InstrumenterAdvice { - @Advice.OnMethodExit(suppress = Throwable.class) - @Propagation - public static void onExit( - @Advice.Return Object result, - @Advice.FieldValue("parameterName") String parameterName, - @ActiveRequestContext RequestContext reqCtx) { - if (result instanceof String) { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null) { - IastContext ctx = reqCtx.getData(RequestContextSlot.IAST); - module.taintString(ctx, (String) result, ThreadLocalSourceType.get(), parameterName); - } - } - } - } -} diff --git a/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/AbstractStringReaderAdvice.java b/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/AbstractStringReaderAdvice.java deleted file mode 100644 index 1a07e1103c9..00000000000 --- a/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/AbstractStringReaderAdvice.java +++ /dev/null @@ -1,28 +0,0 @@ -package datadog.trace.instrumentation.jersey; - -import datadog.trace.advice.ActiveRequestContext; -import datadog.trace.advice.RequiresRequestContext; -import datadog.trace.api.gateway.RequestContext; -import datadog.trace.api.gateway.RequestContextSlot; -import datadog.trace.api.iast.IastContext; -import datadog.trace.api.iast.InstrumentationBridge; -import datadog.trace.api.iast.Source; -import datadog.trace.api.iast.SourceTypes; -import datadog.trace.api.iast.propagation.PropagationModule; -import net.bytebuddy.asm.Advice; - -@RequiresRequestContext(RequestContextSlot.IAST) -public class AbstractStringReaderAdvice { - @Advice.OnMethodExit(suppress = Throwable.class) - @Source(SourceTypes.REQUEST_PARAMETER_VALUE) - public static void onExit( - @Advice.Return Object result, @ActiveRequestContext RequestContext reqCtx) { - if (result instanceof String) { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null) { - IastContext ctx = reqCtx.getData(RequestContextSlot.IAST); - module.taintString(ctx, (String) result, SourceTypes.REQUEST_PARAMETER_VALUE); - } - } - } -} diff --git a/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/AbstractStringReaderInstrumentation.java b/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/AbstractStringReaderInstrumentation.java index d3a3794d455..8c71bf95455 100644 --- a/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/AbstractStringReaderInstrumentation.java +++ b/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/AbstractStringReaderInstrumentation.java @@ -5,8 +5,18 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import com.google.auto.service.AutoService; +import datadog.trace.advice.ActiveRequestContext; +import datadog.trace.advice.RequiresRequestContext; import datadog.trace.agent.tooling.Instrumenter; import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.api.iast.IastContext; +import datadog.trace.api.iast.InstrumentationBridge; +import datadog.trace.api.iast.Source; +import datadog.trace.api.iast.SourceTypes; +import datadog.trace.api.iast.propagation.PropagationModule; +import net.bytebuddy.asm.Advice; @AutoService(InstrumenterModule.class) public class AbstractStringReaderInstrumentation extends InstrumenterModule.Iast @@ -20,7 +30,7 @@ public AbstractStringReaderInstrumentation() { public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( named("fromString").and(isPublic().and(takesArguments(String.class))), - packageName + ".AbstractStringReaderAdvice"); + getClass().getName() + "$FromStringAdvice"); } @Override @@ -30,4 +40,24 @@ public String[] knownMatchingTypes() { "org.glassfish.jersey.server.internal.inject.ParamConverters$AbstractStringReader" }; } + + @RequiresRequestContext(RequestContextSlot.IAST) + public static class FromStringAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Source(SourceTypes.REQUEST_PARAMETER_VALUE) + public static void onExit( + @Advice.Argument(0) final String param, + @Advice.Return Object result, + @ActiveRequestContext RequestContext reqCtx) { + if (!(result instanceof String)) { + return; + } + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module == null) { + return; + } + IastContext ctx = reqCtx.getData(RequestContextSlot.IAST); + module.taintStringIfTainted(ctx, (String) result, param); + } + } } diff --git a/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/AbstractFormProviderInstrumentation.java b/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/ContainerRequestInstrumentation.java similarity index 62% rename from dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/AbstractFormProviderInstrumentation.java rename to dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/ContainerRequestInstrumentation.java index 2303e70e123..0c84b867020 100644 --- a/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/AbstractFormProviderInstrumentation.java +++ b/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/ContainerRequestInstrumentation.java @@ -16,49 +16,60 @@ import datadog.trace.api.iast.Source; import datadog.trace.api.iast.SourceTypes; import datadog.trace.api.iast.propagation.PropagationModule; -import java.util.List; -import java.util.Map; import net.bytebuddy.asm.Advice; @AutoService(InstrumenterModule.class) -public class AbstractFormProviderInstrumentation extends InstrumenterModule.Iast +public class ContainerRequestInstrumentation extends InstrumenterModule.Iast implements Instrumenter.ForSingleType { - public AbstractFormProviderInstrumentation() { + public ContainerRequestInstrumentation() { super("jersey"); } @Override public void methodAdvice(MethodTransformer transformer) { + String baseName = ContainerRequestInstrumentation.class.getName(); transformer.applyAdvice( - named("readFrom").and(isPublic()).and(takesArguments(4)), - AbstractFormProviderInstrumentation.class.getName() + "$InstrumenterAdvice"); + named("setProperty").and(isPublic()).and(takesArguments(String.class, Object.class)), + baseName + "$SetPropertyAdvice"); } @Override public String instrumentedType() { - return "org.glassfish.jersey.message.internal.AbstractFormProvider"; + return "org.glassfish.jersey.server.ContainerRequest"; + } + + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".JerseyTaintHelper", + }; } @RequiresRequestContext(RequestContextSlot.IAST) - public static class InstrumenterAdvice { + public static class SetPropertyAdvice { @Advice.OnMethodExit(suppress = Throwable.class) @Source(SourceTypes.REQUEST_PARAMETER_VALUE) public static void onExit( - @Advice.Return Map> result, + @Advice.Argument(0) String name, + @Advice.Argument(1) Object value, @ActiveRequestContext RequestContext reqCtx) { + + if (!"jersey.config.server.representation.decoded.form".equals(name) + && !"jersey.config.server.representation.form".equals(name)) { + return; + } + final PropagationModule prop = InstrumentationBridge.PROPAGATION; - if (prop == null || result == null || result.isEmpty()) { + if (prop == null) { return; } + final IastContext ctx = reqCtx.getData(RequestContextSlot.IAST); - for (Map.Entry> entry : result.entrySet()) { - final String name = entry.getKey(); - prop.taintString(ctx, name, SourceTypes.REQUEST_PARAMETER_NAME, name); - for (String value : entry.getValue()) { - prop.taintString(ctx, value, SourceTypes.REQUEST_PARAMETER_VALUE, name); - } + if (prop.isTainted(ctx, value)) { + return; } + prop.taintObject(ctx, value, SourceTypes.REQUEST_BODY); } } } diff --git a/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/CookieParamValueFactoryInstrumentation.java b/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/CookieParamValueFactoryInstrumentation.java deleted file mode 100644 index 1964fe4dc4a..00000000000 --- a/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/CookieParamValueFactoryInstrumentation.java +++ /dev/null @@ -1,46 +0,0 @@ -package datadog.trace.instrumentation.jersey; - -import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; -import static net.bytebuddy.matcher.ElementMatchers.isPublic; -import static net.bytebuddy.matcher.ElementMatchers.takesArguments; - -import com.google.auto.service.AutoService; -import datadog.trace.agent.tooling.Instrumenter; -import datadog.trace.agent.tooling.InstrumenterModule; -import datadog.trace.api.iast.Source; -import datadog.trace.api.iast.SourceTypes; -import net.bytebuddy.asm.Advice; - -@AutoService(InstrumenterModule.class) -public class CookieParamValueFactoryInstrumentation extends InstrumenterModule.Iast - implements Instrumenter.ForSingleType { - - public CookieParamValueFactoryInstrumentation() { - super("jersey"); - } - - @Override - public void methodAdvice(MethodTransformer transformer) { - transformer.applyAdvice( - named("get").and(isPublic()).and(takesArguments(1)), - CookieParamValueFactoryInstrumentation.class.getName() + "$InstrumenterAdvice"); - } - - @Override - public String[] helperClassNames() { - return new String[] {packageName + ".ThreadLocalSourceType"}; - } - - @Override - public String instrumentedType() { - return "org.glassfish.jersey.server.internal.inject.CookieParamValueFactoryProvider$CookieParamValueFactory"; - } - - public static class InstrumenterAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class) - @Source(SourceTypes.REQUEST_COOKIE_VALUE) - public static void onExit() { - ThreadLocalSourceType.set(SourceTypes.REQUEST_COOKIE_VALUE); - } - } -} diff --git a/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/FormInstrumentation.java b/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/FormInstrumentation.java new file mode 100644 index 00000000000..291d0cb364d --- /dev/null +++ b/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/FormInstrumentation.java @@ -0,0 +1,77 @@ +package datadog.trace.instrumentation.jersey; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static datadog.trace.instrumentation.jersey.JerseyTaintHelper.taintMultiValuedMap; +import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import com.google.auto.service.AutoService; +import datadog.trace.advice.ActiveRequestContext; +import datadog.trace.advice.RequiresRequestContext; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.api.iast.IastContext; +import datadog.trace.api.iast.InstrumentationBridge; +import datadog.trace.api.iast.Source; +import datadog.trace.api.iast.SourceTypes; +import datadog.trace.api.iast.propagation.PropagationModule; +import java.util.List; +import java.util.Map; +import net.bytebuddy.asm.Advice; + +@AutoService(InstrumenterModule.class) +public class FormInstrumentation extends InstrumenterModule.Iast + implements Instrumenter.ForKnownTypes { + + public FormInstrumentation() { + super("jersey"); + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + transformer.applyAdvice( + named("asMap").and(isPublic()).and(takesArguments(0)), + FormInstrumentation.class.getName() + "$AsMapAdvice"); + } + + @Override + public String[] knownMatchingTypes() { + return new String[] {"jakarta.ws.rs.core.Form", "javax.ws.rs.core.Form"}; + } + + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".JerseyTaintHelper", + }; + } + + @RequiresRequestContext(RequestContextSlot.IAST) + public static class AsMapAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Source(SourceTypes.REQUEST_PARAMETER_VALUE) + public static void onExit( + @Advice.Return Map> form, + @Advice.This Object self, + @ActiveRequestContext RequestContext reqCtx) { + if (form == null || form.isEmpty()) { + return; + } + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module == null) { + return; + } + final IastContext ctx = reqCtx.getData(RequestContextSlot.IAST); + if (!module.isTainted(ctx, self)) { + return; + } + if (module.isTainted(ctx, form)) { + return; + } + module.taintObject(ctx, form, SourceTypes.REQUEST_PARAMETER_VALUE); + taintMultiValuedMap(ctx, module, SourceTypes.REQUEST_PARAMETER_VALUE, form); + } + } +} diff --git a/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/HeaderParamValueFactoryInstrumentation.java b/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/HeaderParamValueFactoryInstrumentation.java deleted file mode 100644 index b727c9c1976..00000000000 --- a/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/HeaderParamValueFactoryInstrumentation.java +++ /dev/null @@ -1,46 +0,0 @@ -package datadog.trace.instrumentation.jersey; - -import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; -import static net.bytebuddy.matcher.ElementMatchers.isPublic; -import static net.bytebuddy.matcher.ElementMatchers.takesArguments; - -import com.google.auto.service.AutoService; -import datadog.trace.agent.tooling.Instrumenter; -import datadog.trace.agent.tooling.InstrumenterModule; -import datadog.trace.api.iast.Source; -import datadog.trace.api.iast.SourceTypes; -import net.bytebuddy.asm.Advice; - -@AutoService(InstrumenterModule.class) -public class HeaderParamValueFactoryInstrumentation extends InstrumenterModule.Iast - implements Instrumenter.ForSingleType { - - public HeaderParamValueFactoryInstrumentation() { - super("jersey"); - } - - @Override - public void methodAdvice(MethodTransformer transformer) { - transformer.applyAdvice( - named("get").and(isPublic()).and(takesArguments(1)), - HeaderParamValueFactoryInstrumentation.class.getName() + "$InstrumenterAdvice"); - } - - @Override - public String[] helperClassNames() { - return new String[] {packageName + ".ThreadLocalSourceType"}; - } - - @Override - public String instrumentedType() { - return "org.glassfish.jersey.server.internal.inject.HeaderParamValueFactoryProvider$HeaderParamValueFactory"; - } - - public static class InstrumenterAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class) - @Source(SourceTypes.REQUEST_HEADER_VALUE) - public static void onExit() { - ThreadLocalSourceType.set(SourceTypes.REQUEST_HEADER_VALUE); - } - } -} diff --git a/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/InboundMessageContextInstrumentation.java b/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/InboundMessageContextInstrumentation.java index 153ce8618d5..a6d92988dc5 100644 --- a/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/InboundMessageContextInstrumentation.java +++ b/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/InboundMessageContextInstrumentation.java @@ -1,7 +1,10 @@ package datadog.trace.instrumentation.jersey; import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.namedOneOf; +import static datadog.trace.instrumentation.jersey.JerseyTaintHelper.taintMultiValuedMap; import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.returns; import static net.bytebuddy.matcher.ElementMatchers.takesArguments; import com.google.auto.service.AutoService; @@ -16,9 +19,11 @@ import datadog.trace.api.iast.Source; import datadog.trace.api.iast.SourceTypes; import datadog.trace.api.iast.propagation.PropagationModule; +import datadog.trace.bootstrap.CallDepthThreadLocalMap; import java.util.List; import java.util.Map; import net.bytebuddy.asm.Advice; +import org.glassfish.jersey.message.internal.InboundMessageContext; @AutoService(InstrumenterModule.class) public class InboundMessageContextInstrumentation extends InstrumenterModule.Iast @@ -30,14 +35,17 @@ public InboundMessageContextInstrumentation() { @Override public void methodAdvice(MethodTransformer transformer) { + String baseName = InboundMessageContextInstrumentation.class.getName(); transformer.applyAdvice( - named("getHeaders").and(isPublic()).and(takesArguments(0)), - InboundMessageContextInstrumentation.class.getName() + "$InstrumenterAdviceGetHeaders"); - + namedOneOf("header", "headers", "remove").and(returns(named(instrumentedType()))), + baseName + "$SetHeadersAdvice"); + transformer.applyAdvice( + named("getHeaders").and(isPublic()).and(takesArguments(0)), baseName + "$GetHeadersAdvice"); transformer.applyAdvice( named("getRequestCookies").and(isPublic()).and(takesArguments(0)), - InboundMessageContextInstrumentation.class.getName() - + "$InstrumenterAdviceGetRequestCookies"); + baseName + "$CookiesAdvice"); + transformer.applyAdvice( + named("readEntity").and(isPublic()).and(takesArguments(4)), baseName + "$ReadEntityAdvice"); } @Override @@ -45,42 +53,93 @@ public String instrumentedType() { return "org.glassfish.jersey.message.internal.InboundMessageContext"; } + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".JerseyTaintHelper", + }; + } + + /** This advice tries to skip tainting the headers before they are ready */ + public static class SetHeadersAdvice { + + @Advice.OnMethodEnter(suppress = Throwable.class) + public static void onEnter() { + CallDepthThreadLocalMap.incrementCallDepth(InboundMessageContext.class); + } + + @Advice.OnMethodExit(suppress = Throwable.class) + public static void onExit() { + CallDepthThreadLocalMap.decrementCallDepth(InboundMessageContext.class); + } + } + @RequiresRequestContext(RequestContextSlot.IAST) - public static class InstrumenterAdviceGetHeaders { + public static class GetHeadersAdvice { @Advice.OnMethodExit(suppress = Throwable.class) @Source(SourceTypes.REQUEST_HEADER_VALUE) public static void onExit( @Advice.Return Map> headers, @ActiveRequestContext RequestContext reqCtx) { + // ignore internal calls populating headers + if (CallDepthThreadLocalMap.getCallDepth(InboundMessageContext.class) != 0) { + return; + } + + if (headers == null || headers.isEmpty()) { + return; + } final PropagationModule prop = InstrumentationBridge.PROPAGATION; - if (prop != null && headers != null && !headers.isEmpty()) { - final IastContext ctx = reqCtx.getData(RequestContextSlot.IAST); - for (Map.Entry> entry : headers.entrySet()) { - final String name = entry.getKey(); - prop.taintString(ctx, name, SourceTypes.REQUEST_HEADER_NAME, name); - for (String value : entry.getValue()) { - prop.taintString(ctx, value, SourceTypes.REQUEST_HEADER_VALUE, name); - } - } + if (prop == null) { + return; + } + final IastContext ctx = reqCtx.getData(RequestContextSlot.IAST); + if (prop.isTainted(ctx, headers)) { + return; } + prop.taintObject(headers, SourceTypes.REQUEST_HEADER_VALUE); + taintMultiValuedMap(ctx, prop, SourceTypes.REQUEST_HEADER_VALUE, headers); } } @RequiresRequestContext(RequestContextSlot.IAST) - public static class InstrumenterAdviceGetRequestCookies { + public static class CookiesAdvice { @Advice.OnMethodExit(suppress = Throwable.class) @Source(SourceTypes.REQUEST_COOKIE_VALUE) public static void onExit( @Advice.Return Map cookies, @ActiveRequestContext RequestContext reqCtx) { + if (cookies == null || cookies.isEmpty()) { + return; + } + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module == null) { + return; + } + final IastContext ctx = reqCtx.getData(RequestContextSlot.IAST); + for (final Map.Entry entry : cookies.entrySet()) { + module.taintObject(ctx, entry.getValue(), SourceTypes.REQUEST_COOKIE_VALUE, entry.getKey()); + } + } + } + + @RequiresRequestContext(RequestContextSlot.IAST) + public static class ReadEntityAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Source(SourceTypes.REQUEST_BODY) + public static void onExit( + @Advice.Return Object result, @ActiveRequestContext RequestContext reqCtx) { + if (result == null) { + return; + } final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null && cookies != null && !cookies.isEmpty()) { - final IastContext ctx = reqCtx.getData(RequestContextSlot.IAST); - for (Map.Entry entry : cookies.entrySet()) { - final String name = entry.getKey(); - module.taintString(ctx, name, SourceTypes.REQUEST_COOKIE_NAME, name); - module.taintObject(ctx, entry.getValue(), SourceTypes.REQUEST_COOKIE_VALUE, name); - } + if (module == null) { + return; + } + final IastContext ctx = reqCtx.getData(RequestContextSlot.IAST); + if (module.isTainted(ctx, result)) { + return; } + module.taintObject(ctx, result, SourceTypes.REQUEST_BODY); } } } diff --git a/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/JerseyTaintHelper.java b/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/JerseyTaintHelper.java new file mode 100644 index 00000000000..63571dca686 --- /dev/null +++ b/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/JerseyTaintHelper.java @@ -0,0 +1,27 @@ +package datadog.trace.instrumentation.jersey; + +import datadog.trace.api.iast.IastContext; +import datadog.trace.api.iast.SourceTypes; +import datadog.trace.api.iast.propagation.PropagationModule; +import java.util.List; +import java.util.Map; + +public abstract class JerseyTaintHelper { + + private JerseyTaintHelper() {} + + public static void taintMultiValuedMap( + final IastContext ctx, + final PropagationModule module, + final byte type, + final Map> target) { + final byte nameType = SourceTypes.namedSource(type); + for (Map.Entry> entry : target.entrySet()) { + final String name = entry.getKey(); + module.taintString(ctx, name, nameType, name); + for (String value : entry.getValue()) { + module.taintString(ctx, value, type, name); + } + } + } +} diff --git a/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/ParamValueFactoryWithSourceInstrumentation.java b/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/ParamValueFactoryWithSourceInstrumentation.java deleted file mode 100644 index b40c01d5e91..00000000000 --- a/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/ParamValueFactoryWithSourceInstrumentation.java +++ /dev/null @@ -1,67 +0,0 @@ -package datadog.trace.instrumentation.jersey; - -import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; -import static net.bytebuddy.matcher.ElementMatchers.isPublic; -import static net.bytebuddy.matcher.ElementMatchers.takesArguments; - -import com.google.auto.service.AutoService; -import datadog.trace.agent.tooling.Instrumenter; -import datadog.trace.agent.tooling.InstrumenterModule; -import datadog.trace.api.iast.Propagation; -import datadog.trace.api.iast.SourceTypes; -import net.bytebuddy.asm.Advice; - -@AutoService(InstrumenterModule.class) -public class ParamValueFactoryWithSourceInstrumentation extends InstrumenterModule.Iast - implements Instrumenter.ForSingleType { - - public ParamValueFactoryWithSourceInstrumentation() { - super("jersey"); - } - - @Override - public void methodAdvice(MethodTransformer transformer) { - transformer.applyAdvice( - named("apply").and(isPublic()).and(takesArguments(1)), - ParamValueFactoryWithSourceInstrumentation.class.getName() + "$InstrumenterAdvice"); - } - - @Override - public String[] helperClassNames() { - return new String[] {packageName + ".ThreadLocalSourceType"}; - } - - @Override - public String instrumentedType() { - return "org.glassfish.jersey.server.spi.internal.ParamValueFactoryWithSource"; - } - - public static class InstrumenterAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class) - @Propagation - public static void onExit(@Advice.FieldValue("parameterSource") Object parameterSource) { - switch (parameterSource.toString()) { - case "COOKIE": - ThreadLocalSourceType.set(SourceTypes.REQUEST_COOKIE_VALUE); - break; - case "PATH": - ThreadLocalSourceType.set(SourceTypes.REQUEST_PATH_PARAMETER); - break; - case "QUERY": - ThreadLocalSourceType.set(SourceTypes.REQUEST_PARAMETER_VALUE); - break; - case "HEADER": - ThreadLocalSourceType.set(SourceTypes.REQUEST_HEADER_VALUE); - break; - case "FORM": - ThreadLocalSourceType.set(SourceTypes.REQUEST_PARAMETER_VALUE); - break; - case "CONTEXT": - ThreadLocalSourceType.set(SourceTypes.NONE); - break; - default: - ThreadLocalSourceType.set(SourceTypes.NONE); - } - } - } -} diff --git a/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/PathParamValueFactoryInstrumentation.java b/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/PathParamValueFactoryInstrumentation.java deleted file mode 100644 index 29d32127667..00000000000 --- a/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/PathParamValueFactoryInstrumentation.java +++ /dev/null @@ -1,45 +0,0 @@ -package datadog.trace.instrumentation.jersey; - -import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; -import static net.bytebuddy.matcher.ElementMatchers.takesArguments; - -import com.google.auto.service.AutoService; -import datadog.trace.agent.tooling.Instrumenter; -import datadog.trace.agent.tooling.InstrumenterModule; -import datadog.trace.api.iast.Source; -import datadog.trace.api.iast.SourceTypes; -import net.bytebuddy.asm.Advice; - -@AutoService(InstrumenterModule.class) -public class PathParamValueFactoryInstrumentation extends InstrumenterModule.Iast - implements Instrumenter.ForSingleType { - - public PathParamValueFactoryInstrumentation() { - super("jersey"); - } - - @Override - public void methodAdvice(MethodTransformer transformer) { - transformer.applyAdvice( - named("get").and(takesArguments(1)), - PathParamValueFactoryInstrumentation.class.getName() + "$InstrumenterAdvice"); - } - - @Override - public String[] helperClassNames() { - return new String[] {packageName + ".ThreadLocalSourceType"}; - } - - @Override - public String instrumentedType() { - return "org.glassfish.jersey.server.internal.inject.PathParamValueFactoryProvider$PathParamValueFactory"; - } - - public static class InstrumenterAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class) - @Source(SourceTypes.REQUEST_PATH_PARAMETER) - public static void onExit() { - ThreadLocalSourceType.set(SourceTypes.REQUEST_PATH_PARAMETER); - } - } -} diff --git a/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/ThreadLocalSourceType.java b/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/ThreadLocalSourceType.java deleted file mode 100644 index 299f2e9bfc6..00000000000 --- a/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/ThreadLocalSourceType.java +++ /dev/null @@ -1,16 +0,0 @@ -package datadog.trace.instrumentation.jersey; - -import datadog.trace.api.iast.SourceTypes; - -public class ThreadLocalSourceType { - private static final ThreadLocal SOURCE = - ThreadLocal.withInitial(() -> SourceTypes.REQUEST_PARAMETER_VALUE); - - public static void set(byte source) { - SOURCE.set(source); - } - - public static byte get() { - return SOURCE.get(); - } -} diff --git a/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/UriRoutingContextInstrumentation.java b/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/UriRoutingContextInstrumentation.java new file mode 100644 index 00000000000..e87fab8fd5b --- /dev/null +++ b/dd-java-agent/instrumentation/jersey/src/main/java/datadog/trace/instrumentation/jersey/UriRoutingContextInstrumentation.java @@ -0,0 +1,110 @@ +package datadog.trace.instrumentation.jersey; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static net.bytebuddy.matcher.ElementMatchers.isPublic; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import com.google.auto.service.AutoService; +import datadog.trace.advice.ActiveRequestContext; +import datadog.trace.advice.RequiresRequestContext; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.InstrumenterModule; +import datadog.trace.api.gateway.RequestContext; +import datadog.trace.api.gateway.RequestContextSlot; +import datadog.trace.api.iast.IastContext; +import datadog.trace.api.iast.InstrumentationBridge; +import datadog.trace.api.iast.Source; +import datadog.trace.api.iast.SourceTypes; +import datadog.trace.api.iast.propagation.PropagationModule; +import java.util.List; +import java.util.Map; +import net.bytebuddy.asm.Advice; + +@AutoService(InstrumenterModule.class) +public class UriRoutingContextInstrumentation extends InstrumenterModule.Iast + implements Instrumenter.ForSingleType { + + public UriRoutingContextInstrumentation() { + super("jersey"); + } + + @Override + public void methodAdvice(MethodTransformer transformer) { + String baseName = UriRoutingContextInstrumentation.class.getName(); + transformer.applyAdvice( + named("getPathParameters").and(isPublic().and(takesArguments(boolean.class))), + baseName + "$GetPathParametersAdvice"); + transformer.applyAdvice( + named("getQueryParameters").and(isPublic().and(takesArguments(boolean.class))), + baseName + "$GetQueryParametersAdvice"); + } + + @Override + public String instrumentedType() { + return "org.glassfish.jersey.server.internal.routing.UriRoutingContext"; + } + + @Override + public String[] helperClassNames() { + return new String[] { + packageName + ".JerseyTaintHelper", + }; + } + + @RequiresRequestContext(RequestContextSlot.IAST) + public static class GetPathParametersAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Source(SourceTypes.REQUEST_PATH_PARAMETER) + public static void onExit( + @Advice.Return Map> pathParams, + @ActiveRequestContext RequestContext reqCtx) { + if (pathParams == null || pathParams.isEmpty()) { + return; + } + final PropagationModule prop = InstrumentationBridge.PROPAGATION; + if (prop == null) { + return; + } + final IastContext ctx = reqCtx.getData(RequestContextSlot.IAST); + if (prop.isTainted(ctx, pathParams)) { + return; + } + prop.taintObject(ctx, pathParams, SourceTypes.REQUEST_PATH_PARAMETER); + for (Map.Entry> entry : pathParams.entrySet()) { + final String name = entry.getKey(); + for (String value : entry.getValue()) { + prop.taintString(ctx, value, SourceTypes.REQUEST_PATH_PARAMETER, name); + } + } + } + } + + @RequiresRequestContext(RequestContextSlot.IAST) + public static class GetQueryParametersAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Source(SourceTypes.REQUEST_PARAMETER_VALUE) + public static void onExit( + @Advice.Return Map> queryParams, + @ActiveRequestContext RequestContext reqCtx) { + if (queryParams == null || queryParams.isEmpty()) { + return; + } + final PropagationModule prop = InstrumentationBridge.PROPAGATION; + if (prop == null) { + return; + } + final IastContext ctx = reqCtx.getData(RequestContextSlot.IAST); + if (prop.isTainted(ctx, queryParams)) { + return; + } + prop.taintObject(ctx, queryParams, SourceTypes.REQUEST_PARAMETER_VALUE); + for (Map.Entry> entry : queryParams.entrySet()) { + final String name = entry.getKey(); + prop.taintString(ctx, name, SourceTypes.REQUEST_PARAMETER_NAME, name); + for (String value : entry.getValue()) { + prop.taintString(ctx, value, SourceTypes.REQUEST_PARAMETER_VALUE, name); + } + } + } + } +} diff --git a/dd-java-agent/instrumentation/jersey/src/test/groovy/datadog/trace/instrumentation/jersey/AbstractStringReaderTest.groovy b/dd-java-agent/instrumentation/jersey/src/test/groovy/datadog/trace/instrumentation/jersey/AbstractStringReaderTest.groovy deleted file mode 100644 index 4a3c0afbb4f..00000000000 --- a/dd-java-agent/instrumentation/jersey/src/test/groovy/datadog/trace/instrumentation/jersey/AbstractStringReaderTest.groovy +++ /dev/null @@ -1,13 +0,0 @@ -package datadog.trace.instrumentation.jersey - -import datadog.trace.agent.test.AgentTestRunner - -class AbstractStringReaderTest extends AgentTestRunner { - - - @Override - protected void configurePreAgent() { - injectSysConfig("dd.iast.enabled", "true") - } -} - diff --git a/dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractJerseySmokeTest.groovy b/dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractJerseySmokeTest.groovy index 077279fb917..6d6bd4762f2 100644 --- a/dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractJerseySmokeTest.groovy +++ b/dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractJerseySmokeTest.groovy @@ -63,7 +63,7 @@ class AbstractJerseySmokeTest extends AbstractIastServerSmokeTest { assert response.code() == 200 hasTainted { tainted -> tainted.value == 'pepito' && - tainted.ranges[0].source.name == 'X-Custom-header' && + tainted.ranges[0].source.name.equalsIgnoreCase('X-Custom-header') && tainted.ranges[0].source.origin == 'http.request.header' } }