From 4ad36f5440edeb0bfcfa081e043a8e245a03b3d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20=C3=81lvarez=20=C3=81lvarez?= Date: Tue, 31 Oct 2023 18:30:04 +0100 Subject: [PATCH] Remove callsites for IAST sources and use bytebuddy advices (#6083) --- .../tooling/iast/TaintableEnumeration.java | 101 ++++ .../bytebuddy/matcher/ignored_class_name.trie | 5 - .../iast/TaintableEnumerationTest.groovy | 99 ++++ .../callsite/HttpServlet3RequestCallSite.java | 39 -- .../callsite/Servlet3RequestCallSite.java | 38 -- ...let3TestGetParameterInstrumentation.groovy | 51 -- .../bar/smoketest/HttpServlet3TestSuite.java | 12 - .../HttpServletWrapper3TestSuite.java | 12 - .../foo/bar/smoketest/Servlet3TestSuite.java | 12 - .../java/foo/bar/smoketest/ServletSuite.java | 9 - .../JakartaHttpServletRequestCallSite.java | 144 ++---- ...HttpServletRequestInputStreamCallSite.java | 33 -- ...artaHttpServletRequestInstrumentation.java | 285 +++++++++++ .../JakartaServletRequestCallSite.java | 159 ------ ...tpServletRequestInstrumentationTest.groovy | 459 ++++++++++++++++++ .../JakartaServletRequestCallSiteTest.groovy | 145 ------ .../JakartaHttpServletRequestTestSuite.java | 38 +- ...rtaHttpServletRequestWrapperTestSuite.java | 38 +- .../JakartaServletRequestTestSuite.java | 46 -- ...JakartaServletRequestWrapperTestSuite.java | 47 -- .../smoketest/ServletRequestTestSuite.java | 21 +- .../http/HttpServletRequestCallSite.java | 59 +++ .../HttpServletRequestInstrumentation.java | 285 +++++++++++ .../request/HttpServletRequestCallSite.java | 304 ------------ ...HttpServletRequestInputStreamCallSite.java | 33 -- .../request/ServletRequestCallSite.java | 152 ------ .../groovy/CookieInstrumentationTest.groovy | 15 +- .../HttpServletRequestCallSiteTest.groovy | 273 ----------- .../test/groovy/HttpServletRequestTest.groovy | 459 ++++++++++++++++++ .../groovy/ServletRequestCallSiteTest.groovy | 96 ---- .../TestGetParameterInstrumentation.groovy | 65 --- .../smoketest/controller/CookieTestSuite.java | 24 - .../JavaxHttpServletRequestTestSuite.java | 39 +- ...vaxHttpServletRequestWrapperTestSuite.java | 40 +- .../JavaxServletRequestTestSuite.java | 47 -- .../JavaxServletRequestWrapperTestSuite.java | 48 -- .../controller/ServletRequestTestSuite.java | 22 +- .../TestHttpServletRequestCallSiteSuite.java | 50 -- ...tractHttpServerRequestInstrumentation.java | 28 +- ...CaseInsensitiveHeadersInstrumentation.java | 60 +-- .../core/HeadersAdaptorInstrumentation.java | 62 +-- .../Http2ServerRequestInstrumentation.java | 8 +- .../HttpServerRequestInstrumentation.java | 10 +- .../server/CookieImplInstrumentation.java | 18 +- ...IastRoutingContextImplInstrumentation.java | 12 +- .../core/VertxHttpHeadersInstrumentation.java | 60 +-- 46 files changed, 1929 insertions(+), 2133 deletions(-) create mode 100644 dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/iast/TaintableEnumeration.java create mode 100644 dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/iast/TaintableEnumerationTest.groovy delete mode 100644 dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/callsite/HttpServlet3RequestCallSite.java delete mode 100644 dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/callsite/Servlet3RequestCallSite.java delete mode 100644 dd-java-agent/instrumentation/servlet/request-3/src/test/groovy/Servlet3TestGetParameterInstrumentation.groovy delete mode 100644 dd-java-agent/instrumentation/servlet/request-3/src/test/java/foo/bar/smoketest/HttpServlet3TestSuite.java delete mode 100644 dd-java-agent/instrumentation/servlet/request-3/src/test/java/foo/bar/smoketest/HttpServletWrapper3TestSuite.java delete mode 100644 dd-java-agent/instrumentation/servlet/request-3/src/test/java/foo/bar/smoketest/Servlet3TestSuite.java delete mode 100644 dd-java-agent/instrumentation/servlet/request-3/src/test/java/foo/bar/smoketest/ServletSuite.java delete mode 100644 dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaHttpServletRequestInputStreamCallSite.java create mode 100644 dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaHttpServletRequestInstrumentation.java delete mode 100644 dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaServletRequestCallSite.java create mode 100644 dd-java-agent/instrumentation/servlet/request-5/src/test/groovy/JakartaHttpServletRequestInstrumentationTest.groovy delete mode 100644 dd-java-agent/instrumentation/servlet/request-5/src/test/groovy/JakartaServletRequestCallSiteTest.groovy delete mode 100644 dd-java-agent/instrumentation/servlet/request-5/src/test/java/foo/bar/smoketest/JakartaServletRequestTestSuite.java delete mode 100644 dd-java-agent/instrumentation/servlet/request-5/src/test/java/foo/bar/smoketest/JakartaServletRequestWrapperTestSuite.java create mode 100644 dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletRequestCallSite.java create mode 100644 dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletRequestInstrumentation.java delete mode 100644 dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/request/HttpServletRequestCallSite.java delete mode 100644 dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/request/HttpServletRequestInputStreamCallSite.java delete mode 100644 dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/request/ServletRequestCallSite.java delete mode 100644 dd-java-agent/instrumentation/servlet/src/test/groovy/HttpServletRequestCallSiteTest.groovy create mode 100644 dd-java-agent/instrumentation/servlet/src/test/groovy/HttpServletRequestTest.groovy delete mode 100644 dd-java-agent/instrumentation/servlet/src/test/groovy/ServletRequestCallSiteTest.groovy delete mode 100644 dd-java-agent/instrumentation/servlet/src/test/groovy/TestGetParameterInstrumentation.groovy delete mode 100644 dd-java-agent/instrumentation/servlet/src/test/java/datadog/smoketest/controller/CookieTestSuite.java delete mode 100644 dd-java-agent/instrumentation/servlet/src/test/java/datadog/smoketest/controller/JavaxServletRequestTestSuite.java delete mode 100644 dd-java-agent/instrumentation/servlet/src/test/java/datadog/smoketest/controller/JavaxServletRequestWrapperTestSuite.java delete mode 100644 dd-java-agent/instrumentation/servlet/src/test/java/datadog/smoketest/controller/TestHttpServletRequestCallSiteSuite.java diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/iast/TaintableEnumeration.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/iast/TaintableEnumeration.java new file mode 100644 index 00000000000..cfc8fb3b44f --- /dev/null +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/iast/TaintableEnumeration.java @@ -0,0 +1,101 @@ +package datadog.trace.agent.tooling.iast; + +import datadog.trace.api.iast.IastContext; +import datadog.trace.api.iast.propagation.PropagationModule; +import datadog.trace.util.stacktrace.StackUtils; +import edu.umd.cs.findbugs.annotations.NonNull; +import java.util.Enumeration; +import javax.annotation.Nullable; + +public class TaintableEnumeration implements Enumeration { + + private static final String CLASS_NAME = TaintableEnumeration.class.getName(); + + private volatile IastContext context; + private volatile boolean contextFetched; + + private final PropagationModule module; + + private final byte origin; + + private final CharSequence name; + + private final boolean useValueAsName; + + private final Enumeration delegate; + + private TaintableEnumeration( + @NonNull final Enumeration delegate, + @NonNull final PropagationModule module, + final byte origin, + @Nullable final CharSequence name, + final boolean useValueAsName) { + this.delegate = delegate; + this.module = module; + this.origin = origin; + this.name = name; + this.useValueAsName = useValueAsName; + } + + @Override + public boolean hasMoreElements() { + try { + return delegate.hasMoreElements(); + } catch (Throwable e) { + StackUtils.filterFirst(e, TaintableEnumeration::nonTaintableEnumerationStack); + throw e; + } + } + + @Override + public String nextElement() { + final String next; + try { + next = delegate.nextElement(); + } catch (Throwable e) { + StackUtils.filterFirst(e, TaintableEnumeration::nonTaintableEnumerationStack); + throw e; + } + try { + module.taint(context(), next, origin, name(next)); + } catch (final Throwable e) { + module.onUnexpectedException("Failed to taint enumeration", e); + } + return next; + } + + private IastContext context() { + if (!contextFetched) { + contextFetched = true; + context = IastContext.Provider.get(); + } + return context; + } + + private CharSequence name(final String value) { + if (name != null) { + return name; + } + return useValueAsName ? value : null; + } + + private static boolean nonTaintableEnumerationStack(final StackTraceElement element) { + return !CLASS_NAME.equals(element.getClassName()); + } + + public static Enumeration wrap( + @NonNull final Enumeration delegate, + @NonNull final PropagationModule module, + final byte origin, + @Nullable final CharSequence name) { + return new TaintableEnumeration(delegate, module, origin, name, false); + } + + public static Enumeration wrap( + @NonNull final Enumeration delegate, + @NonNull final PropagationModule module, + final byte origin, + boolean useValueAsName) { + return new TaintableEnumeration(delegate, module, origin, null, useValueAsName); + } +} diff --git a/dd-java-agent/agent-tooling/src/main/resources/datadog/trace/agent/tooling/bytebuddy/matcher/ignored_class_name.trie b/dd-java-agent/agent-tooling/src/main/resources/datadog/trace/agent/tooling/bytebuddy/matcher/ignored_class_name.trie index fd259e3cd80..ede87f07ae0 100644 --- a/dd-java-agent/agent-tooling/src/main/resources/datadog/trace/agent/tooling/bytebuddy/matcher/ignored_class_name.trie +++ b/dd-java-agent/agent-tooling/src/main/resources/datadog/trace/agent/tooling/bytebuddy/matcher/ignored_class_name.trie @@ -284,8 +284,6 @@ 2 org.springframework.expression.* 2 org.springframework.format.* 2 org.springframework.http.* -# Need for IAST: calls ServletRequest methods instrumented at callsite -0 org.springframework.http.server.ServletServerHttpRequest # Need for IAST: we instrument these classes 0 org.springframework.http.HttpHeaders 0 org.springframework.http.ReadOnlyHttpHeaders @@ -321,14 +319,11 @@ 2 org.springframework.validation.* 2 org.springframework.web.* 0 org.springframework.web.context.request.async.* -0 org.springframework.web.context.request.* 0 org.springframework.web.context.support.AbstractRefreshableWebApplicationContext 0 org.springframework.web.context.support.GenericWebApplicationContext 0 org.springframework.web.context.support.XmlWebApplicationContext 0 org.springframework.web.reactive.* 0 org.springframework.web.servlet.* -# Included for IAST -0 org.springframework.web.util.WebUtils # Need for IAST so propagation of tainted objects is complete in spring 2.7.5 0 org.springframework.util.StreamUtils$NonClosingInputStream # Included for IAST Spring mvc unvalidated redirect and xss vulnerability diff --git a/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/iast/TaintableEnumerationTest.groovy b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/iast/TaintableEnumerationTest.groovy new file mode 100644 index 00000000000..78566a8bec7 --- /dev/null +++ b/dd-java-agent/agent-tooling/src/test/groovy/datadog/trace/agent/tooling/iast/TaintableEnumerationTest.groovy @@ -0,0 +1,99 @@ +package datadog.trace.agent.tooling.iast + +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.SourceTypes +import datadog.trace.api.iast.propagation.PropagationModule +import datadog.trace.bootstrap.instrumentation.api.AgentSpan +import datadog.trace.bootstrap.instrumentation.api.AgentTracer +import datadog.trace.test.util.DDSpecification +import spock.lang.Shared + +class TaintableEnumerationTest extends DDSpecification { + + @Shared + protected static final AgentTracer.TracerAPI ORIGINAL_TRACER = AgentTracer.get() + + protected AgentTracer.TracerAPI tracer = Mock(AgentTracer.TracerAPI) + + protected IastContext iastCtx = Mock(IastContext) + + protected RequestContext reqCtx = Mock(RequestContext) { + getData(RequestContextSlot.IAST) >> iastCtx + } + + protected AgentSpan span = Mock(AgentSpan) { + getRequestContext() >> reqCtx + } + + protected PropagationModule module + + + void setup() { + AgentTracer.forceRegister(tracer) + module = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(module) + } + + void cleanup() { + AgentTracer.forceRegister(ORIGINAL_TRACER) + InstrumentationBridge.clearIastModules() + } + + void 'underlying enumerated values are tainted with a name'() { + given: + final values = (1..10).collect { "value$it".toString() } + final origin = SourceTypes.REQUEST_PARAMETER_NAME + final name = 'test' + final enumeration = TaintableEnumeration.wrap(Collections.enumeration(values), module, origin, name) + + when: + final result = enumeration.collect() + + then: + result == values + values.each { 1 * module.taint(_, it, origin, name) } + 1 * tracer.activeSpan() >> span // only one access to the active context + } + + void 'underlying enumerated values are tainted with the value as a name'() { + given: + final values = (1..10).collect { "value$it".toString() } + final origin = SourceTypes.REQUEST_PARAMETER_NAME + final enumeration = TaintableEnumeration.wrap(Collections.enumeration(values), module, origin, true) + + when: + final result = enumeration.collect() + + then: + result == values + values.each { 1 * module.taint(_, it, origin, it) } + } + + void 'taintable enumeration leaves no trace in case of error'() { + given: + final origin = SourceTypes.REQUEST_PARAMETER_NAME + final enumeration = TaintableEnumeration.wrap(new BadEnumeration(), module, origin, true) + + when: + enumeration.hasMoreElements() + + then: + final first = thrown(Error) + first.stackTrace.find { it.className == TaintableEnumeration.name } == null + } + + private static class BadEnumeration implements Enumeration { + @Override + boolean hasMoreElements() { + throw new Error('Ooops!!!') + } + + @Override + String nextElement() { + throw new Error('Boom!!!') + } + } +} diff --git a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/callsite/HttpServlet3RequestCallSite.java b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/callsite/HttpServlet3RequestCallSite.java deleted file mode 100644 index 4156080ee2c..00000000000 --- a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/callsite/HttpServlet3RequestCallSite.java +++ /dev/null @@ -1,39 +0,0 @@ -package datadog.trace.instrumentation.servlet3.callsite; - -import datadog.trace.agent.tooling.csi.CallSite; -import datadog.trace.api.iast.IastCallSites; -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.Map; -import javax.servlet.http.HttpServletRequest; - -@CallSite(spi = IastCallSites.class) -public class HttpServlet3RequestCallSite { - - @Source(SourceTypes.REQUEST_PARAMETER_VALUE) - @CallSite.After("java.util.Map javax.servlet.http.HttpServletRequest.getParameterMap()") - @CallSite.After("java.util.Map javax.servlet.http.HttpServletRequestWrapper.getParameterMap()") - public static java.util.Map afterGetParameterMap( - @CallSite.This final HttpServletRequest self, - @CallSite.Return final Map map) { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null && map != null && !map.isEmpty()) { - try { - final IastContext ctx = IastContext.Provider.get(); - for (final Map.Entry entry : map.entrySet()) { - final String name = entry.getKey(); - module.taint(ctx, name, SourceTypes.REQUEST_PARAMETER_NAME, name); - for (final String value : entry.getValue()) { - module.taint(ctx, value, SourceTypes.REQUEST_PARAMETER_VALUE, name); - } - } - } catch (final Throwable e) { - module.onUnexpectedException("afterGetParameter threw", e); - } - } - return map; - } -} diff --git a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/callsite/Servlet3RequestCallSite.java b/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/callsite/Servlet3RequestCallSite.java deleted file mode 100644 index 063e8b11e2e..00000000000 --- a/dd-java-agent/instrumentation/servlet/request-3/src/main/java/datadog/trace/instrumentation/servlet3/callsite/Servlet3RequestCallSite.java +++ /dev/null @@ -1,38 +0,0 @@ -package datadog.trace.instrumentation.servlet3.callsite; - -import datadog.trace.agent.tooling.csi.CallSite; -import datadog.trace.api.iast.IastCallSites; -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.Map; -import javax.servlet.ServletRequest; - -@CallSite(spi = IastCallSites.class) -public class Servlet3RequestCallSite { - - @Source(SourceTypes.REQUEST_PARAMETER_VALUE) - @CallSite.After("java.util.Map javax.servlet.ServletRequest.getParameterMap()") - @CallSite.After("java.util.Map javax.servlet.ServletRequestWrapper.getParameterMap()") - public static java.util.Map afterGetParameterMap( - @CallSite.This final ServletRequest self, @CallSite.Return final Map map) { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null && map != null && !map.isEmpty()) { - try { - final IastContext ctx = IastContext.Provider.get(); - for (final Map.Entry entry : map.entrySet()) { - final String name = entry.getKey(); - module.taint(ctx, name, SourceTypes.REQUEST_PARAMETER_NAME, name); - for (final String value : entry.getValue()) { - module.taint(ctx, value, SourceTypes.REQUEST_PARAMETER_VALUE, name); - } - } - } catch (final Throwable e) { - module.onUnexpectedException("afterGetParameter threw", e); - } - } - return map; - } -} diff --git a/dd-java-agent/instrumentation/servlet/request-3/src/test/groovy/Servlet3TestGetParameterInstrumentation.groovy b/dd-java-agent/instrumentation/servlet/request-3/src/test/groovy/Servlet3TestGetParameterInstrumentation.groovy deleted file mode 100644 index 5378773471b..00000000000 --- a/dd-java-agent/instrumentation/servlet/request-3/src/test/groovy/Servlet3TestGetParameterInstrumentation.groovy +++ /dev/null @@ -1,51 +0,0 @@ -import datadog.trace.agent.test.AgentTestRunner -import datadog.trace.api.iast.InstrumentationBridge -import datadog.trace.api.iast.SourceTypes -import datadog.trace.api.iast.propagation.PropagationModule -import foo.bar.smoketest.HttpServlet3TestSuite -import foo.bar.smoketest.HttpServletWrapper3TestSuite -import foo.bar.smoketest.Servlet3TestSuite - -import javax.servlet.ServletRequest -import javax.servlet.http.HttpServletRequest -import javax.servlet.http.HttpServletRequestWrapper - -class Servlet3TestGetParameterInstrumentation extends AgentTestRunner { - - @Override - protected void configurePreAgent() { - injectSysConfig("dd.iast.enabled", "true") - } - - void cleanup() { - InstrumentationBridge.clearIastModules() - } - - void 'test getParameter'() { - setup: - final iastModule = Mock(PropagationModule) - InstrumentationBridge.registerIastModule(iastModule) - final map = [param1: ['value1', 'value2'] as String[]] - final request = Mock(requestClass) { - getParameterMap() >> map - } - - when: - final returnedMap = suite.getParameterMap(request) - - then: - returnedMap == map - map.each { key, values -> - 1 * iastModule.taint(_, key, SourceTypes.REQUEST_PARAMETER_NAME, key) - values.each { value -> - 1 * iastModule.taint(_, value, SourceTypes.REQUEST_PARAMETER_VALUE, key) - } - } - - where: - suite | requestClass - new Servlet3TestSuite() | ServletRequest - new HttpServlet3TestSuite() | HttpServletRequest - new HttpServletWrapper3TestSuite() | HttpServletRequestWrapper - } -} diff --git a/dd-java-agent/instrumentation/servlet/request-3/src/test/java/foo/bar/smoketest/HttpServlet3TestSuite.java b/dd-java-agent/instrumentation/servlet/request-3/src/test/java/foo/bar/smoketest/HttpServlet3TestSuite.java deleted file mode 100644 index 073a426ff30..00000000000 --- a/dd-java-agent/instrumentation/servlet/request-3/src/test/java/foo/bar/smoketest/HttpServlet3TestSuite.java +++ /dev/null @@ -1,12 +0,0 @@ -package foo.bar.smoketest; - -import java.util.Map; -import javax.servlet.http.HttpServletRequest; - -public class HttpServlet3TestSuite implements ServletSuite { - - @Override - public Map getParameterMap(HttpServletRequest request) { - return request.getParameterMap(); - } -} diff --git a/dd-java-agent/instrumentation/servlet/request-3/src/test/java/foo/bar/smoketest/HttpServletWrapper3TestSuite.java b/dd-java-agent/instrumentation/servlet/request-3/src/test/java/foo/bar/smoketest/HttpServletWrapper3TestSuite.java deleted file mode 100644 index 185370b1a76..00000000000 --- a/dd-java-agent/instrumentation/servlet/request-3/src/test/java/foo/bar/smoketest/HttpServletWrapper3TestSuite.java +++ /dev/null @@ -1,12 +0,0 @@ -package foo.bar.smoketest; - -import java.util.Map; -import javax.servlet.http.HttpServletRequestWrapper; - -public class HttpServletWrapper3TestSuite implements ServletSuite { - - @Override - public Map getParameterMap(HttpServletRequestWrapper request) { - return request.getParameterMap(); - } -} diff --git a/dd-java-agent/instrumentation/servlet/request-3/src/test/java/foo/bar/smoketest/Servlet3TestSuite.java b/dd-java-agent/instrumentation/servlet/request-3/src/test/java/foo/bar/smoketest/Servlet3TestSuite.java deleted file mode 100644 index e3d1ccfd606..00000000000 --- a/dd-java-agent/instrumentation/servlet/request-3/src/test/java/foo/bar/smoketest/Servlet3TestSuite.java +++ /dev/null @@ -1,12 +0,0 @@ -package foo.bar.smoketest; - -import java.util.Map; -import javax.servlet.ServletRequest; - -public class Servlet3TestSuite implements ServletSuite { - - @Override - public Map getParameterMap(ServletRequest request) { - return request.getParameterMap(); - } -} diff --git a/dd-java-agent/instrumentation/servlet/request-3/src/test/java/foo/bar/smoketest/ServletSuite.java b/dd-java-agent/instrumentation/servlet/request-3/src/test/java/foo/bar/smoketest/ServletSuite.java deleted file mode 100644 index 7e55e71ecba..00000000000 --- a/dd-java-agent/instrumentation/servlet/request-3/src/test/java/foo/bar/smoketest/ServletSuite.java +++ /dev/null @@ -1,9 +0,0 @@ -package foo.bar.smoketest; - -import java.util.Map; -import javax.servlet.ServletRequest; - -public interface ServletSuite { - - Map getParameterMap(S request); -} diff --git a/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaHttpServletRequestCallSite.java b/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaHttpServletRequestCallSite.java index 4430587e3f7..9691777a6e8 100644 --- a/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaHttpServletRequestCallSite.java +++ b/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaHttpServletRequestCallSite.java @@ -2,142 +2,58 @@ import datadog.trace.agent.tooling.csi.CallSite; import datadog.trace.api.iast.IastCallSites; -import datadog.trace.api.iast.IastContext; import datadog.trace.api.iast.InstrumentationBridge; -import datadog.trace.api.iast.Sink; import datadog.trace.api.iast.Source; import datadog.trace.api.iast.SourceTypes; -import datadog.trace.api.iast.VulnerabilityTypes; import datadog.trace.api.iast.propagation.PropagationModule; -import datadog.trace.api.iast.sink.UnvalidatedRedirectModule; -import datadog.trace.util.stacktrace.StackUtils; import jakarta.servlet.http.HttpServletRequest; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Enumeration; -import java.util.List; -import java.util.Map; +/** + * Calls to these methods are often triggered outside of customer code, we use call sites to avoid + * all these unwanted tainting operations + */ @CallSite(spi = IastCallSites.class) public class JakartaHttpServletRequestCallSite { - @Source(SourceTypes.REQUEST_PARAMETER_VALUE) + @Source(SourceTypes.REQUEST_PATH) + @CallSite.After("java.lang.String jakarta.servlet.http.HttpServletRequest.getRequestURI()") + @CallSite.After("java.lang.String jakarta.servlet.http.HttpServletRequestWrapper.getRequestURI()") + @CallSite.After("java.lang.String jakarta.servlet.http.HttpServletRequest.getPathInfo()") + @CallSite.After("java.lang.String jakarta.servlet.http.HttpServletRequestWrapper.getPathInfo()") + @CallSite.After("java.lang.String jakarta.servlet.http.HttpServletRequest.getPathTranslated()") @CallSite.After( - "java.lang.String jakarta.servlet.http.HttpServletRequest.getParameter(java.lang.String)") - @CallSite.After( - "java.lang.String jakarta.servlet.http.HttpServletRequestWrapper.getParameter(java.lang.String)") - public static String afterGetParameter( - @CallSite.This final HttpServletRequest self, - @CallSite.Argument final String name, - @CallSite.Return final String value) { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null) { - try { - module.taint(value, SourceTypes.REQUEST_PARAMETER_VALUE, name); - } catch (final Throwable e) { - module.onUnexpectedException("afterGetParameter threw", e); - } - } - return value; - } - - @Source(SourceTypes.REQUEST_PARAMETER_NAME) - @CallSite.After( - "java.util.Enumeration jakarta.servlet.http.HttpServletRequest.getParameterNames()") - @CallSite.After( - "java.util.Enumeration jakarta.servlet.http.HttpServletRequestWrapper.getParameterNames()") - public static Enumeration afterGetParameterNames( - @CallSite.This final HttpServletRequest self, - @CallSite.Return final Enumeration enumeration) - throws Throwable { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module == null || enumeration == null) { - return enumeration; - } - try { - final List parameterNames = new ArrayList<>(); - while (enumeration.hasMoreElements()) { - final String paramName = enumeration.nextElement(); - parameterNames.add(paramName); - } - try { - final IastContext ctx = IastContext.Provider.get(); - for (final String name : parameterNames) { - module.taint(ctx, name, SourceTypes.REQUEST_PARAMETER_NAME, name); + "java.lang.String jakarta.servlet.http.HttpServletRequestWrapper.getPathTranslated()") + public static String afterPath( + @CallSite.This final HttpServletRequest self, @CallSite.Return final String retValue) { + if (null != retValue && !retValue.isEmpty()) { + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module != null) { + try { + module.taint(retValue, SourceTypes.REQUEST_PATH); + } catch (final Throwable e) { + module.onUnexpectedException("afterPath threw", e); } - } catch (final Throwable e) { - module.onUnexpectedException("afterGetParameterNames threw", e); } - return Collections.enumeration(parameterNames); - } catch (final Throwable e) { - module.onUnexpectedException( - "afterGetParameterNames threw while iterating parameter names", e); - throw StackUtils.filterFirstDatadog(e); } + return retValue; } - @Source(SourceTypes.REQUEST_PARAMETER_VALUE) - @CallSite.After( - "java.lang.String[] jakarta.servlet.http.HttpServletRequest.getParameterValues(java.lang.String)") + @Source(SourceTypes.REQUEST_URI) + @CallSite.After("java.lang.StringBuffer jakarta.servlet.http.HttpServletRequest.getRequestURL()") @CallSite.After( - "java.lang.String[] jakarta.servlet.http.HttpServletRequestWrapper.getParameterValues(java.lang.String)") - public static String[] afterGetParameterValues( - @CallSite.This final HttpServletRequest self, - @CallSite.Argument final String paramName, - @CallSite.Return final String[] parameterValues) { - if (null != parameterValues && parameterValues.length > 0) { + "java.lang.StringBuffer jakarta.servlet.http.HttpServletRequestWrapper.getRequestURL()") + public static StringBuffer afterGetRequestURL( + @CallSite.This final HttpServletRequest self, @CallSite.Return final StringBuffer retValue) { + if (null != retValue && retValue.length() > 0) { final PropagationModule module = InstrumentationBridge.PROPAGATION; if (module != null) { try { - final IastContext ctx = IastContext.Provider.get(); - for (final String value : parameterValues) { - module.taint(ctx, value, SourceTypes.REQUEST_PARAMETER_VALUE, paramName); - } + module.taint(retValue, SourceTypes.REQUEST_URI); } catch (final Throwable e) { - module.onUnexpectedException("afterGetParameterValues threw", e); + module.onUnexpectedException("afterGetRequestURL threw", e); } } } - return parameterValues; - } - - @Source(SourceTypes.REQUEST_PARAMETER_VALUE) - @CallSite.After("java.util.Map jakarta.servlet.http.HttpServletRequest.getParameterMap()") - @CallSite.After("java.util.Map jakarta.servlet.http.HttpServletRequestWrapper.getParameterMap()") - public static java.util.Map afterGetParameterMap( - @CallSite.This final HttpServletRequest self, - @CallSite.Return final Map map) { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null && map != null && !map.isEmpty()) { - try { - final IastContext ctx = IastContext.Provider.get(); - for (final Map.Entry entry : map.entrySet()) { - final String name = entry.getKey(); - module.taint(ctx, name, SourceTypes.REQUEST_PARAMETER_NAME, name); - for (final String value : entry.getValue()) { - module.taint(ctx, value, SourceTypes.REQUEST_PARAMETER_VALUE, name); - } - } - } catch (final Throwable e) { - module.onUnexpectedException("afterGetParameter threw", e); - } - } - return map; - } - - @Sink(VulnerabilityTypes.UNVALIDATED_REDIRECT) - @CallSite.Before( - "jakarta.servlet.RequestDispatcher jakarta.servlet.http.HttpServletRequest.getRequestDispatcher(java.lang.String)") - @CallSite.Before( - "jakarta.servlet.RequestDispatcher jakarta.servlet.http.HttpServletRequestWrapper.getRequestDispatcher(java.lang.String)") - public static void beforeRequestDispatcher(@CallSite.Argument final String path) { - final UnvalidatedRedirectModule module = InstrumentationBridge.UNVALIDATED_REDIRECT; - if (module != null && path != null) { - try { - module.onRedirect(path); - } catch (final Throwable e) { - module.onUnexpectedException("beforeRequestDispatcher threw", e); - } - } + return retValue; } } diff --git a/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaHttpServletRequestInputStreamCallSite.java b/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaHttpServletRequestInputStreamCallSite.java deleted file mode 100644 index ed2cc609df8..00000000000 --- a/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaHttpServletRequestInputStreamCallSite.java +++ /dev/null @@ -1,33 +0,0 @@ -package datadog.trace.instrumentation.servlet5; - -import datadog.trace.agent.tooling.csi.CallSite; -import datadog.trace.api.iast.IastCallSites; -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 jakarta.servlet.ServletInputStream; -import jakarta.servlet.http.HttpServletRequest; - -@CallSite(spi = IastCallSites.class) -public class JakartaHttpServletRequestInputStreamCallSite { - - @Source(SourceTypes.REQUEST_BODY) - @CallSite.After( - "jakarta.servlet.ServletInputStream jakarta.servlet.http.HttpServletRequest.getInputStream()") - @CallSite.After( - "jakarta.servlet.ServletInputStream jakarta.servlet.http.HttpServletRequestWrapper.getInputStream()") - public static ServletInputStream afterGetInputStream( - @CallSite.This final HttpServletRequest self, - @CallSite.Return final ServletInputStream inputStream) { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null && inputStream != null) { - try { - module.taint(inputStream, SourceTypes.REQUEST_BODY); - } catch (final Throwable e) { - module.onUnexpectedException("afterGetInputStream threw", e); - } - } - return inputStream; - } -} diff --git a/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaHttpServletRequestInstrumentation.java b/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaHttpServletRequestInstrumentation.java new file mode 100644 index 00000000000..e9d373b9fb8 --- /dev/null +++ b/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaHttpServletRequestInstrumentation.java @@ -0,0 +1,285 @@ +package datadog.trace.instrumentation.servlet5; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.extendsClass; +import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.implementsInterface; +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.namedOneOf; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.not; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.iast.TaintableEnumeration; +import datadog.trace.api.iast.IastContext; +import datadog.trace.api.iast.InstrumentationBridge; +import datadog.trace.api.iast.Sink; +import datadog.trace.api.iast.Source; +import datadog.trace.api.iast.SourceTypes; +import datadog.trace.api.iast.VulnerabilityTypes; +import datadog.trace.api.iast.propagation.PropagationModule; +import datadog.trace.api.iast.sink.UnvalidatedRedirectModule; +import jakarta.servlet.http.Cookie; +import java.util.Enumeration; +import java.util.Map; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@SuppressWarnings("unused") +@AutoService(Instrumenter.class) +public class JakartaHttpServletRequestInstrumentation extends Instrumenter.Iast + implements Instrumenter.ForTypeHierarchy { + + private static final String CLASS_NAME = JakartaHttpServletRequestInstrumentation.class.getName(); + private static final ElementMatcher.Junction WRAPPER_CLASS = + named("jakarta.servlet.http.HttpServletRequestWrapper"); + + public JakartaHttpServletRequestInstrumentation() { + super("servlet", "servlet-5", "servlet-request"); + } + + @Override + public String hierarchyMarkerType() { + return "jakarta.servlet.http.HttpServletRequest"; + } + + @Override + public ElementMatcher hierarchyMatcher() { + return implementsInterface(named(hierarchyMarkerType())) + .and(not(WRAPPER_CLASS)) + .and(not(extendsClass(WRAPPER_CLASS))); + } + + @Override + public String[] helperClassNames() { + return new String[] {"datadog.trace.agent.tooling.iast.TaintableEnumeration"}; + } + + @Override + public void adviceTransformations(AdviceTransformation transformation) { + transformation.applyAdvice( + isMethod().and(named("getHeader")).and(takesArguments(String.class)), + CLASS_NAME + "$GetHeaderAdvice"); + transformation.applyAdvice( + isMethod().and(named("getHeaders")).and(takesArguments(String.class)), + CLASS_NAME + "$GetHeadersAdvice"); + transformation.applyAdvice( + isMethod().and(named("getHeaderNames")).and(takesArguments(0)), + CLASS_NAME + "$GetHeaderNamesAdvice"); + transformation.applyAdvice( + isMethod().and(named("getParameter")).and(takesArguments(String.class)), + CLASS_NAME + "$GetParameterAdvice"); + transformation.applyAdvice( + isMethod().and(named("getParameterValues")).and(takesArguments(String.class)), + CLASS_NAME + "$GetParameterValuesAdvice"); + transformation.applyAdvice( + isMethod().and(named("getParameterMap")).and(takesArguments(0)), + CLASS_NAME + "$GetParameterMapAdvice"); + transformation.applyAdvice( + isMethod().and(named("getParameterNames")).and(takesArguments(0)), + CLASS_NAME + "$GetParameterNamesAdvice"); + transformation.applyAdvice( + isMethod().and(named("getCookies")).and(takesArguments(0)), + CLASS_NAME + "$GetCookiesAdvice"); + transformation.applyAdvice( + isMethod().and(named("getQueryString")).and(takesArguments(0)), + CLASS_NAME + "$GetQueryStringAdvice"); + transformation.applyAdvice( + isMethod().and(namedOneOf("getInputStream", "getReader")).and(takesArguments(0)), + CLASS_NAME + "$GetBodyAdvice"); + transformation.applyAdvice( + isMethod().and(named("getRequestDispatcher")).and(takesArguments(String.class)), + CLASS_NAME + "$GetRequestDispatcherAdvice"); + } + + public static class GetHeaderAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Source(SourceTypes.REQUEST_HEADER_VALUE) + public static void onExit( + @Advice.Argument(0) final String name, @Advice.Return final String value) { + if (value == null) { + return; + } + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module == null) { + return; + } + module.taint(value, SourceTypes.REQUEST_HEADER_VALUE, name); + } + } + + public static class GetHeadersAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Source(SourceTypes.REQUEST_HEADER_VALUE) + public static void onExit( + @Advice.Argument(0) final String name, + @Advice.Return(readOnly = false) Enumeration enumeration) { + if (enumeration == null) { + return; + } + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module == null) { + return; + } + enumeration = + TaintableEnumeration.wrap(enumeration, module, SourceTypes.REQUEST_HEADER_VALUE, name); + } + } + + public static class GetHeaderNamesAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Source(SourceTypes.REQUEST_HEADER_NAME) + public static void onExit(@Advice.Return(readOnly = false) Enumeration enumeration) { + if (enumeration == null) { + return; + } + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module == null) { + return; + } + enumeration = + TaintableEnumeration.wrap(enumeration, module, SourceTypes.REQUEST_HEADER_NAME, true); + } + } + + public static class GetParameterAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Source(SourceTypes.REQUEST_PARAMETER_VALUE) + public static void onExit( + @Advice.Argument(0) final String name, @Advice.Return final String value) { + if (value == null) { + return; + } + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module == null) { + return; + } + module.taint(value, SourceTypes.REQUEST_PARAMETER_VALUE, name); + } + } + + public static class GetParameterValuesAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Source(SourceTypes.REQUEST_PARAMETER_VALUE) + public static void onExit( + @Advice.Argument(0) final String name, @Advice.Return final String[] values) { + if (values == null || values.length == 0) { + return; + } + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module == null) { + return; + } + final IastContext ctx = IastContext.Provider.get(); + for (final String value : values) { + module.taint(ctx, value, SourceTypes.REQUEST_PARAMETER_VALUE, name); + } + } + } + + public static class GetParameterMapAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Source(SourceTypes.REQUEST_PARAMETER_VALUE) + public static void onExit(@Advice.Return final Map parameters) { + if (parameters == null || parameters.isEmpty()) { + return; + } + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module == null) { + return; + } + final IastContext ctx = IastContext.Provider.get(); + for (final Map.Entry entry : parameters.entrySet()) { + final String name = entry.getKey(); + module.taint(ctx, name, SourceTypes.REQUEST_PARAMETER_NAME, name); + final String[] values = entry.getValue(); + if (values != null) { + for (final String value : entry.getValue()) { + module.taint(ctx, value, SourceTypes.REQUEST_PARAMETER_VALUE, name); + } + } + } + } + } + + public static class GetParameterNamesAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Source(SourceTypes.REQUEST_PARAMETER_NAME) + public static void onExit(@Advice.Return(readOnly = false) Enumeration enumeration) { + if (enumeration == null) { + return; + } + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module == null) { + return; + } + enumeration = + TaintableEnumeration.wrap(enumeration, module, SourceTypes.REQUEST_PARAMETER_NAME, true); + } + } + + public static class GetCookiesAdvice { + + @Advice.OnMethodExit(suppress = Throwable.class) + @Source(SourceTypes.REQUEST_COOKIE_VALUE) + public static void onExit(@Advice.Return final Cookie[] cookies) { + if (cookies == null || cookies.length == 0) { + return; + } + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module == null) { + return; + } + final IastContext ctx = IastContext.Provider.get(); + for (final Cookie cookie : cookies) { + module.taint(ctx, cookie, SourceTypes.REQUEST_COOKIE_VALUE); + } + } + } + + public static class GetQueryStringAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Source(SourceTypes.REQUEST_QUERY) + public static void onExit(@Advice.Return final String queryString) { + if (queryString == null) { + return; + } + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module == null) { + return; + } + module.taint(queryString, SourceTypes.REQUEST_QUERY); + } + } + + public static class GetBodyAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Source(SourceTypes.REQUEST_BODY) + public static void onExit(@Advice.Return final Object body) { + if (body == null) { + return; + } + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module == null) { + return; + } + module.taint(body, SourceTypes.REQUEST_BODY); + } + } + + public static class GetRequestDispatcherAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Sink(VulnerabilityTypes.UNVALIDATED_REDIRECT) + public static void onExit(@Advice.Argument(0) final String path) { + if (path == null) { + return; + } + final UnvalidatedRedirectModule module = InstrumentationBridge.UNVALIDATED_REDIRECT; + if (module == null) { + return; + } + module.onRedirect(path); + } + } +} diff --git a/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaServletRequestCallSite.java b/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaServletRequestCallSite.java deleted file mode 100644 index bab15ca138f..00000000000 --- a/dd-java-agent/instrumentation/servlet/request-5/src/main/java/datadog/trace/instrumentation/servlet5/JakartaServletRequestCallSite.java +++ /dev/null @@ -1,159 +0,0 @@ -package datadog.trace.instrumentation.servlet5; - -import datadog.trace.agent.tooling.csi.CallSite; -import datadog.trace.api.iast.IastCallSites; -import datadog.trace.api.iast.IastContext; -import datadog.trace.api.iast.InstrumentationBridge; -import datadog.trace.api.iast.Sink; -import datadog.trace.api.iast.Source; -import datadog.trace.api.iast.SourceTypes; -import datadog.trace.api.iast.VulnerabilityTypes; -import datadog.trace.api.iast.propagation.PropagationModule; -import datadog.trace.api.iast.sink.UnvalidatedRedirectModule; -import datadog.trace.util.stacktrace.StackUtils; -import jakarta.servlet.ServletInputStream; -import jakarta.servlet.ServletRequest; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Enumeration; -import java.util.List; -import java.util.Map; - -@CallSite(spi = IastCallSites.class) -public class JakartaServletRequestCallSite { - - @Source(SourceTypes.REQUEST_PARAMETER_VALUE) - @CallSite.After("java.lang.String jakarta.servlet.ServletRequest.getParameter(java.lang.String)") - @CallSite.After( - "java.lang.String jakarta.servlet.ServletRequestWrapper.getParameter(java.lang.String)") - public static String afterGetParameter( - @CallSite.This final ServletRequest self, - @CallSite.Argument final String name, - @CallSite.Return final String value) { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null) { - try { - module.taint(value, SourceTypes.REQUEST_PARAMETER_VALUE, name); - } catch (final Throwable e) { - module.onUnexpectedException("afterGetParameter threw", e); - } - } - return value; - } - - @Source(SourceTypes.REQUEST_PARAMETER_NAME) - @CallSite.After("java.util.Enumeration jakarta.servlet.ServletRequest.getParameterNames()") - @CallSite.After("java.util.Enumeration jakarta.servlet.ServletRequestWrapper.getParameterNames()") - public static Enumeration afterGetParameterNames( - @CallSite.This final ServletRequest self, - @CallSite.Return final Enumeration enumeration) - throws Throwable { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module == null || enumeration == null) { - return enumeration; - } - try { - final List parameterNames = new ArrayList<>(); - while (enumeration.hasMoreElements()) { - final String paramName = enumeration.nextElement(); - parameterNames.add(paramName); - } - try { - final IastContext ctx = IastContext.Provider.get(); - for (final String name : parameterNames) { - module.taint(ctx, name, SourceTypes.REQUEST_PARAMETER_NAME, name); - } - } catch (final Throwable e) { - module.onUnexpectedException("afterGetParameterNames threw", e); - } - return Collections.enumeration(parameterNames); - } catch (final Throwable e) { - module.onUnexpectedException( - "afterGetParameterNames threw while iterating parameter names", e); - throw StackUtils.filterFirstDatadog(e); - } - } - - @Source(SourceTypes.REQUEST_PARAMETER_VALUE) - @CallSite.After( - "java.lang.String[] jakarta.servlet.ServletRequest.getParameterValues(java.lang.String)") - @CallSite.After( - "java.lang.String[] jakarta.servlet.ServletRequestWrapper.getParameterValues(java.lang.String)") - public static String[] afterGetParameterValues( - @CallSite.This final ServletRequest self, - @CallSite.Argument final String paramName, - @CallSite.Return final String[] parameterValues) { - if (null != parameterValues && parameterValues.length > 0) { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null) { - try { - final IastContext ctx = IastContext.Provider.get(); - for (final String value : parameterValues) { - module.taint(ctx, value, SourceTypes.REQUEST_PARAMETER_VALUE, paramName); - } - } catch (final Throwable e) { - module.onUnexpectedException("afterGetParameterValues threw", e); - } - } - } - return parameterValues; - } - - @Source(SourceTypes.REQUEST_PARAMETER_VALUE) - @CallSite.After("java.util.Map jakarta.servlet.ServletRequest.getParameterMap()") - @CallSite.After("java.util.Map jakarta.servlet.ServletRequestWrapper.getParameterMap()") - public static java.util.Map afterGetParameterMap( - @CallSite.This final ServletRequest self, @CallSite.Return final Map map) { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null && map != null && !map.isEmpty()) { - try { - final IastContext ctx = IastContext.Provider.get(); - for (final Map.Entry entry : map.entrySet()) { - final String name = entry.getKey(); - module.taint(ctx, name, SourceTypes.REQUEST_PARAMETER_NAME, name); - for (final String value : entry.getValue()) { - module.taint(ctx, value, SourceTypes.REQUEST_PARAMETER_VALUE, name); - } - } - } catch (final Throwable e) { - module.onUnexpectedException("afterGetParameter threw", e); - } - } - return map; - } - - @Sink(VulnerabilityTypes.UNVALIDATED_REDIRECT) - @CallSite.Before( - "jakarta.servlet.RequestDispatcher jakarta.servlet.ServletRequest.getRequestDispatcher(java.lang.String)") - @CallSite.Before( - "jakarta.servlet.RequestDispatcher jakarta.servlet.ServletRequestWrapper.getRequestDispatcher(java.lang.String)") - public static void beforeRequestDispatcher(@CallSite.Argument final String path) { - final UnvalidatedRedirectModule module = InstrumentationBridge.UNVALIDATED_REDIRECT; - if (module != null && path != null) { - try { - module.onRedirect(path); - } catch (final Throwable e) { - module.onUnexpectedException("beforeRequestDispatcher threw", e); - } - } - } - - @Source(SourceTypes.REQUEST_BODY) - @CallSite.After( - "jakarta.servlet.ServletInputStream jakarta.servlet.ServletRequest.getInputStream()") - @CallSite.After( - "jakarta.servlet.ServletInputStream jakarta.servlet.ServletRequestWrapper.getInputStream()") - public static ServletInputStream afterGetInputStream( - @CallSite.This final ServletRequest self, - @CallSite.Return final ServletInputStream inputStream) { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null && inputStream != null) { - try { - module.taint(inputStream, SourceTypes.REQUEST_BODY); - } catch (final Throwable e) { - module.onUnexpectedException("afterGetInputStream threw", e); - } - } - return inputStream; - } -} diff --git a/dd-java-agent/instrumentation/servlet/request-5/src/test/groovy/JakartaHttpServletRequestInstrumentationTest.groovy b/dd-java-agent/instrumentation/servlet/request-5/src/test/groovy/JakartaHttpServletRequestInstrumentationTest.groovy new file mode 100644 index 00000000000..7c27dc724f1 --- /dev/null +++ b/dd-java-agent/instrumentation/servlet/request-5/src/test/groovy/JakartaHttpServletRequestInstrumentationTest.groovy @@ -0,0 +1,459 @@ +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.api.iast.InstrumentationBridge +import datadog.trace.api.iast.SourceTypes +import datadog.trace.api.iast.propagation.PropagationModule +import datadog.trace.api.iast.sink.UnvalidatedRedirectModule +import foo.bar.smoketest.JakartaHttpServletRequestTestSuite +import foo.bar.smoketest.JakartaHttpServletRequestWrapperTestSuite +import foo.bar.smoketest.ServletRequestTestSuite +import jakarta.servlet.RequestDispatcher +import jakarta.servlet.ServletInputStream +import jakarta.servlet.http.Cookie +import jakarta.servlet.http.HttpServletRequest +import jakarta.servlet.http.HttpServletRequestWrapper + +import datadog.trace.agent.tooling.iast.TaintableEnumeration + +class JakartaHttpServletRequestInstrumentationTest extends AgentTestRunner { + + @Override + protected void configurePreAgent() { + injectSysConfig('dd.iast.enabled', 'true') + } + + void cleanup() { + InstrumentationBridge.clearIastModules() + } + + void 'test getHeader'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getHeader('header') + + then: + result == 'value' + 1 * mock.getHeader('header') >> 'value' + 1 * iastModule.taint('value', SourceTypes.REQUEST_HEADER_VALUE, 'header') + 0 * _ + + where: + suite << testSuite() + } + + void 'test getHeaders'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final headers = ['value1', 'value2'] + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getHeaders('headers').collect() + + then: + result == headers + 1 * mock.getHeaders('headers') >> Collections.enumeration(headers) + headers.each { 1 * iastModule.taint(_, it, SourceTypes.REQUEST_HEADER_VALUE, 'headers') } + 0 * _ + + where: + suite << testSuite() + } + + void 'test getHeaderNames'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final headers = ['header1', 'header2'] + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getHeaderNames().collect() + + then: + result == headers + 1 * mock.getHeaderNames() >> Collections.enumeration(headers) + headers.each { 1 * iastModule.taint(_, it, SourceTypes.REQUEST_HEADER_NAME, it) } + 0 * _ + + where: + suite << testSuite() + } + + void 'test getParameter'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getParameter('parameter') + + then: + result == 'value' + 1 * mock.getParameter('parameter') >> 'value' + 1 * iastModule.taint('value', SourceTypes.REQUEST_PARAMETER_VALUE, 'parameter') + 0 * _ + + where: + suite << testSuite() + } + + void 'test getParameterValues'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final values = ['value1', 'value2'] + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getParameterValues('parameter').collect() + + then: + result == values + 1 * mock.getParameterValues('parameter') >> { values as String[] } + values.each { 1 * iastModule.taint(_, it, SourceTypes.REQUEST_PARAMETER_VALUE, 'parameter') } + 0 * _ + + where: + suite << testSuite() + } + + void 'test getParameterMap'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final parameters = [parameter: ['header1', 'header2'] as String[]] + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getParameterMap() + + then: + result == parameters + 1 * mock.getParameterMap() >> parameters + parameters.each { key, values -> + 1 * iastModule.taint(_, key, SourceTypes.REQUEST_PARAMETER_NAME, key) + values.each { value -> + 1 * iastModule.taint(_, value, SourceTypes.REQUEST_PARAMETER_VALUE, key) + } + } + 0 * _ + + where: + suite << testSuite() + } + + + void 'test getParameterNames'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final parameters = ['param1', 'param2'] + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getParameterNames().collect() + + then: + result == parameters + 1 * mock.getParameterNames() >> Collections.enumeration(parameters) + parameters.each { 1 * iastModule.taint(_, it, SourceTypes.REQUEST_PARAMETER_NAME, it) } + 0 * _ + + where: + suite << testSuite() + } + + void 'test getCookies'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final cookies = [new Cookie('name1', 'value1'), new Cookie('name2', 'value2')] as Cookie[] + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getCookies() + + then: + result == cookies + 1 * mock.getCookies() >> cookies + cookies.each { 1 * iastModule.taint(_, it, SourceTypes.REQUEST_COOKIE_VALUE) } + 0 * _ + + where: + suite << testSuite() + } + + void 'test that get headers does not fail when servlet related code fails'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final enumeration = Mock(Enumeration) { + hasMoreElements() >> { throw new NuclearBomb('Boom!!!') } + } + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final headers = request.getHeaders('header') + + then: + 1 * mock.getHeaders('header') >> enumeration + noExceptionThrown() + + when: + headers.hasMoreElements() + + then: + final bomb = thrown(NuclearBomb) + bomb.stackTrace.find { it.className == TaintableEnumeration.name } == null + + where: + suite << testSuite() + } + + void 'test that get header names does not fail when servlet related code fails'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final enumeration = Mock(Enumeration) { + hasMoreElements() >> { throw new NuclearBomb('Boom!!!') } + } + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getHeaderNames() + + then: + 1 * mock.getHeaderNames() >> enumeration + noExceptionThrown() + + when: + result.hasMoreElements() + + then: + final bomb = thrown(NuclearBomb) + bomb.stackTrace.find { it.className == TaintableEnumeration.name } == null + + where: + suite << testSuite() + } + + void 'test get query string'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final queryString = 'paramName=paramValue' + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final String result = request.getQueryString() + + then: + result == queryString + 1 * mock.getQueryString() >> queryString + 1 * iastModule.taint(queryString, SourceTypes.REQUEST_QUERY) + 0 * _ + + where: + suite << testSuite() + } + + void 'test getInputStream'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final is = Mock(ServletInputStream) + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getInputStream() + + then: + result == is + 1 * mock.getInputStream() >> is + 1 * iastModule.taint(is, SourceTypes.REQUEST_BODY) + 0 * _ + + where: + suite << testSuite() + } + + void 'test getReader'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final reader = Mock(BufferedReader) + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getReader() + + then: + result == reader + 1 * mock.getReader() >> reader + 1 * iastModule.taint(reader, SourceTypes.REQUEST_BODY) + 0 * _ + + where: + suite << testSuite() + } + + void 'test getRequestDispatcher'() { + setup: + final iastModule = Mock(UnvalidatedRedirectModule) + InstrumentationBridge.registerIastModule(iastModule) + final path = 'http://dummy.location.com' + final dispatcher = Mock(RequestDispatcher) + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getRequestDispatcher(path) + + then: + result == dispatcher + 1 * mock.getRequestDispatcher(path) >> dispatcher + 1 * iastModule.onRedirect(path) + 0 * _ + + where: + suite << testSuite() + } + + void 'test getRequestURI'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final uri = 'retValue' + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getRequestURI() + + then: + result == uri + 1 * mock.getRequestURI() >> uri + 1 * iastModule.taint(uri, SourceTypes.REQUEST_PATH) + 0 * _ + + where: + suite << testSuiteCallSites() + } + + void 'test getPathInfo'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final pathInfo = 'retValue' + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getPathInfo() + + then: + result == pathInfo + 1 * mock.getPathInfo() >> pathInfo + 1 * iastModule.taint(pathInfo, SourceTypes.REQUEST_PATH) + 0 * _ + + where: + suite << testSuiteCallSites() + } + + void 'test getPathTranslated'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final pathTranslated = 'retValue' + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getPathTranslated() + + then: + result == pathTranslated + 1 * mock.getPathTranslated() >> pathTranslated + 1 * iastModule.taint(pathTranslated, SourceTypes.REQUEST_PATH) + 0 * _ + + where: + suite << testSuiteCallSites() + } + + void 'test getRequestURL'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final url = new StringBuffer('retValue') + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getRequestURL() + + then: + result == url + 1 * mock.getRequestURL() >> url + 1 * iastModule.taint(url, SourceTypes.REQUEST_URI) + 0 * _ + + where: + suite << testSuiteCallSites() + } + + private List> testSuite() { + return [ + { HttpServletRequest request -> new CustomRequest(request: request) }, + { HttpServletRequest request -> new CustomRequestWrapper(new CustomRequest(request: request)) }, + { HttpServletRequest request -> + new HttpServletRequestWrapper(new CustomRequest(request: request)) + } + ] + } + + private List> testSuiteCallSites() { + return [ + { HttpServletRequest request -> new JakartaHttpServletRequestTestSuite(request) }, + { HttpServletRequest request -> new JakartaHttpServletRequestWrapperTestSuite(new CustomRequestWrapper(request)) }, + ] + } + + private static class NuclearBomb extends RuntimeException { + NuclearBomb(final String message) { + super(message) + } + } + + private static class CustomRequest implements HttpServletRequest { + @Delegate + private HttpServletRequest request + } + + private static class CustomRequestWrapper extends HttpServletRequestWrapper { + + CustomRequestWrapper(final HttpServletRequest request) { + super(request) + } + } +} diff --git a/dd-java-agent/instrumentation/servlet/request-5/src/test/groovy/JakartaServletRequestCallSiteTest.groovy b/dd-java-agent/instrumentation/servlet/request-5/src/test/groovy/JakartaServletRequestCallSiteTest.groovy deleted file mode 100644 index 6c4682089c0..00000000000 --- a/dd-java-agent/instrumentation/servlet/request-5/src/test/groovy/JakartaServletRequestCallSiteTest.groovy +++ /dev/null @@ -1,145 +0,0 @@ -import datadog.trace.agent.test.AgentTestRunner -import datadog.trace.api.iast.InstrumentationBridge -import datadog.trace.api.iast.SourceTypes -import datadog.trace.api.iast.propagation.PropagationModule -import datadog.trace.api.iast.sink.UnvalidatedRedirectModule -import foo.bar.smoketest.JakartaHttpServletRequestTestSuite -import foo.bar.smoketest.JakartaHttpServletRequestWrapperTestSuite -import foo.bar.smoketest.JakartaServletRequestTestSuite -import foo.bar.smoketest.JakartaServletRequestWrapperTestSuite -import groovy.transform.CompileDynamic -import jakarta.servlet.ServletInputStream - -@CompileDynamic -class JakartaServletRequestCallSiteTest extends AgentTestRunner { - - @Override - protected void configurePreAgent() { - injectSysConfig("dd.iast.enabled", "true") - } - - void cleanup() { - InstrumentationBridge.clearIastModules() - } - - void 'test getParameter map'() { - setup: - final iastModule = Mock(PropagationModule) - InstrumentationBridge.registerIastModule(iastModule) - final servletRequest = Mock(clazz) - testSuite.init(servletRequest) - final map = [param1: ['value1', 'value2'] as String[]] - servletRequest.getParameterMap() >> map - - when: - final returnedMap = testSuite.getParameterMap() - - then: - returnedMap == map - map.each { key, values -> - 1 * iastModule.taint(_, key, SourceTypes.REQUEST_PARAMETER_NAME, key) - values.each { value -> - 1 * iastModule.taint(_, value, SourceTypes.REQUEST_PARAMETER_VALUE, key) - } - } - - where: - testSuite | clazz - new JakartaServletRequestTestSuite() | jakarta.servlet.ServletRequest - new JakartaHttpServletRequestTestSuite() | jakarta.servlet.http.HttpServletRequest - new JakartaServletRequestWrapperTestSuite() | jakarta.servlet.ServletRequestWrapper - new JakartaHttpServletRequestWrapperTestSuite() | jakarta.servlet.http.HttpServletRequestWrapper - } - - void 'test getParameterValues and getParameterNames'() { - setup: - final webMod = Mock(PropagationModule) - final propMod = Mock(PropagationModule) - InstrumentationBridge.registerIastModule(webMod) - InstrumentationBridge.registerIastModule(propMod) - final map = [param1: ['value1', 'value2'] as String[]] - final servletRequest = Mock(clazz) { - getParameter(_ as String) >> { map.get(it[0]).first() } - getParameterValues(_ as String) >> { map.get(it[0]) } - getParameterNames() >> { Collections.enumeration(map.keySet()) } - } - - testSuite.init(servletRequest) - - when: - testSuite.getParameter('param1') - - then: - 1 * propMod.taint('value1', SourceTypes.REQUEST_PARAMETER_VALUE, 'param1') - - when: - testSuite.getParameterValues('param1') - - then: - map['param1'].each { value -> - 1 * propMod.taint(_, value, SourceTypes.REQUEST_PARAMETER_VALUE, 'param1') - } - - when: - testSuite.getParameterNames() - - then: - map.keySet().each {param -> - 1 * propMod.taint(_, param, SourceTypes.REQUEST_PARAMETER_NAME, param) - } - - where: - testSuite | clazz - new JakartaServletRequestTestSuite() | jakarta.servlet.ServletRequest - new JakartaHttpServletRequestTestSuite() | jakarta.servlet.http.HttpServletRequest - new JakartaServletRequestWrapperTestSuite() | jakarta.servlet.ServletRequestWrapper - new JakartaHttpServletRequestWrapperTestSuite() | jakarta.servlet.http.HttpServletRequestWrapper - } - - void 'test getRequestDispatcher'() { - setup: - final iastModule = Mock(UnvalidatedRedirectModule) - InstrumentationBridge.registerIastModule(iastModule) - final servletRequest = Mock(clazz) - final path = 'http://dummy.location.com' - testSuite.init(servletRequest) - - when: - testSuite.getRequestDispatcher(path) - - then: - 1 * servletRequest.getRequestDispatcher(path) - 1 * iastModule.onRedirect(path) - - where: - testSuite | clazz - new JakartaServletRequestTestSuite() | jakarta.servlet.ServletRequest - new JakartaHttpServletRequestTestSuite() | jakarta.servlet.http.HttpServletRequest - new JakartaServletRequestWrapperTestSuite() | jakarta.servlet.ServletRequestWrapper - new JakartaHttpServletRequestWrapperTestSuite() | jakarta.servlet.http.HttpServletRequestWrapper - } - - void 'test getInputStream'() { - setup: - final iastModule = Mock(PropagationModule) - InstrumentationBridge.registerIastModule(iastModule) - final stream = Mock(ServletInputStream) - final servletRequest = Mock(clazz) { - getInputStream() >> stream - } - testSuite.init(servletRequest) - - when: - testSuite.getInputStream() - - then: - 1 * iastModule.taint(stream, SourceTypes.REQUEST_BODY) - - where: - testSuite | clazz - new JakartaServletRequestTestSuite() | jakarta.servlet.ServletRequest - new JakartaHttpServletRequestTestSuite() | jakarta.servlet.http.HttpServletRequest - new JakartaServletRequestWrapperTestSuite() | jakarta.servlet.ServletRequestWrapper - new JakartaHttpServletRequestWrapperTestSuite() | jakarta.servlet.http.HttpServletRequestWrapper - } -} diff --git a/dd-java-agent/instrumentation/servlet/request-5/src/test/java/foo/bar/smoketest/JakartaHttpServletRequestTestSuite.java b/dd-java-agent/instrumentation/servlet/request-5/src/test/java/foo/bar/smoketest/JakartaHttpServletRequestTestSuite.java index b25e9925599..41f3b87641e 100644 --- a/dd-java-agent/instrumentation/servlet/request-5/src/test/java/foo/bar/smoketest/JakartaHttpServletRequestTestSuite.java +++ b/dd-java-agent/instrumentation/servlet/request-5/src/test/java/foo/bar/smoketest/JakartaHttpServletRequestTestSuite.java @@ -1,47 +1,31 @@ package foo.bar.smoketest; -import jakarta.servlet.RequestDispatcher; -import jakarta.servlet.ServletInputStream; import jakarta.servlet.http.HttpServletRequest; -import java.io.IOException; -import java.util.Enumeration; -public class JakartaHttpServletRequestTestSuite - implements ServletRequestTestSuite { - HttpServletRequest request; +public class JakartaHttpServletRequestTestSuite implements ServletRequestTestSuite { + private final HttpServletRequest request; - @Override - public void init(HttpServletRequest request) { + public JakartaHttpServletRequestTestSuite(final HttpServletRequest request) { this.request = request; } @Override - public java.util.Map getParameterMap() { - return request.getParameterMap(); - } - - @Override - public String getParameter(String paramName) { - return request.getParameter(paramName); - } - - @Override - public String[] getParameterValues(String paramName) { - return request.getParameterValues(paramName); + public String getRequestURI() { + return request.getRequestURI(); } @Override - public Enumeration getParameterNames() { - return request.getParameterNames(); + public String getPathInfo() { + return request.getPathInfo(); } @Override - public RequestDispatcher getRequestDispatcher(String path) { - return request.getRequestDispatcher(path); + public String getPathTranslated() { + return request.getPathTranslated(); } @Override - public ServletInputStream getInputStream() throws IOException { - return request.getInputStream(); + public StringBuffer getRequestURL() { + return request.getRequestURL(); } } diff --git a/dd-java-agent/instrumentation/servlet/request-5/src/test/java/foo/bar/smoketest/JakartaHttpServletRequestWrapperTestSuite.java b/dd-java-agent/instrumentation/servlet/request-5/src/test/java/foo/bar/smoketest/JakartaHttpServletRequestWrapperTestSuite.java index 318c5a631df..80039d4cdca 100644 --- a/dd-java-agent/instrumentation/servlet/request-5/src/test/java/foo/bar/smoketest/JakartaHttpServletRequestWrapperTestSuite.java +++ b/dd-java-agent/instrumentation/servlet/request-5/src/test/java/foo/bar/smoketest/JakartaHttpServletRequestWrapperTestSuite.java @@ -1,47 +1,31 @@ package foo.bar.smoketest; -import jakarta.servlet.RequestDispatcher; -import jakarta.servlet.ServletInputStream; import jakarta.servlet.http.HttpServletRequestWrapper; -import java.io.IOException; -import java.util.Enumeration; -public class JakartaHttpServletRequestWrapperTestSuite - implements ServletRequestTestSuite { - HttpServletRequestWrapper request; +public class JakartaHttpServletRequestWrapperTestSuite implements ServletRequestTestSuite { + private final HttpServletRequestWrapper request; - @Override - public void init(HttpServletRequestWrapper request) { + public JakartaHttpServletRequestWrapperTestSuite(final HttpServletRequestWrapper request) { this.request = request; } @Override - public java.util.Map getParameterMap() { - return request.getParameterMap(); - } - - @Override - public String getParameter(String paramName) { - return request.getParameter(paramName); - } - - @Override - public String[] getParameterValues(String paramName) { - return request.getParameterValues(paramName); + public String getRequestURI() { + return request.getRequestURI(); } @Override - public Enumeration getParameterNames() { - return request.getParameterNames(); + public String getPathInfo() { + return request.getPathInfo(); } @Override - public RequestDispatcher getRequestDispatcher(String path) { - return request.getRequestDispatcher(path); + public String getPathTranslated() { + return request.getPathTranslated(); } @Override - public ServletInputStream getInputStream() throws IOException { - return request.getInputStream(); + public StringBuffer getRequestURL() { + return request.getRequestURL(); } } diff --git a/dd-java-agent/instrumentation/servlet/request-5/src/test/java/foo/bar/smoketest/JakartaServletRequestTestSuite.java b/dd-java-agent/instrumentation/servlet/request-5/src/test/java/foo/bar/smoketest/JakartaServletRequestTestSuite.java deleted file mode 100644 index 486af8292e8..00000000000 --- a/dd-java-agent/instrumentation/servlet/request-5/src/test/java/foo/bar/smoketest/JakartaServletRequestTestSuite.java +++ /dev/null @@ -1,46 +0,0 @@ -package foo.bar.smoketest; - -import jakarta.servlet.RequestDispatcher; -import jakarta.servlet.ServletInputStream; -import jakarta.servlet.ServletRequest; -import java.io.IOException; -import java.util.Enumeration; - -public class JakartaServletRequestTestSuite implements ServletRequestTestSuite { - ServletRequest request; - - @Override - public void init(ServletRequest request) { - this.request = request; - } - - @Override - public java.util.Map getParameterMap() { - return request.getParameterMap(); - } - - @Override - public String getParameter(String paramName) { - return request.getParameter(paramName); - } - - @Override - public String[] getParameterValues(String paramName) { - return request.getParameterValues(paramName); - } - - @Override - public Enumeration getParameterNames() { - return request.getParameterNames(); - } - - @Override - public RequestDispatcher getRequestDispatcher(String path) { - return request.getRequestDispatcher(path); - } - - @Override - public ServletInputStream getInputStream() throws IOException { - return request.getInputStream(); - } -} diff --git a/dd-java-agent/instrumentation/servlet/request-5/src/test/java/foo/bar/smoketest/JakartaServletRequestWrapperTestSuite.java b/dd-java-agent/instrumentation/servlet/request-5/src/test/java/foo/bar/smoketest/JakartaServletRequestWrapperTestSuite.java deleted file mode 100644 index 710cc7a4d37..00000000000 --- a/dd-java-agent/instrumentation/servlet/request-5/src/test/java/foo/bar/smoketest/JakartaServletRequestWrapperTestSuite.java +++ /dev/null @@ -1,47 +0,0 @@ -package foo.bar.smoketest; - -import jakarta.servlet.RequestDispatcher; -import jakarta.servlet.ServletInputStream; -import jakarta.servlet.ServletRequestWrapper; -import java.io.IOException; -import java.util.Enumeration; - -public class JakartaServletRequestWrapperTestSuite - implements ServletRequestTestSuite { - ServletRequestWrapper request; - - @Override - public void init(ServletRequestWrapper request) { - this.request = request; - } - - @Override - public java.util.Map getParameterMap() { - return request.getParameterMap(); - } - - @Override - public String getParameter(String paramName) { - return request.getParameter(paramName); - } - - @Override - public String[] getParameterValues(String paramName) { - return request.getParameterValues(paramName); - } - - @Override - public Enumeration getParameterNames() { - return request.getParameterNames(); - } - - @Override - public RequestDispatcher getRequestDispatcher(String path) { - return request.getRequestDispatcher(path); - } - - @Override - public ServletInputStream getInputStream() throws IOException { - return request.getInputStream(); - } -} diff --git a/dd-java-agent/instrumentation/servlet/request-5/src/test/java/foo/bar/smoketest/ServletRequestTestSuite.java b/dd-java-agent/instrumentation/servlet/request-5/src/test/java/foo/bar/smoketest/ServletRequestTestSuite.java index 0fd50c559d8..c1e5e45cde4 100644 --- a/dd-java-agent/instrumentation/servlet/request-5/src/test/java/foo/bar/smoketest/ServletRequestTestSuite.java +++ b/dd-java-agent/instrumentation/servlet/request-5/src/test/java/foo/bar/smoketest/ServletRequestTestSuite.java @@ -1,23 +1,12 @@ package foo.bar.smoketest; -import jakarta.servlet.RequestDispatcher; -import jakarta.servlet.ServletInputStream; -import java.io.IOException; -import java.util.Enumeration; +public interface ServletRequestTestSuite { -public interface ServletRequestTestSuite { + String getRequestURI(); - void init(final E request); + String getPathInfo(); - java.util.Map getParameterMap(); + String getPathTranslated(); - String getParameter(String paramName); - - String[] getParameterValues(String paramName); - - Enumeration getParameterNames(); - - RequestDispatcher getRequestDispatcher(String path); - - ServletInputStream getInputStream() throws IOException; + StringBuffer getRequestURL(); } diff --git a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletRequestCallSite.java b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletRequestCallSite.java new file mode 100644 index 00000000000..fce80bb78bc --- /dev/null +++ b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletRequestCallSite.java @@ -0,0 +1,59 @@ +package datadog.trace.instrumentation.servlet.http; + +import datadog.trace.agent.tooling.csi.CallSite; +import datadog.trace.api.iast.IastCallSites; +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 javax.servlet.http.HttpServletRequest; + +/** + * Calls to these methods are often triggered outside of customer code, we use call sites to avoid + * all these unwanted tainting operations + */ +@CallSite(spi = IastCallSites.class) +public class HttpServletRequestCallSite { + + @Source(SourceTypes.REQUEST_PATH) + @CallSite.After("java.lang.String javax.servlet.http.HttpServletRequest.getRequestURI()") + @CallSite.After("java.lang.String javax.servlet.http.HttpServletRequestWrapper.getRequestURI()") + @CallSite.After("java.lang.String javax.servlet.http.HttpServletRequest.getPathInfo()") + @CallSite.After("java.lang.String javax.servlet.http.HttpServletRequestWrapper.getPathInfo()") + @CallSite.After("java.lang.String javax.servlet.http.HttpServletRequest.getPathTranslated()") + @CallSite.After( + "java.lang.String javax.servlet.http.HttpServletRequestWrapper.getPathTranslated()") + public static String afterPath( + @CallSite.This final HttpServletRequest self, @CallSite.Return final String retValue) { + if (null != retValue && !retValue.isEmpty()) { + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module != null) { + try { + module.taint(retValue, SourceTypes.REQUEST_PATH); + } catch (final Throwable e) { + module.onUnexpectedException("afterPath threw", e); + } + } + } + return retValue; + } + + @Source(SourceTypes.REQUEST_URI) + @CallSite.After("java.lang.StringBuffer javax.servlet.http.HttpServletRequest.getRequestURL()") + @CallSite.After( + "java.lang.StringBuffer javax.servlet.http.HttpServletRequestWrapper.getRequestURL()") + public static StringBuffer afterGetRequestURL( + @CallSite.This final HttpServletRequest self, @CallSite.Return final StringBuffer retValue) { + if (null != retValue && retValue.length() > 0) { + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module != null) { + try { + module.taint(retValue, SourceTypes.REQUEST_URI); + } catch (final Throwable e) { + module.onUnexpectedException("afterGetRequestURL threw", e); + } + } + } + return retValue; + } +} diff --git a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletRequestInstrumentation.java b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletRequestInstrumentation.java new file mode 100644 index 00000000000..952e95f4409 --- /dev/null +++ b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/http/HttpServletRequestInstrumentation.java @@ -0,0 +1,285 @@ +package datadog.trace.instrumentation.servlet.http; + +import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.extendsClass; +import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.implementsInterface; +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; +import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.namedOneOf; +import static net.bytebuddy.matcher.ElementMatchers.isMethod; +import static net.bytebuddy.matcher.ElementMatchers.not; +import static net.bytebuddy.matcher.ElementMatchers.takesArguments; + +import com.google.auto.service.AutoService; +import datadog.trace.agent.tooling.Instrumenter; +import datadog.trace.agent.tooling.iast.TaintableEnumeration; +import datadog.trace.api.iast.IastContext; +import datadog.trace.api.iast.InstrumentationBridge; +import datadog.trace.api.iast.Sink; +import datadog.trace.api.iast.Source; +import datadog.trace.api.iast.SourceTypes; +import datadog.trace.api.iast.VulnerabilityTypes; +import datadog.trace.api.iast.propagation.PropagationModule; +import datadog.trace.api.iast.sink.UnvalidatedRedirectModule; +import java.util.Enumeration; +import java.util.Map; +import javax.servlet.http.Cookie; +import net.bytebuddy.asm.Advice; +import net.bytebuddy.description.type.TypeDescription; +import net.bytebuddy.matcher.ElementMatcher; + +@SuppressWarnings("unused") +@AutoService(Instrumenter.class) +public class HttpServletRequestInstrumentation extends Instrumenter.Iast + implements Instrumenter.ForTypeHierarchy { + + private static final String CLASS_NAME = HttpServletRequestInstrumentation.class.getName(); + private static final ElementMatcher.Junction WRAPPER_CLASS = + named("javax.servlet.http.HttpServletRequestWrapper"); + + public HttpServletRequestInstrumentation() { + super("servlet", "servlet-request"); + } + + @Override + public String hierarchyMarkerType() { + return "javax.servlet.http.HttpServletRequest"; + } + + @Override + public ElementMatcher hierarchyMatcher() { + return implementsInterface(named(hierarchyMarkerType())) + .and(not(WRAPPER_CLASS)) + .and(not(extendsClass(WRAPPER_CLASS))); + } + + @Override + public String[] helperClassNames() { + return new String[] {"datadog.trace.agent.tooling.iast.TaintableEnumeration"}; + } + + @Override + public void adviceTransformations(AdviceTransformation transformation) { + transformation.applyAdvice( + isMethod().and(named("getHeader")).and(takesArguments(String.class)), + CLASS_NAME + "$GetHeaderAdvice"); + transformation.applyAdvice( + isMethod().and(named("getHeaders")).and(takesArguments(String.class)), + CLASS_NAME + "$GetHeadersAdvice"); + transformation.applyAdvice( + isMethod().and(named("getHeaderNames")).and(takesArguments(0)), + CLASS_NAME + "$GetHeaderNamesAdvice"); + transformation.applyAdvice( + isMethod().and(named("getParameter")).and(takesArguments(String.class)), + CLASS_NAME + "$GetParameterAdvice"); + transformation.applyAdvice( + isMethod().and(named("getParameterValues")).and(takesArguments(String.class)), + CLASS_NAME + "$GetParameterValuesAdvice"); + transformation.applyAdvice( + isMethod().and(named("getParameterMap")).and(takesArguments(0)), + CLASS_NAME + "$GetParameterMapAdvice"); + transformation.applyAdvice( + isMethod().and(named("getParameterNames")).and(takesArguments(0)), + CLASS_NAME + "$GetParameterNamesAdvice"); + transformation.applyAdvice( + isMethod().and(named("getCookies")).and(takesArguments(0)), + CLASS_NAME + "$GetCookiesAdvice"); + transformation.applyAdvice( + isMethod().and(named("getQueryString")).and(takesArguments(0)), + CLASS_NAME + "$GetQueryStringAdvice"); + transformation.applyAdvice( + isMethod().and(namedOneOf("getInputStream", "getReader")).and(takesArguments(0)), + CLASS_NAME + "$GetBodyAdvice"); + transformation.applyAdvice( + isMethod().and(named("getRequestDispatcher")).and(takesArguments(String.class)), + CLASS_NAME + "$GetRequestDispatcherAdvice"); + } + + public static class GetHeaderAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Source(SourceTypes.REQUEST_HEADER_VALUE) + public static void onExit( + @Advice.Argument(0) final String name, @Advice.Return final String value) { + if (value == null) { + return; + } + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module == null) { + return; + } + module.taint(value, SourceTypes.REQUEST_HEADER_VALUE, name); + } + } + + public static class GetHeadersAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Source(SourceTypes.REQUEST_HEADER_VALUE) + public static void onExit( + @Advice.Argument(0) final String name, + @Advice.Return(readOnly = false) Enumeration enumeration) { + if (enumeration == null) { + return; + } + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module == null) { + return; + } + enumeration = + TaintableEnumeration.wrap(enumeration, module, SourceTypes.REQUEST_HEADER_VALUE, name); + } + } + + public static class GetHeaderNamesAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Source(SourceTypes.REQUEST_HEADER_NAME) + public static void onExit(@Advice.Return(readOnly = false) Enumeration enumeration) { + if (enumeration == null) { + return; + } + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module == null) { + return; + } + enumeration = + TaintableEnumeration.wrap(enumeration, module, SourceTypes.REQUEST_HEADER_NAME, true); + } + } + + public static class GetParameterAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Source(SourceTypes.REQUEST_PARAMETER_VALUE) + public static void onExit( + @Advice.Argument(0) final String name, @Advice.Return final String value) { + if (value == null) { + return; + } + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module == null) { + return; + } + module.taint(value, SourceTypes.REQUEST_PARAMETER_VALUE, name); + } + } + + public static class GetParameterValuesAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Source(SourceTypes.REQUEST_PARAMETER_VALUE) + public static void onExit( + @Advice.Argument(0) final String name, @Advice.Return final String[] values) { + if (values == null || values.length == 0) { + return; + } + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module == null) { + return; + } + final IastContext ctx = IastContext.Provider.get(); + for (final String value : values) { + module.taint(ctx, value, SourceTypes.REQUEST_PARAMETER_VALUE, name); + } + } + } + + public static class GetParameterMapAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Source(SourceTypes.REQUEST_PARAMETER_VALUE) + public static void onExit(@Advice.Return final Map parameters) { + if (parameters == null || parameters.isEmpty()) { + return; + } + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module == null) { + return; + } + final IastContext ctx = IastContext.Provider.get(); + for (final Map.Entry entry : parameters.entrySet()) { + final String name = entry.getKey(); + module.taint(ctx, name, SourceTypes.REQUEST_PARAMETER_NAME, name); + final String[] values = entry.getValue(); + if (values != null) { + for (final String value : entry.getValue()) { + module.taint(ctx, value, SourceTypes.REQUEST_PARAMETER_VALUE, name); + } + } + } + } + } + + public static class GetParameterNamesAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Source(SourceTypes.REQUEST_PARAMETER_NAME) + public static void onExit(@Advice.Return(readOnly = false) Enumeration enumeration) { + if (enumeration == null) { + return; + } + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module == null) { + return; + } + enumeration = + TaintableEnumeration.wrap(enumeration, module, SourceTypes.REQUEST_PARAMETER_NAME, true); + } + } + + public static class GetCookiesAdvice { + + @Advice.OnMethodExit(suppress = Throwable.class) + @Source(SourceTypes.REQUEST_COOKIE_VALUE) + public static void onExit(@Advice.Return final Cookie[] cookies) { + if (cookies == null || cookies.length == 0) { + return; + } + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module == null) { + return; + } + final IastContext ctx = IastContext.Provider.get(); + for (final Cookie cookie : cookies) { + module.taint(ctx, cookie, SourceTypes.REQUEST_COOKIE_VALUE); + } + } + } + + public static class GetQueryStringAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Source(SourceTypes.REQUEST_QUERY) + public static void onExit(@Advice.Return final String queryString) { + if (queryString == null) { + return; + } + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module == null) { + return; + } + module.taint(queryString, SourceTypes.REQUEST_QUERY); + } + } + + public static class GetBodyAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Source(SourceTypes.REQUEST_BODY) + public static void onExit(@Advice.Return final Object body) { + if (body == null) { + return; + } + final PropagationModule module = InstrumentationBridge.PROPAGATION; + if (module == null) { + return; + } + module.taint(body, SourceTypes.REQUEST_BODY); + } + } + + public static class GetRequestDispatcherAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) + @Sink(VulnerabilityTypes.UNVALIDATED_REDIRECT) + public static void onExit(@Advice.Argument(0) final String path) { + if (path == null) { + return; + } + final UnvalidatedRedirectModule module = InstrumentationBridge.UNVALIDATED_REDIRECT; + if (module == null) { + return; + } + module.onRedirect(path); + } + } +} diff --git a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/request/HttpServletRequestCallSite.java b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/request/HttpServletRequestCallSite.java deleted file mode 100644 index b71af2a7aba..00000000000 --- a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/request/HttpServletRequestCallSite.java +++ /dev/null @@ -1,304 +0,0 @@ -package datadog.trace.instrumentation.servlet.request; - -import datadog.trace.agent.tooling.csi.CallSite; -import datadog.trace.api.iast.IastCallSites; -import datadog.trace.api.iast.IastContext; -import datadog.trace.api.iast.InstrumentationBridge; -import datadog.trace.api.iast.Sink; -import datadog.trace.api.iast.Source; -import datadog.trace.api.iast.SourceTypes; -import datadog.trace.api.iast.VulnerabilityTypes; -import datadog.trace.api.iast.propagation.PropagationModule; -import datadog.trace.api.iast.sink.UnvalidatedRedirectModule; -import datadog.trace.util.stacktrace.StackUtils; -import java.io.BufferedReader; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Enumeration; -import java.util.List; -import javax.servlet.http.Cookie; -import javax.servlet.http.HttpServletRequest; - -@CallSite(spi = IastCallSites.class) -public class HttpServletRequestCallSite { - - @Source(SourceTypes.REQUEST_HEADER_VALUE) - @CallSite.After( - "java.lang.String javax.servlet.http.HttpServletRequest.getHeader(java.lang.String)") - @CallSite.After( - "java.lang.String javax.servlet.http.HttpServletRequestWrapper.getHeader(java.lang.String)") - public static String afterGetHeader( - @CallSite.This final HttpServletRequest self, - @CallSite.Argument final String name, - @CallSite.Return final String value) { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null) { - try { - module.taint(value, SourceTypes.REQUEST_HEADER_VALUE, name); - } catch (final Throwable e) { - module.onUnexpectedException("afterGetHeader threw", e); - } - } - return value; - } - - @Source(SourceTypes.REQUEST_HEADER_VALUE) - @CallSite.After( - "java.util.Enumeration javax.servlet.http.HttpServletRequest.getHeaders(java.lang.String)") - @CallSite.After( - "java.util.Enumeration javax.servlet.http.HttpServletRequestWrapper.getHeaders(java.lang.String)") - public static Enumeration afterGetHeaders( - @CallSite.This final HttpServletRequest self, - @CallSite.Argument final String headerName, - @CallSite.Return final Enumeration enumeration) - throws Throwable { - if (enumeration == null) { - return null; - } - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module == null) { - return enumeration; - } - try { - final List headerValues = new ArrayList<>(); - while (enumeration.hasMoreElements()) { - final String headerValue = enumeration.nextElement(); - headerValues.add(headerValue); - } - try { - final IastContext ctx = IastContext.Provider.get(); - for (final String value : headerValues) { - module.taint(ctx, value, SourceTypes.REQUEST_HEADER_VALUE, headerName); - } - } catch (final Throwable e) { - module.onUnexpectedException("afterGetHeaders threw", e); - } - return Collections.enumeration(headerValues); - } catch (final Throwable e) { - module.onUnexpectedException("afterGetHeaders threw while iterating headers", e); - throw StackUtils.filterFirstDatadog(e); - } - } - - @Source(SourceTypes.REQUEST_HEADER_NAME) - @CallSite.After("java.util.Enumeration javax.servlet.http.HttpServletRequest.getHeaderNames()") - @CallSite.After( - "java.util.Enumeration javax.servlet.http.HttpServletRequestWrapper.getHeaderNames()") - public static Enumeration afterGetHeaderNames( - @CallSite.This final HttpServletRequest self, - @CallSite.Return final Enumeration enumeration) - throws Throwable { - if (enumeration == null) { - return null; - } - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module == null) { - return enumeration; - } - try { - final List headerNames = new ArrayList<>(); - while (enumeration.hasMoreElements()) { - final String headerName = enumeration.nextElement(); - headerNames.add(headerName); - } - try { - final IastContext ctx = IastContext.Provider.get(); - for (final String name : headerNames) { - module.taint(ctx, name, SourceTypes.REQUEST_HEADER_NAME, name); - } - } catch (final Throwable e) { - module.onUnexpectedException("afterGetHeaderNames threw", e); - } - return Collections.enumeration(headerNames); - } catch (final Throwable e) { - module.onUnexpectedException("afterGetHeaderNames threw while iterating header names", e); - throw StackUtils.filterFirstDatadog(e); - } - } - - @Source(SourceTypes.REQUEST_COOKIE_VALUE) - @CallSite.After("javax.servlet.http.Cookie[] javax.servlet.http.HttpServletRequest.getCookies()") - @CallSite.After( - "javax.servlet.http.Cookie[] javax.servlet.http.HttpServletRequestWrapper.getCookies()") - public static Cookie[] afterGetCookies( - @CallSite.This final HttpServletRequest self, @CallSite.Return final Cookie[] cookies) { - if (null != cookies && cookies.length > 0) { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null) { - try { - final IastContext ctx = IastContext.Provider.get(); - for (final Cookie cookie : cookies) { - module.taint(ctx, cookie, SourceTypes.REQUEST_COOKIE_VALUE); - } - } catch (final Throwable e) { - module.onUnexpectedException("afterGetCookies threw", e); - } - } - } - return cookies; - } - - @Source(SourceTypes.REQUEST_QUERY) - @CallSite.After("java.lang.String javax.servlet.http.HttpServletRequest.getQueryString()") - @CallSite.After("java.lang.String javax.servlet.http.HttpServletRequestWrapper.getQueryString()") - public static String afterGetQueryString( - @CallSite.This final HttpServletRequest self, @CallSite.Return final String queryString) { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null) { - try { - module.taint(queryString, SourceTypes.REQUEST_QUERY); - } catch (final Throwable e) { - module.onUnexpectedException("afterGetQueryString threw", e); - } - } - return queryString; - } - - @Source(SourceTypes.REQUEST_PARAMETER_VALUE) - @CallSite.After( - "java.lang.String javax.servlet.http.HttpServletRequest.getParameter(java.lang.String)") - @CallSite.After( - "java.lang.String javax.servlet.http.HttpServletRequestWrapper.getParameter(java.lang.String)") - public static String afterGetParameter( - @CallSite.This final HttpServletRequest self, - @CallSite.Argument final String name, - @CallSite.Return final String value) { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null) { - try { - module.taint(value, SourceTypes.REQUEST_PARAMETER_VALUE, name); - } catch (final Throwable e) { - module.onUnexpectedException("afterGetParameter threw", e); - } - } - return value; - } - - @Source(SourceTypes.REQUEST_PARAMETER_NAME) - @CallSite.After("java.util.Enumeration javax.servlet.http.HttpServletRequest.getParameterNames()") - @CallSite.After( - "java.util.Enumeration javax.servlet.http.HttpServletRequestWrapper.getParameterNames()") - public static Enumeration afterGetParameterNames( - @CallSite.This final HttpServletRequest self, - @CallSite.Return final Enumeration enumeration) - throws Throwable { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module == null || enumeration == null) { - return enumeration; - } - try { - final List parameterNames = new ArrayList<>(); - while (enumeration.hasMoreElements()) { - final String paramName = enumeration.nextElement(); - parameterNames.add(paramName); - } - try { - final IastContext ctx = IastContext.Provider.get(); - for (final String name : parameterNames) { - module.taint(ctx, name, SourceTypes.REQUEST_PARAMETER_NAME, name); - } - } catch (final Throwable e) { - module.onUnexpectedException("afterGetParameterNames threw", e); - } - return Collections.enumeration(parameterNames); - } catch (final Throwable e) { - module.onUnexpectedException( - "afterGetParameterNames threw while iterating parameter names", e); - throw StackUtils.filterFirstDatadog(e); - } - } - - @Source(SourceTypes.REQUEST_PARAMETER_VALUE) - @CallSite.After( - "java.lang.String[] javax.servlet.http.HttpServletRequest.getParameterValues(java.lang.String)") - @CallSite.After( - "java.lang.String[] javax.servlet.http.HttpServletRequestWrapper.getParameterValues(java.lang.String)") - public static String[] afterGetParameterValues( - @CallSite.This final HttpServletRequest self, - @CallSite.Argument final String paramName, - @CallSite.Return final String[] parameterValues) { - if (null != parameterValues && parameterValues.length > 0) { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null) { - try { - final IastContext ctx = IastContext.Provider.get(); - for (final String value : parameterValues) { - module.taint(ctx, value, SourceTypes.REQUEST_PARAMETER_VALUE, paramName); - } - } catch (final Throwable e) { - module.onUnexpectedException("afterGetParameterValues threw", e); - } - } - } - return parameterValues; - } - - @Source(SourceTypes.REQUEST_BODY) - @CallSite.After("java.io.BufferedReader javax.servlet.http.HttpServletRequest.getReader()") - @CallSite.After("java.io.BufferedReader javax.servlet.http.HttpServletRequestWrapper.getReader()") - public static BufferedReader afterGetReader( - @CallSite.This final HttpServletRequest self, - @CallSite.Return final BufferedReader bufferedReader) { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null && bufferedReader != null) { - try { - module.taint(bufferedReader, SourceTypes.REQUEST_BODY); - } catch (final Throwable e) { - module.onUnexpectedException("afterGetReader threw", e); - } - } - return bufferedReader; - } - - @Sink(VulnerabilityTypes.UNVALIDATED_REDIRECT) - @CallSite.Before( - "javax.servlet.RequestDispatcher javax.servlet.http.HttpServletRequest.getRequestDispatcher(java.lang.String)") - @CallSite.Before( - "javax.servlet.RequestDispatcher javax.servlet.http.HttpServletRequestWrapper.getRequestDispatcher(java.lang.String)") - public static void beforeRequestDispatcher(@CallSite.Argument final String path) { - final UnvalidatedRedirectModule module = InstrumentationBridge.UNVALIDATED_REDIRECT; - if (module != null && path != null) { - try { - module.onRedirect(path); - } catch (final Throwable e) { - module.onUnexpectedException("beforeRequestDispatcher threw", e); - } - } - } - - @Source(SourceTypes.REQUEST_URI) - @CallSite.After("java.lang.StringBuffer javax.servlet.http.HttpServletRequest.getRequestURL()") - public static StringBuffer afterGetRequestURL( - @CallSite.This final HttpServletRequest self, @CallSite.Return final StringBuffer retValue) { - if (null != retValue && retValue.length() > 0) { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null) { - try { - module.taint(retValue, SourceTypes.REQUEST_URI); - } catch (final Throwable e) { - module.onUnexpectedException("afterGetRequestURL threw", e); - } - } - } - return retValue; - } - - @Source(SourceTypes.REQUEST_PATH) - @CallSite.After("java.lang.String javax.servlet.http.HttpServletRequest.getRequestURI()") - @CallSite.After("java.lang.String javax.servlet.http.HttpServletRequest.getPathInfo()") - @CallSite.After("java.lang.String javax.servlet.http.HttpServletRequest.getPathTranslated()") - public static String afterGetPath( - @CallSite.This final HttpServletRequest self, @CallSite.Return final String retValue) { - if (null != retValue && !retValue.isEmpty()) { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null) { - try { - module.taint(retValue, SourceTypes.REQUEST_PATH); - } catch (final Throwable e) { - module.onUnexpectedException("afterGetPathInfo threw", e); - } - } - } - return retValue; - } -} diff --git a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/request/HttpServletRequestInputStreamCallSite.java b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/request/HttpServletRequestInputStreamCallSite.java deleted file mode 100644 index 8a909d75951..00000000000 --- a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/request/HttpServletRequestInputStreamCallSite.java +++ /dev/null @@ -1,33 +0,0 @@ -package datadog.trace.instrumentation.servlet.request; - -import datadog.trace.agent.tooling.csi.CallSite; -import datadog.trace.api.iast.IastCallSites; -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 javax.servlet.ServletInputStream; -import javax.servlet.http.HttpServletRequest; - -@CallSite(spi = IastCallSites.class) -public class HttpServletRequestInputStreamCallSite { - - @Source(SourceTypes.REQUEST_BODY) - @CallSite.After( - "javax.servlet.ServletInputStream javax.servlet.http.HttpServletRequest.getInputStream()") - @CallSite.After( - "javax.servlet.ServletInputStream javax.servlet.http.HttpServletRequestWrapper.getInputStream()") - public static ServletInputStream afterGetInputStream( - @CallSite.This final HttpServletRequest self, - @CallSite.Return final ServletInputStream inputStream) { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null && inputStream != null) { - try { - module.taint(inputStream, SourceTypes.REQUEST_BODY); - } catch (final Throwable e) { - module.onUnexpectedException("afterGetInputStream threw", e); - } - } - return inputStream; - } -} diff --git a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/request/ServletRequestCallSite.java b/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/request/ServletRequestCallSite.java deleted file mode 100644 index 31ffdf42e19..00000000000 --- a/dd-java-agent/instrumentation/servlet/src/main/java/datadog/trace/instrumentation/servlet/request/ServletRequestCallSite.java +++ /dev/null @@ -1,152 +0,0 @@ -package datadog.trace.instrumentation.servlet.request; - -import datadog.trace.agent.tooling.csi.CallSite; -import datadog.trace.api.iast.IastCallSites; -import datadog.trace.api.iast.IastContext; -import datadog.trace.api.iast.InstrumentationBridge; -import datadog.trace.api.iast.Sink; -import datadog.trace.api.iast.Source; -import datadog.trace.api.iast.SourceTypes; -import datadog.trace.api.iast.VulnerabilityTypes; -import datadog.trace.api.iast.propagation.PropagationModule; -import datadog.trace.api.iast.sink.UnvalidatedRedirectModule; -import datadog.trace.util.stacktrace.StackUtils; -import java.io.BufferedReader; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Enumeration; -import java.util.List; -import javax.servlet.ServletInputStream; -import javax.servlet.ServletRequest; - -@CallSite(spi = IastCallSites.class) -public class ServletRequestCallSite { - - @Source(SourceTypes.REQUEST_PARAMETER_VALUE) - @CallSite.After("java.lang.String javax.servlet.ServletRequest.getParameter(java.lang.String)") - @CallSite.After( - "java.lang.String javax.servlet.ServletRequestWrapper.getParameter(java.lang.String)") - public static String afterGetParameter( - @CallSite.This final ServletRequest self, - @CallSite.Argument final String name, - @CallSite.Return final String value) { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null) { - try { - module.taint(value, SourceTypes.REQUEST_PARAMETER_VALUE, name); - } catch (final Throwable e) { - module.onUnexpectedException("afterGetParameter threw", e); - } - } - return value; - } - - @Source(SourceTypes.REQUEST_PARAMETER_NAME) - @CallSite.After("java.util.Enumeration javax.servlet.ServletRequest.getParameterNames()") - @CallSite.After("java.util.Enumeration javax.servlet.ServletRequestWrapper.getParameterNames()") - public static Enumeration afterGetParameterNames( - @CallSite.This final ServletRequest self, - @CallSite.Return final Enumeration enumeration) - throws Throwable { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module == null || enumeration == null) { - return enumeration; - } - try { - final List parameterNames = new ArrayList<>(); - while (enumeration.hasMoreElements()) { - final String paramName = enumeration.nextElement(); - parameterNames.add(paramName); - } - try { - final IastContext ctx = IastContext.Provider.get(); - for (final String name : parameterNames) { - module.taint(ctx, name, SourceTypes.REQUEST_PARAMETER_NAME, name); - } - } catch (final Throwable e) { - module.onUnexpectedException("afterGetParameterNames threw", e); - } - return Collections.enumeration(parameterNames); - } catch (final Throwable e) { - module.onUnexpectedException( - "afterGetParameterNames threw while iterating parameter names", e); - throw StackUtils.filterFirstDatadog(e); - } - } - - @Source(SourceTypes.REQUEST_PARAMETER_VALUE) - @CallSite.After( - "java.lang.String[] javax.servlet.ServletRequest.getParameterValues(java.lang.String)") - @CallSite.After( - "java.lang.String[] javax.servlet.ServletRequestWrapper.getParameterValues(java.lang.String)") - public static String[] afterGetParameterValues( - @CallSite.This final ServletRequest self, - @CallSite.Argument final String paramName, - @CallSite.Return final String[] parameterValues) { - if (null != parameterValues && parameterValues.length > 0) { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null) { - try { - final IastContext ctx = IastContext.Provider.get(); - for (final String value : parameterValues) { - module.taint(ctx, value, SourceTypes.REQUEST_PARAMETER_VALUE, paramName); - } - } catch (final Throwable e) { - module.onUnexpectedException("afterGetParameterValues threw", e); - } - } - } - return parameterValues; - } - - @Source(SourceTypes.REQUEST_BODY) - @CallSite.After("javax.servlet.ServletInputStream javax.servlet.ServletRequest.getInputStream()") - @CallSite.After( - "javax.servlet.ServletInputStream javax.servlet.ServletRequestWrapper.getInputStream()") - public static ServletInputStream afterGetInputStream( - @CallSite.This final ServletRequest self, - @CallSite.Return final ServletInputStream inputStream) { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null && inputStream != null) { - try { - module.taint(inputStream, SourceTypes.REQUEST_BODY); - } catch (final Throwable e) { - module.onUnexpectedException("afterGetInputStream threw", e); - } - } - return inputStream; - } - - @Source(SourceTypes.REQUEST_BODY) - @CallSite.After("java.io.BufferedReader javax.servlet.ServletRequest.getReader()") - @CallSite.After("java.io.BufferedReader javax.servlet.ServletRequestWrapper.getReader()") - public static BufferedReader afterGetReader( - @CallSite.This final ServletRequest self, - @CallSite.Return final BufferedReader bufferedReader) { - final PropagationModule module = InstrumentationBridge.PROPAGATION; - if (module != null && bufferedReader != null) { - try { - module.taint(bufferedReader, SourceTypes.REQUEST_BODY); - } catch (final Throwable e) { - module.onUnexpectedException("afterGetReader threw", e); - } - } - return bufferedReader; - } - - @Sink(VulnerabilityTypes.UNVALIDATED_REDIRECT) - @CallSite.Before( - "javax.servlet.RequestDispatcher javax.servlet.ServletRequest.getRequestDispatcher(java.lang.String)") - @CallSite.Before( - "javax.servlet.RequestDispatcher javax.servlet.ServletRequestWrapper.getRequestDispatcher(java.lang.String)") - public static void beforeRequestDispatcher(@CallSite.Argument final String path) { - final UnvalidatedRedirectModule module = InstrumentationBridge.UNVALIDATED_REDIRECT; - if (module != null && path != null) { - try { - module.onRedirect(path); - } catch (final Throwable e) { - module.onUnexpectedException("beforeRequestDispatcher threw", e); - } - } - } -} diff --git a/dd-java-agent/instrumentation/servlet/src/test/groovy/CookieInstrumentationTest.groovy b/dd-java-agent/instrumentation/servlet/src/test/groovy/CookieInstrumentationTest.groovy index f531d094187..2d7173c8446 100644 --- a/dd-java-agent/instrumentation/servlet/src/test/groovy/CookieInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/servlet/src/test/groovy/CookieInstrumentationTest.groovy @@ -1,10 +1,11 @@ -import datadog.smoketest.controller.CookieTestSuite import datadog.trace.agent.test.AgentTestRunner import datadog.trace.api.iast.InstrumentationBridge import datadog.trace.api.iast.SourceTypes import datadog.trace.api.iast.propagation.PropagationModule import groovy.transform.CompileDynamic +import javax.servlet.http.Cookie + @CompileDynamic class CookieInstrumentationTest extends AgentTestRunner { @@ -20,27 +21,27 @@ class CookieInstrumentationTest extends AgentTestRunner { setup: final iastModule = Mock(PropagationModule) InstrumentationBridge.registerIastModule(iastModule) - final cookieTestSuite = new CookieTestSuite(NAME, VALUE) + final cookie = new Cookie(NAME, VALUE) when: - final result = cookieTestSuite.getName() + final result = cookie.getName() then: result == NAME - 1 * iastModule.taintIfTainted(NAME, cookieTestSuite.getCookie(), SourceTypes.REQUEST_COOKIE_NAME, NAME) + 1 * iastModule.taintIfTainted(NAME, cookie, SourceTypes.REQUEST_COOKIE_NAME, NAME) } void 'test getValue'() { setup: final iastModule = Mock(PropagationModule) InstrumentationBridge.registerIastModule(iastModule) - final cookieTestSuite = new CookieTestSuite(NAME, VALUE) + final cookie = new Cookie(NAME, VALUE) when: - final result = cookieTestSuite.getValue() + final result = cookie.getValue() then: result == VALUE - 1 * iastModule.taintIfTainted(VALUE, cookieTestSuite.getCookie(), SourceTypes.REQUEST_COOKIE_VALUE, NAME) + 1 * iastModule.taintIfTainted(VALUE, cookie, SourceTypes.REQUEST_COOKIE_VALUE, NAME) } } diff --git a/dd-java-agent/instrumentation/servlet/src/test/groovy/HttpServletRequestCallSiteTest.groovy b/dd-java-agent/instrumentation/servlet/src/test/groovy/HttpServletRequestCallSiteTest.groovy deleted file mode 100644 index ee8a6eb9ca0..00000000000 --- a/dd-java-agent/instrumentation/servlet/src/test/groovy/HttpServletRequestCallSiteTest.groovy +++ /dev/null @@ -1,273 +0,0 @@ -import datadog.smoketest.controller.TestHttpServletRequestCallSiteSuite -import datadog.trace.agent.test.AgentTestRunner -import datadog.trace.api.iast.InstrumentationBridge -import datadog.trace.api.iast.SourceTypes -import datadog.trace.api.iast.propagation.PropagationModule -import datadog.trace.instrumentation.servlet.request.HttpServletRequestCallSite -import groovy.transform.CompileDynamic - -import javax.servlet.http.Cookie -import javax.servlet.http.HttpServletRequest -import javax.servlet.http.HttpServletRequestWrapper - -@CompileDynamic -class HttpServletRequestCallSiteTest extends AgentTestRunner { - - @Override - protected void configurePreAgent() { - injectSysConfig('dd.iast.enabled', 'true') - } - - def cleanup() { - InstrumentationBridge.clearIastModules() - } - - def 'test getHeader'() { - setup: - final iastModule = Mock(PropagationModule) - InstrumentationBridge.registerIastModule(iastModule) - final testSuite = new TestHttpServletRequestCallSiteSuite(Mock(clazz) { - getHeader('header') >> 'value' - }) - - when: - final result = testSuite.getHeader('header') - - then: - result == 'value' - 1 * iastModule.taint('value', SourceTypes.REQUEST_HEADER_VALUE, 'header') - - where: - clazz | _ - HttpServletRequest | _ - HttpServletRequestWrapper | _ - } - - void 'test getHeaders'() { - setup: - final iastModule = Mock(PropagationModule) - InstrumentationBridge.registerIastModule(iastModule) - final headers = ['value1', 'value2'] - final testSuite = new TestHttpServletRequestCallSiteSuite(Mock(clazz) { - getHeaders('headers') >> Collections.enumeration(headers) - }) - - when: - final result = testSuite.getHeaders('headers')?.toList() - - then: - result == headers - headers.each { 1 * iastModule.taint(_, it, SourceTypes.REQUEST_HEADER_VALUE, 'headers') } - - where: - clazz | _ - HttpServletRequest | _ - HttpServletRequestWrapper | _ - } - - void 'test getHeaderNames'() { - setup: - final iastModule = Mock(PropagationModule) - InstrumentationBridge.registerIastModule(iastModule) - final headers = ['header1', 'header2'] - final testSuite = new TestHttpServletRequestCallSiteSuite(Mock(clazz) { - getHeaderNames() >> Collections.enumeration(headers) - }) - - when: - final result = testSuite.getHeaderNames()?.toList() - - then: - result == headers - headers.each { 1 * iastModule.taint(_, it, SourceTypes.REQUEST_HEADER_NAME, it) } - - where: - clazz | _ - HttpServletRequest | _ - HttpServletRequestWrapper | _ - } - - void 'test getCookies'() { - setup: - final iastModule = Mock(PropagationModule) - InstrumentationBridge.registerIastModule(iastModule) - final cookies = [new Cookie('name1', 'value1'), new Cookie('name2', 'value2')] as Cookie[] - final testSuite = new TestHttpServletRequestCallSiteSuite(Mock(clazz) { - getCookies() >> cookies - }) - - when: - final result = testSuite.getCookies() - - then: - result == cookies - cookies.each { 1 * iastModule.taint(_, it, SourceTypes.REQUEST_COOKIE_VALUE) } - - where: - clazz | _ - HttpServletRequest | _ - HttpServletRequestWrapper | _ - } - - def 'test that get headers does not fail when servlet related code fails'(final Class clazz) { - setup: - final iastModule = Mock(PropagationModule) - InstrumentationBridge.registerIastModule(iastModule) - final enumeration = Mock(Enumeration) { - hasMoreElements() >> { throw new NuclearBomb('Boom!!!')} - } - final testSuite = new TestHttpServletRequestCallSiteSuite(Mock(clazz) { - getHeaders('header') >> enumeration - }) - - when: - testSuite.getHeaders('header') - - then: - final bomb = thrown(NuclearBomb) - bomb.stackTrace.find { it.className == HttpServletRequestCallSite.name } == null - - where: - clazz | _ - HttpServletRequest | _ - HttpServletRequestWrapper | _ - } - - def 'test that get header names does not fail when servlet related code fails'(final Class clazz) { - setup: - final iastModule = Mock(PropagationModule) - InstrumentationBridge.registerIastModule(iastModule) - final enumeration = Mock(Enumeration) { - hasMoreElements() >> { throw new NuclearBomb('Boom!!!')} - } - final testSuite = new TestHttpServletRequestCallSiteSuite(Mock(clazz) { - getHeaderNames() >> enumeration - }) - - when: - testSuite.getHeaderNames() - - then: - final bomb = thrown(NuclearBomb) - bomb.stackTrace.find { it.className == HttpServletRequestCallSite.name } == null - - where: - clazz | _ - HttpServletRequest | _ - HttpServletRequestWrapper | _ - } - - void 'test get query string'() { - setup: - final iastModule = Mock(PropagationModule) - InstrumentationBridge.registerIastModule(iastModule) - final testSuite = new TestHttpServletRequestCallSiteSuite(Mock(clazz) { - getQueryString() >> 'paramName=paramValue' - }) - - when: - testSuite.getQueryString() - - then: - - 1 * iastModule.taint('paramName=paramValue', SourceTypes.REQUEST_QUERY) - - where: - clazz | _ - HttpServletRequest | _ - HttpServletRequestWrapper | _ - } - - void 'test getRequestURI'() { - setup: - final iastModule = Mock(PropagationModule) - InstrumentationBridge.registerIastModule(iastModule) - final mock = Mock(clazz){ - getRequestURI() >> 'retValue' - } - final testSuite = new TestHttpServletRequestCallSiteSuite(mock) - - when: - testSuite.getRequestURI() - - then: - 1 * iastModule.taint('retValue', SourceTypes.REQUEST_PATH) - - where: - clazz | _ - HttpServletRequest | _ - HttpServletRequestWrapper | _ - } - - void 'test getRequestURL'() { - setup: - final module = Mock(PropagationModule) - final retValue = new StringBuffer("retValue") - InstrumentationBridge.registerIastModule(module) - final mock = Mock(clazz){ - getRequestURL() >> retValue - } - final testSuite = new TestHttpServletRequestCallSiteSuite(mock) - - when: - testSuite.getRequestURL() - - then: - 1 * module.taint(retValue, SourceTypes.REQUEST_URI) - - where: - clazz | _ - HttpServletRequest | _ - HttpServletRequestWrapper | _ - } - - - void 'test getPathInfo'() { - setup: - final iastModule = Mock(PropagationModule) - InstrumentationBridge.registerIastModule(iastModule) - final mock = Mock(HttpServletRequest){ - getPathInfo() >> 'retValue' - } - final testSuite = new TestHttpServletRequestCallSiteSuite(mock) - - when: - testSuite.getPathInfo() - - then: - 1 * iastModule.taint('retValue', SourceTypes.REQUEST_PATH) - - where: - clazz | _ - HttpServletRequest | _ - HttpServletRequestWrapper | _ - } - - void 'test getPathTranslated'() { - setup: - final iastModule = Mock(PropagationModule) - InstrumentationBridge.registerIastModule(iastModule) - final mock = Mock(HttpServletRequest){ - getPathTranslated() >> 'retValue' - } - final testSuite = new TestHttpServletRequestCallSiteSuite(mock) - - when: - testSuite.getPathTranslated() - - then: - 1 * iastModule.taint('retValue', SourceTypes.REQUEST_PATH) - - where: - clazz | _ - HttpServletRequest | _ - HttpServletRequestWrapper | _ - } - - - private static class NuclearBomb extends RuntimeException { - NuclearBomb(final String message) { - super(message) - } - } -} diff --git a/dd-java-agent/instrumentation/servlet/src/test/groovy/HttpServletRequestTest.groovy b/dd-java-agent/instrumentation/servlet/src/test/groovy/HttpServletRequestTest.groovy new file mode 100644 index 00000000000..c501d89ad47 --- /dev/null +++ b/dd-java-agent/instrumentation/servlet/src/test/groovy/HttpServletRequestTest.groovy @@ -0,0 +1,459 @@ +import datadog.smoketest.controller.JavaxHttpServletRequestTestSuite +import datadog.smoketest.controller.JavaxHttpServletRequestWrapperTestSuite +import datadog.smoketest.controller.ServletRequestTestSuite +import datadog.trace.agent.test.AgentTestRunner +import datadog.trace.api.iast.InstrumentationBridge +import datadog.trace.api.iast.SourceTypes +import datadog.trace.api.iast.propagation.PropagationModule +import datadog.trace.api.iast.sink.UnvalidatedRedirectModule + +import javax.servlet.RequestDispatcher +import javax.servlet.ServletInputStream +import javax.servlet.http.Cookie +import javax.servlet.http.HttpServletRequest +import javax.servlet.http.HttpServletRequestWrapper + +import datadog.trace.agent.tooling.iast.TaintableEnumeration + +class HttpServletRequestTest extends AgentTestRunner { + + @Override + protected void configurePreAgent() { + injectSysConfig('dd.iast.enabled', 'true') + } + + void cleanup() { + InstrumentationBridge.clearIastModules() + } + + void 'test getHeader'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getHeader('header') + + then: + result == 'value' + 1 * mock.getHeader('header') >> 'value' + 1 * iastModule.taint('value', SourceTypes.REQUEST_HEADER_VALUE, 'header') + 0 * _ + + where: + suite << testSuite() + } + + void 'test getHeaders'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final headers = ['value1', 'value2'] + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getHeaders('headers').collect() + + then: + result == headers + 1 * mock.getHeaders('headers') >> Collections.enumeration(headers) + headers.each { 1 * iastModule.taint(_, it, SourceTypes.REQUEST_HEADER_VALUE, 'headers') } + 0 * _ + + where: + suite << testSuite() + } + + void 'test getHeaderNames'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final headers = ['header1', 'header2'] + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getHeaderNames().collect() + + then: + result == headers + 1 * mock.getHeaderNames() >> Collections.enumeration(headers) + headers.each { 1 * iastModule.taint(_, it, SourceTypes.REQUEST_HEADER_NAME, it) } + 0 * _ + + where: + suite << testSuite() + } + + void 'test getParameter'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getParameter('parameter') + + then: + result == 'value' + 1 * mock.getParameter('parameter') >> 'value' + 1 * iastModule.taint('value', SourceTypes.REQUEST_PARAMETER_VALUE, 'parameter') + 0 * _ + + where: + suite << testSuite() + } + + void 'test getParameterValues'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final values = ['value1', 'value2'] + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getParameterValues('parameter').collect() + + then: + result == values + 1 * mock.getParameterValues('parameter') >> { values as String[] } + values.each { 1 * iastModule.taint(_, it, SourceTypes.REQUEST_PARAMETER_VALUE, 'parameter') } + 0 * _ + + where: + suite << testSuite() + } + + void 'test getParameterMap'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final parameters = [parameter: ['header1', 'header2'] as String[]] + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getParameterMap() + + then: + result == parameters + 1 * mock.getParameterMap() >> parameters + parameters.each { key, values -> + 1 * iastModule.taint(_, key, SourceTypes.REQUEST_PARAMETER_NAME, key) + values.each { value -> + 1 * iastModule.taint(_, value, SourceTypes.REQUEST_PARAMETER_VALUE, key) + } + } + 0 * _ + + where: + suite << testSuite() + } + + void 'test getParameterNames'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final parameters = ['param1', 'param2'] + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getParameterNames().collect() + + then: + result == parameters + 1 * mock.getParameterNames() >> Collections.enumeration(parameters) + parameters.each { 1 * iastModule.taint(_, it, SourceTypes.REQUEST_PARAMETER_NAME, it) } + 0 * _ + + where: + suite << testSuite() + } + + void 'test getCookies'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final cookies = [new Cookie('name1', 'value1'), new Cookie('name2', 'value2')] as Cookie[] + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getCookies() + + then: + result == cookies + 1 * mock.getCookies() >> cookies + cookies.each { 1 * iastModule.taint(_, it, SourceTypes.REQUEST_COOKIE_VALUE) } + 0 * _ + + where: + suite << testSuite() + } + + void 'test that get headers does not fail when servlet related code fails'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final enumeration = Mock(Enumeration) { + hasMoreElements() >> { throw new NuclearBomb('Boom!!!') } + } + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final headers = request.getHeaders('header') + + then: + 1 * mock.getHeaders('header') >> enumeration + noExceptionThrown() + + when: + headers.hasMoreElements() + + then: + final bomb = thrown(NuclearBomb) + bomb.stackTrace.find { it.className == TaintableEnumeration.name } == null + + where: + suite << testSuite() + } + + void 'test that get header names does not fail when servlet related code fails'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final enumeration = Mock(Enumeration) { + hasMoreElements() >> { throw new NuclearBomb('Boom!!!') } + } + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getHeaderNames() + + then: + 1 * mock.getHeaderNames() >> enumeration + noExceptionThrown() + + when: + result.hasMoreElements() + + then: + final bomb = thrown(NuclearBomb) + bomb.stackTrace.find { it.className == TaintableEnumeration.name } == null + + where: + suite << testSuite() + } + + void 'test get query string'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final queryString = 'paramName=paramValue' + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final String result = request.getQueryString() + + then: + result == queryString + 1 * mock.getQueryString() >> queryString + 1 * iastModule.taint(queryString, SourceTypes.REQUEST_QUERY) + 0 * _ + + where: + suite << testSuite() + } + + void 'test getInputStream'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final is = Mock(ServletInputStream) + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getInputStream() + + then: + result == is + 1 * mock.getInputStream() >> is + 1 * iastModule.taint(is, SourceTypes.REQUEST_BODY) + 0 * _ + + where: + suite << testSuite() + } + + void 'test getReader'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final reader = Mock(BufferedReader) + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getReader() + + then: + result == reader + 1 * mock.getReader() >> reader + 1 * iastModule.taint(reader, SourceTypes.REQUEST_BODY) + 0 * _ + + where: + suite << testSuite() + } + + void 'test getRequestDispatcher'() { + setup: + final iastModule = Mock(UnvalidatedRedirectModule) + InstrumentationBridge.registerIastModule(iastModule) + final path = 'http://dummy.location.com' + final dispatcher = Mock(RequestDispatcher) + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getRequestDispatcher(path) + + then: + result == dispatcher + 1 * mock.getRequestDispatcher(path) >> dispatcher + 1 * iastModule.onRedirect(path) + 0 * _ + + where: + suite << testSuite() + } + + void 'test getRequestURI'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final uri = 'retValue' + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getRequestURI() + + then: + result == uri + 1 * mock.getRequestURI() >> uri + 1 * iastModule.taint(uri, SourceTypes.REQUEST_PATH) + 0 * _ + + where: + suite << testSuiteCallSites() + } + + void 'test getPathInfo'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final pathInfo = 'retValue' + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getPathInfo() + + then: + result == pathInfo + 1 * mock.getPathInfo() >> pathInfo + 1 * iastModule.taint(pathInfo, SourceTypes.REQUEST_PATH) + 0 * _ + + where: + suite << testSuiteCallSites() + } + + void 'test getPathTranslated'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final pathTranslated = 'retValue' + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getPathTranslated() + + then: + result == pathTranslated + 1 * mock.getPathTranslated() >> pathTranslated + 1 * iastModule.taint(pathTranslated, SourceTypes.REQUEST_PATH) + 0 * _ + + where: + suite << testSuiteCallSites() + } + + void 'test getRequestURL'() { + setup: + final iastModule = Mock(PropagationModule) + InstrumentationBridge.registerIastModule(iastModule) + final url = new StringBuffer('retValue') + final mock = Mock(HttpServletRequest) + final request = suite.call(mock) + + when: + final result = request.getRequestURL() + + then: + result == url + 1 * mock.getRequestURL() >> url + 1 * iastModule.taint(url, SourceTypes.REQUEST_URI) + 0 * _ + + where: + suite << testSuiteCallSites() + } + + private List> testSuite() { + return [ + { HttpServletRequest request -> new CustomRequest(request: request) }, + { HttpServletRequest request -> new CustomRequestWrapper(new CustomRequest(request: request)) }, + { HttpServletRequest request -> + new HttpServletRequestWrapper(new CustomRequest(request: request)) + } + ] + } + + private List> testSuiteCallSites() { + return [ + { HttpServletRequest request -> new JavaxHttpServletRequestTestSuite(request) }, + { HttpServletRequest request -> new JavaxHttpServletRequestWrapperTestSuite(new CustomRequestWrapper(request)) }, + ] + } + + private static class NuclearBomb extends RuntimeException { + NuclearBomb(final String message) { + super(message) + } + } + + private static class CustomRequest implements HttpServletRequest { + @Delegate + private HttpServletRequest request + } + + private static class CustomRequestWrapper extends HttpServletRequestWrapper { + + CustomRequestWrapper(final HttpServletRequest request) { + super(request) + } + } +} diff --git a/dd-java-agent/instrumentation/servlet/src/test/groovy/ServletRequestCallSiteTest.groovy b/dd-java-agent/instrumentation/servlet/src/test/groovy/ServletRequestCallSiteTest.groovy deleted file mode 100644 index 6e0ad88ca66..00000000000 --- a/dd-java-agent/instrumentation/servlet/src/test/groovy/ServletRequestCallSiteTest.groovy +++ /dev/null @@ -1,96 +0,0 @@ -import datadog.smoketest.controller.JavaxHttpServletRequestTestSuite -import datadog.smoketest.controller.JavaxHttpServletRequestWrapperTestSuite -import datadog.smoketest.controller.JavaxServletRequestTestSuite -import datadog.smoketest.controller.JavaxServletRequestWrapperTestSuite -import datadog.trace.agent.test.AgentTestRunner -import datadog.trace.api.iast.InstrumentationBridge -import datadog.trace.api.iast.SourceTypes -import datadog.trace.api.iast.propagation.PropagationModule -import datadog.trace.api.iast.sink.UnvalidatedRedirectModule -import groovy.transform.CompileDynamic - -import javax.servlet.ServletInputStream - -@CompileDynamic -class ServletRequestCallSiteTest extends AgentTestRunner { - - @Override - protected void configurePreAgent() { - injectSysConfig("dd.iast.enabled", "true") - } - - void 'test getInputStream'() { - setup: - final iastModule = Mock(PropagationModule) - InstrumentationBridge.registerIastModule(iastModule) - final is = Mock(ServletInputStream) - final servletRequest = Mock(clazz) { - getInputStream() >> is - } - testSuite.init(servletRequest) - - when: - final result = testSuite.getInputStream() - - then: - result == is - 1 * iastModule.taint(is, SourceTypes.REQUEST_BODY) - - where: - testSuite | clazz - new JavaxServletRequestTestSuite() | javax.servlet.ServletRequest - new JavaxHttpServletRequestTestSuite() | javax.servlet.http.HttpServletRequest - new JavaxServletRequestWrapperTestSuite() | javax.servlet.ServletRequestWrapper - new JavaxHttpServletRequestWrapperTestSuite() | javax.servlet.http.HttpServletRequestWrapper - } - - void 'test getReader'() { - setup: - final iastModule = Mock(PropagationModule) - InstrumentationBridge.registerIastModule(iastModule) - final reader = Mock(BufferedReader) - final servletRequest = Mock(clazz) { - getReader() >> reader - } - - testSuite.init(servletRequest) - - when: - final result = testSuite.getReader() - - then: - result == reader - 1 * iastModule.taint(reader, SourceTypes.REQUEST_BODY) - - where: - testSuite | clazz - new JavaxServletRequestTestSuite() | javax.servlet.ServletRequest - new JavaxHttpServletRequestTestSuite() | javax.servlet.http.HttpServletRequest - new JavaxServletRequestWrapperTestSuite() | javax.servlet.ServletRequestWrapper - new JavaxHttpServletRequestWrapperTestSuite() | javax.servlet.http.HttpServletRequestWrapper - } - - void 'test getRequestDispatcher'() { - setup: - final iastModule = Mock(UnvalidatedRedirectModule) - InstrumentationBridge.registerIastModule(iastModule) - final servletRequest = Mock(clazz) - final path = 'http://dummy.location.com' - - testSuite.init(servletRequest) - - when: - testSuite.getRequestDispatcher(path) - - then: - 1 * servletRequest.getRequestDispatcher(path) - 1 * iastModule.onRedirect(path) - - where: - testSuite | clazz - new JavaxServletRequestTestSuite() | javax.servlet.ServletRequest - new JavaxHttpServletRequestTestSuite() | javax.servlet.http.HttpServletRequest - new JavaxServletRequestWrapperTestSuite() | javax.servlet.ServletRequestWrapper - new JavaxHttpServletRequestWrapperTestSuite() | javax.servlet.http.HttpServletRequestWrapper - } -} diff --git a/dd-java-agent/instrumentation/servlet/src/test/groovy/TestGetParameterInstrumentation.groovy b/dd-java-agent/instrumentation/servlet/src/test/groovy/TestGetParameterInstrumentation.groovy deleted file mode 100644 index 52eaf6b29b4..00000000000 --- a/dd-java-agent/instrumentation/servlet/src/test/groovy/TestGetParameterInstrumentation.groovy +++ /dev/null @@ -1,65 +0,0 @@ -import datadog.smoketest.controller.JavaxHttpServletRequestTestSuite -import datadog.smoketest.controller.JavaxHttpServletRequestWrapperTestSuite -import datadog.smoketest.controller.JavaxServletRequestTestSuite -import datadog.smoketest.controller.JavaxServletRequestWrapperTestSuite -import datadog.trace.agent.test.AgentTestRunner -import datadog.trace.api.config.TracerConfig -import datadog.trace.api.iast.InstrumentationBridge -import datadog.trace.api.iast.SourceTypes -import datadog.trace.api.iast.propagation.PropagationModule -import groovy.transform.CompileDynamic - -@CompileDynamic -class TestGetParameterInstrumentation extends AgentTestRunner { - - @Override - protected void configurePreAgent() { - injectSysConfig(TracerConfig.SCOPE_ITERATION_KEEP_ALIVE, "1") // don't let iteration spans linger - injectSysConfig("dd.iast.enabled", "true") - } - - void cleanup() { - InstrumentationBridge.clearIastModules() - } - - void 'test getParameter'() { - final propMod = Mock(PropagationModule) - InstrumentationBridge.registerIastModule(propMod) - final map = [param1: ['value1', 'value2'] as String[]] - final servletRequest = Mock(clazz) { - getParameter(_ as String) >> { map.get(it[0]).first() } - getParameterValues(_ as String) >> { map.get(it[0]) } - getParameterNames() >> { Collections.enumeration(map.keySet()) } - } - testSuite.init(servletRequest) - - when: - testSuite.getParameter('param1') - - then: - 1 * propMod.taint('value1', SourceTypes.REQUEST_PARAMETER_VALUE, 'param1') - - when: - testSuite.getParameterValues('param1') - - then: - map['param1'].each { value -> - 1 * propMod.taint(_, value, SourceTypes.REQUEST_PARAMETER_VALUE, 'param1') - } - - when: - testSuite.getParameterNames() - - then: - map.keySet().each {param -> - 1 * propMod.taint(_, param, SourceTypes.REQUEST_PARAMETER_NAME, param) - } - - where: - testSuite | clazz - new JavaxServletRequestTestSuite() | javax.servlet.ServletRequest - new JavaxHttpServletRequestTestSuite() | javax.servlet.http.HttpServletRequest - new JavaxServletRequestWrapperTestSuite() | javax.servlet.ServletRequestWrapper - new JavaxHttpServletRequestWrapperTestSuite() | javax.servlet.http.HttpServletRequestWrapper - } -} diff --git a/dd-java-agent/instrumentation/servlet/src/test/java/datadog/smoketest/controller/CookieTestSuite.java b/dd-java-agent/instrumentation/servlet/src/test/java/datadog/smoketest/controller/CookieTestSuite.java deleted file mode 100644 index 2987529a120..00000000000 --- a/dd-java-agent/instrumentation/servlet/src/test/java/datadog/smoketest/controller/CookieTestSuite.java +++ /dev/null @@ -1,24 +0,0 @@ -package datadog.smoketest.controller; - -import javax.servlet.http.Cookie; - -public class CookieTestSuite { - - private Cookie cookie; - - public CookieTestSuite(final String name, final String value) { - cookie = new Cookie(name, value); - } - - public String getName() { - return cookie.getName(); - } - - public String getValue() { - return cookie.getValue(); - } - - public Cookie getCookie() { - return cookie; - } -} diff --git a/dd-java-agent/instrumentation/servlet/src/test/java/datadog/smoketest/controller/JavaxHttpServletRequestTestSuite.java b/dd-java-agent/instrumentation/servlet/src/test/java/datadog/smoketest/controller/JavaxHttpServletRequestTestSuite.java index 2cdc3fe2429..0c55d315158 100644 --- a/dd-java-agent/instrumentation/servlet/src/test/java/datadog/smoketest/controller/JavaxHttpServletRequestTestSuite.java +++ b/dd-java-agent/instrumentation/servlet/src/test/java/datadog/smoketest/controller/JavaxHttpServletRequestTestSuite.java @@ -1,48 +1,31 @@ package datadog.smoketest.controller; -import java.io.BufferedReader; -import java.io.IOException; -import java.util.Enumeration; -import javax.servlet.RequestDispatcher; -import javax.servlet.ServletInputStream; import javax.servlet.http.HttpServletRequest; -public class JavaxHttpServletRequestTestSuite - implements ServletRequestTestSuite { - HttpServletRequest request; +public class JavaxHttpServletRequestTestSuite implements ServletRequestTestSuite { + private final HttpServletRequest request; - @Override - public void init(HttpServletRequest request) { + public JavaxHttpServletRequestTestSuite(final HttpServletRequest request) { this.request = request; } @Override - public String getParameter(String paramName) { - return request.getParameter(paramName); - } - - @Override - public String[] getParameterValues(String paramName) { - return request.getParameterValues(paramName); - } - - @Override - public Enumeration getParameterNames() { - return request.getParameterNames(); + public String getRequestURI() { + return request.getRequestURI(); } @Override - public ServletInputStream getInputStream() throws IOException { - return request.getInputStream(); + public String getPathInfo() { + return request.getPathInfo(); } @Override - public BufferedReader getReader() throws IOException { - return request.getReader(); + public String getPathTranslated() { + return request.getPathTranslated(); } @Override - public RequestDispatcher getRequestDispatcher(String path) { - return request.getRequestDispatcher(path); + public StringBuffer getRequestURL() { + return request.getRequestURL(); } } diff --git a/dd-java-agent/instrumentation/servlet/src/test/java/datadog/smoketest/controller/JavaxHttpServletRequestWrapperTestSuite.java b/dd-java-agent/instrumentation/servlet/src/test/java/datadog/smoketest/controller/JavaxHttpServletRequestWrapperTestSuite.java index 12952c8bb2e..79a81ef2f3b 100644 --- a/dd-java-agent/instrumentation/servlet/src/test/java/datadog/smoketest/controller/JavaxHttpServletRequestWrapperTestSuite.java +++ b/dd-java-agent/instrumentation/servlet/src/test/java/datadog/smoketest/controller/JavaxHttpServletRequestWrapperTestSuite.java @@ -1,48 +1,32 @@ package datadog.smoketest.controller; -import java.io.BufferedReader; -import java.io.IOException; -import java.util.Enumeration; -import javax.servlet.RequestDispatcher; -import javax.servlet.ServletInputStream; import javax.servlet.http.HttpServletRequestWrapper; -public class JavaxHttpServletRequestWrapperTestSuite - implements ServletRequestTestSuite { - HttpServletRequestWrapper request; +public class JavaxHttpServletRequestWrapperTestSuite implements ServletRequestTestSuite { - @Override - public void init(HttpServletRequestWrapper request) { - this.request = request; - } - - @Override - public String getParameter(String paramName) { - return request.getParameter(paramName); - } + private final HttpServletRequestWrapper request; - @Override - public String[] getParameterValues(String paramName) { - return request.getParameterValues(paramName); + public JavaxHttpServletRequestWrapperTestSuite(final HttpServletRequestWrapper request) { + this.request = request; } @Override - public Enumeration getParameterNames() { - return request.getParameterNames(); + public String getRequestURI() { + return request.getRequestURI(); } @Override - public ServletInputStream getInputStream() throws IOException { - return request.getInputStream(); + public String getPathInfo() { + return request.getPathInfo(); } @Override - public BufferedReader getReader() throws IOException { - return request.getReader(); + public String getPathTranslated() { + return request.getPathTranslated(); } @Override - public RequestDispatcher getRequestDispatcher(String path) { - return request.getRequestDispatcher(path); + public StringBuffer getRequestURL() { + return request.getRequestURL(); } } diff --git a/dd-java-agent/instrumentation/servlet/src/test/java/datadog/smoketest/controller/JavaxServletRequestTestSuite.java b/dd-java-agent/instrumentation/servlet/src/test/java/datadog/smoketest/controller/JavaxServletRequestTestSuite.java deleted file mode 100644 index bf9f1a66f68..00000000000 --- a/dd-java-agent/instrumentation/servlet/src/test/java/datadog/smoketest/controller/JavaxServletRequestTestSuite.java +++ /dev/null @@ -1,47 +0,0 @@ -package datadog.smoketest.controller; - -import java.io.BufferedReader; -import java.io.IOException; -import java.util.Enumeration; -import javax.servlet.RequestDispatcher; -import javax.servlet.ServletInputStream; -import javax.servlet.ServletRequest; - -public class JavaxServletRequestTestSuite implements ServletRequestTestSuite { - ServletRequest request; - - @Override - public void init(ServletRequest request) { - this.request = request; - } - - @Override - public String getParameter(String paramName) { - return request.getParameter(paramName); - } - - @Override - public String[] getParameterValues(String paramName) { - return request.getParameterValues(paramName); - } - - @Override - public Enumeration getParameterNames() { - return request.getParameterNames(); - } - - @Override - public ServletInputStream getInputStream() throws IOException { - return request.getInputStream(); - } - - @Override - public BufferedReader getReader() throws IOException { - return request.getReader(); - } - - @Override - public RequestDispatcher getRequestDispatcher(String path) { - return request.getRequestDispatcher(path); - } -} diff --git a/dd-java-agent/instrumentation/servlet/src/test/java/datadog/smoketest/controller/JavaxServletRequestWrapperTestSuite.java b/dd-java-agent/instrumentation/servlet/src/test/java/datadog/smoketest/controller/JavaxServletRequestWrapperTestSuite.java deleted file mode 100644 index 35947b5e255..00000000000 --- a/dd-java-agent/instrumentation/servlet/src/test/java/datadog/smoketest/controller/JavaxServletRequestWrapperTestSuite.java +++ /dev/null @@ -1,48 +0,0 @@ -package datadog.smoketest.controller; - -import java.io.BufferedReader; -import java.io.IOException; -import java.util.Enumeration; -import javax.servlet.RequestDispatcher; -import javax.servlet.ServletInputStream; -import javax.servlet.ServletRequestWrapper; - -public class JavaxServletRequestWrapperTestSuite - implements ServletRequestTestSuite { - ServletRequestWrapper request; - - @Override - public void init(ServletRequestWrapper request) { - this.request = request; - } - - @Override - public String getParameter(String paramName) { - return request.getParameter(paramName); - } - - @Override - public String[] getParameterValues(String paramName) { - return request.getParameterValues(paramName); - } - - @Override - public Enumeration getParameterNames() { - return request.getParameterNames(); - } - - @Override - public ServletInputStream getInputStream() throws IOException { - return request.getInputStream(); - } - - @Override - public BufferedReader getReader() throws IOException { - return request.getReader(); - } - - @Override - public RequestDispatcher getRequestDispatcher(String path) { - return request.getRequestDispatcher(path); - } -} diff --git a/dd-java-agent/instrumentation/servlet/src/test/java/datadog/smoketest/controller/ServletRequestTestSuite.java b/dd-java-agent/instrumentation/servlet/src/test/java/datadog/smoketest/controller/ServletRequestTestSuite.java index 5a021e89541..cfa4c98d997 100644 --- a/dd-java-agent/instrumentation/servlet/src/test/java/datadog/smoketest/controller/ServletRequestTestSuite.java +++ b/dd-java-agent/instrumentation/servlet/src/test/java/datadog/smoketest/controller/ServletRequestTestSuite.java @@ -1,24 +1,12 @@ package datadog.smoketest.controller; -import java.io.BufferedReader; -import java.io.IOException; -import java.util.Enumeration; -import javax.servlet.RequestDispatcher; -import javax.servlet.ServletInputStream; +public interface ServletRequestTestSuite { -public interface ServletRequestTestSuite { + String getRequestURI(); - void init(final E request); + String getPathInfo(); - String getParameter(String paramName); + String getPathTranslated(); - String[] getParameterValues(String paramName); - - Enumeration getParameterNames(); - - RequestDispatcher getRequestDispatcher(String path); - - ServletInputStream getInputStream() throws IOException; - - BufferedReader getReader() throws IOException; + StringBuffer getRequestURL(); } diff --git a/dd-java-agent/instrumentation/servlet/src/test/java/datadog/smoketest/controller/TestHttpServletRequestCallSiteSuite.java b/dd-java-agent/instrumentation/servlet/src/test/java/datadog/smoketest/controller/TestHttpServletRequestCallSiteSuite.java deleted file mode 100644 index e62408401f9..00000000000 --- a/dd-java-agent/instrumentation/servlet/src/test/java/datadog/smoketest/controller/TestHttpServletRequestCallSiteSuite.java +++ /dev/null @@ -1,50 +0,0 @@ -package datadog.smoketest.controller; - -import java.util.Enumeration; -import javax.servlet.http.Cookie; -import javax.servlet.http.HttpServletRequest; - -public class TestHttpServletRequestCallSiteSuite { - - private HttpServletRequest request; - - public TestHttpServletRequestCallSiteSuite(final HttpServletRequest request) { - this.request = request; - } - - public String getHeader(final String headerName) { - return request.getHeader(headerName); - } - - public StringBuffer getRequestURL() { - return request.getRequestURL(); - } - - public Enumeration getHeaders(final String headerName) { - return request.getHeaders(headerName); - } - - public Enumeration getHeaderNames() { - return request.getHeaderNames(); - } - - public Cookie[] getCookies() { - return request.getCookies(); - } - - public String getQueryString() { - return request.getQueryString(); - } - - public String getRequestURI() { - return request.getRequestURI(); - } - - public String getPathInfo() { - return request.getPathInfo(); - } - - public String getPathTranslated() { - return request.getPathTranslated(); - } -} diff --git a/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/core/AbstractHttpServerRequestInstrumentation.java b/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/core/AbstractHttpServerRequestInstrumentation.java index 5f3d7dc6b0e..26b4388f78f 100644 --- a/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/core/AbstractHttpServerRequestInstrumentation.java +++ b/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/core/AbstractHttpServerRequestInstrumentation.java @@ -52,14 +52,14 @@ public void adviceTransformations(final AdviceTransformation transformation) { public static class ParamsAdvice { - @Advice.OnMethodEnter + @Advice.OnMethodEnter(suppress = Throwable.class) public static void onEnter( @Advice.Local("beforeParams") Object beforeParams, @Advice.FieldValue("params") final Object params) { beforeParams = params; } - @Advice.OnMethodExit + @Advice.OnMethodExit(suppress = Throwable.class) @Source(SourceTypes.REQUEST_PARAMETER_VALUE) public static void onExit( @Advice.Local("beforeParams") final Object beforeParams, @@ -68,11 +68,7 @@ public static void onExit( if (beforeParams != multiMap) { final PropagationModule module = InstrumentationBridge.PROPAGATION; if (module != null) { - try { - module.taint(multiMap, SourceTypes.REQUEST_PARAMETER_VALUE); - } catch (final Throwable e) { - module.onUnexpectedException("params threw", e); - } + module.taint(multiMap, SourceTypes.REQUEST_PARAMETER_VALUE); } } } @@ -80,14 +76,14 @@ public static void onExit( public static class AttributesAdvice { - @Advice.OnMethodEnter + @Advice.OnMethodEnter(suppress = Throwable.class) public static void onEnter( @Advice.Local("beforeAttributes") Object beforeAttributes, @Advice.FieldValue("attributes") final Object attributes) { beforeAttributes = attributes; } - @Advice.OnMethodExit + @Advice.OnMethodExit(suppress = Throwable.class) @Source(SourceTypes.REQUEST_PARAMETER_VALUE) public static void onExit( @Advice.Local("beforeAttributes") final Object beforeAttributes, @@ -96,11 +92,7 @@ public static void onExit( if (beforeAttributes != multiMap) { final PropagationModule module = InstrumentationBridge.PROPAGATION; if (module != null) { - try { - module.taint(multiMap, SourceTypes.REQUEST_PARAMETER_VALUE); - } catch (final Throwable e) { - module.onUnexpectedException("formAttributes threw", e); - } + module.taint(multiMap, SourceTypes.REQUEST_PARAMETER_VALUE); } } } @@ -108,16 +100,12 @@ public static void onExit( public static class DataAdvice { - @Advice.OnMethodEnter + @Advice.OnMethodEnter(suppress = Throwable.class) @Source(SourceTypes.REQUEST_BODY) public static void onExit(@Advice.Argument(0) final Object data) { final PropagationModule module = InstrumentationBridge.PROPAGATION; if (module != null) { - try { - module.taint(data, SourceTypes.REQUEST_BODY); - } catch (final Throwable e) { - module.onUnexpectedException("handleData threw", e); - } + module.taint(data, SourceTypes.REQUEST_BODY); } } } diff --git a/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/core/CaseInsensitiveHeadersInstrumentation.java b/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/core/CaseInsensitiveHeadersInstrumentation.java index 9a649726239..c9e368fe651 100644 --- a/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/core/CaseInsensitiveHeadersInstrumentation.java +++ b/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/core/CaseInsensitiveHeadersInstrumentation.java @@ -75,7 +75,7 @@ public AdviceTransformer transformer() { } public static class GetAdvice { - @Advice.OnMethodExit + @Advice.OnMethodExit(suppress = Throwable.class) @Source(SourceTypes.REQUEST_PARAMETER_VALUE) public static void afterGet( @Advice.This final Object self, @@ -83,17 +83,13 @@ public static void afterGet( @Advice.Return final String result) { final PropagationModule propagation = InstrumentationBridge.PROPAGATION; if (propagation != null) { - try { - propagation.taintIfTainted(result, self, SourceTypes.REQUEST_PARAMETER_VALUE, name); - } catch (final Throwable e) { - propagation.onUnexpectedException("get threw", e); - } + propagation.taintIfTainted(result, self, SourceTypes.REQUEST_PARAMETER_VALUE, name); } } } public static class GetAllAdvice { - @Advice.OnMethodExit + @Advice.OnMethodExit(suppress = Throwable.class) @Source(SourceTypes.REQUEST_PARAMETER_VALUE) public static void afterGetAll( @Advice.This final Object self, @@ -101,64 +97,52 @@ public static void afterGetAll( @Advice.Return final Collection result) { final PropagationModule propagation = InstrumentationBridge.PROPAGATION; if (propagation != null && result != null && !result.isEmpty()) { - try { - if (propagation.isTainted(self)) { - final IastContext ctx = IastContext.Provider.get(); - for (final String value : result) { - propagation.taint(ctx, value, SourceTypes.REQUEST_PARAMETER_VALUE, name); - } + if (propagation.isTainted(self)) { + final IastContext ctx = IastContext.Provider.get(); + for (final String value : result) { + propagation.taint(ctx, value, SourceTypes.REQUEST_PARAMETER_VALUE, name); } - } catch (final Throwable e) { - propagation.onUnexpectedException("getAll threw", e); } } } } public static class EntriesAdvice { - @Advice.OnMethodExit + @Advice.OnMethodExit(suppress = Throwable.class) @Source(SourceTypes.REQUEST_PARAMETER_VALUE) public static void afterEntries( @Advice.This final Object self, @Advice.Return final List> result) { final PropagationModule propagation = InstrumentationBridge.PROPAGATION; if (propagation != null && result != null && !result.isEmpty()) { - try { - if (propagation.isTainted(self)) { - final IastContext ctx = IastContext.Provider.get(); - final Set names = new HashSet<>(); - for (final Map.Entry entry : result) { - final String name = entry.getKey(); - final String value = entry.getValue(); - if (names.add(name)) { - propagation.taint(ctx, name, SourceTypes.REQUEST_PARAMETER_NAME, name); - } - propagation.taint(ctx, value, SourceTypes.REQUEST_PARAMETER_VALUE, name); + if (propagation.isTainted(self)) { + final IastContext ctx = IastContext.Provider.get(); + final Set names = new HashSet<>(); + for (final Map.Entry entry : result) { + final String name = entry.getKey(); + final String value = entry.getValue(); + if (names.add(name)) { + propagation.taint(ctx, name, SourceTypes.REQUEST_PARAMETER_NAME, name); } + propagation.taint(ctx, value, SourceTypes.REQUEST_PARAMETER_VALUE, name); } - } catch (final Throwable e) { - propagation.onUnexpectedException("entries threw", e); } } } } public static class NamesAdvice { - @Advice.OnMethodExit + @Advice.OnMethodExit(suppress = Throwable.class) @Source(SourceTypes.REQUEST_PARAMETER_NAME) public static void afterNames( @Advice.This final Object self, @Advice.Return final Set result) { final PropagationModule propagation = InstrumentationBridge.PROPAGATION; if (propagation != null && result != null && !result.isEmpty()) { - try { - if (propagation.isTainted(self)) { - final IastContext ctx = IastContext.Provider.get(); - for (final String name : result) { - propagation.taint(ctx, name, SourceTypes.REQUEST_PARAMETER_NAME, name); - } + if (propagation.isTainted(self)) { + final IastContext ctx = IastContext.Provider.get(); + for (final String name : result) { + propagation.taint(ctx, name, SourceTypes.REQUEST_PARAMETER_NAME, name); } - } catch (final Throwable e) { - propagation.onUnexpectedException("names threw", e); } } } diff --git a/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/core/HeadersAdaptorInstrumentation.java b/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/core/HeadersAdaptorInstrumentation.java index 17250607fa4..4b865b9ed2e 100644 --- a/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/core/HeadersAdaptorInstrumentation.java +++ b/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/core/HeadersAdaptorInstrumentation.java @@ -67,7 +67,7 @@ public AdviceTransformer transformer() { } public static class GetAdvice { - @Advice.OnMethodExit + @Advice.OnMethodExit(suppress = Throwable.class) @Source(SourceTypes.REQUEST_HEADER_VALUE) public static void afterGet( @Advice.This final Object self, @@ -75,17 +75,13 @@ public static void afterGet( @Advice.Return final String result) { final PropagationModule propagation = InstrumentationBridge.PROPAGATION; if (propagation != null) { - try { - propagation.taintIfTainted(result, self, SourceTypes.REQUEST_HEADER_VALUE, name); - } catch (final Throwable e) { - propagation.onUnexpectedException("get threw", e); - } + propagation.taintIfTainted(result, self, SourceTypes.REQUEST_HEADER_VALUE, name); } } } public static class GetAllAdvice { - @Advice.OnMethodExit + @Advice.OnMethodExit(suppress = Throwable.class) @Source(SourceTypes.REQUEST_HEADER_VALUE) public static void afterGetAll( @Advice.This final Object self, @@ -93,65 +89,53 @@ public static void afterGetAll( @Advice.Return final Collection result) { final PropagationModule propagation = InstrumentationBridge.PROPAGATION; if (propagation != null && result != null && !result.isEmpty()) { - try { - if (propagation.isTainted(self)) { - final IastContext ctx = IastContext.Provider.get(); - final String headerName = name == null ? null : name.toString(); - for (final String value : result) { - propagation.taint(ctx, value, SourceTypes.REQUEST_HEADER_VALUE, headerName); - } + if (propagation.isTainted(self)) { + final IastContext ctx = IastContext.Provider.get(); + final String headerName = name == null ? null : name.toString(); + for (final String value : result) { + propagation.taint(ctx, value, SourceTypes.REQUEST_HEADER_VALUE, headerName); } - } catch (final Throwable e) { - propagation.onUnexpectedException("getAll threw", e); } } } } public static class EntriesAdvice { - @Advice.OnMethodExit + @Advice.OnMethodExit(suppress = Throwable.class) @Source(SourceTypes.REQUEST_HEADER_VALUE) public static void afterEntries( @Advice.This final Object self, @Advice.Return final List> result) { final PropagationModule propagation = InstrumentationBridge.PROPAGATION; if (propagation != null && result != null && !result.isEmpty()) { - try { - if (propagation.isTainted(self)) { - final IastContext ctx = IastContext.Provider.get(); - final Set names = new HashSet<>(); - for (Map.Entry entry : result) { - final String name = entry.getKey(); - final String value = entry.getValue(); - if (names.add(name)) { - propagation.taint(ctx, name, SourceTypes.REQUEST_HEADER_NAME, name); - } - propagation.taint(ctx, value, SourceTypes.REQUEST_HEADER_VALUE, name); + if (propagation.isTainted(self)) { + final IastContext ctx = IastContext.Provider.get(); + final Set names = new HashSet<>(); + for (Map.Entry entry : result) { + final String name = entry.getKey(); + final String value = entry.getValue(); + if (names.add(name)) { + propagation.taint(ctx, name, SourceTypes.REQUEST_HEADER_NAME, name); } + propagation.taint(ctx, value, SourceTypes.REQUEST_HEADER_VALUE, name); } - } catch (final Throwable e) { - propagation.onUnexpectedException("entries threw", e); } } } } public static class NamesAdvice { - @Advice.OnMethodExit + @Advice.OnMethodExit(suppress = Throwable.class) @Source(SourceTypes.REQUEST_HEADER_NAME) public static void afterNames( @Advice.This final Object self, @Advice.Return final Set result) { final PropagationModule propagation = InstrumentationBridge.PROPAGATION; if (propagation != null && result != null && !result.isEmpty()) { - try { - if (propagation.isTainted(self)) { - final IastContext ctx = IastContext.Provider.get(); - for (final String name : result) { - propagation.taint(ctx, name, SourceTypes.REQUEST_HEADER_NAME, name); - } + if (propagation.isTainted(self)) { + final IastContext ctx = IastContext.Provider.get(); + for (final String name : result) { + propagation.taint(ctx, name, SourceTypes.REQUEST_HEADER_NAME, name); } - } catch (final Throwable e) { - propagation.onUnexpectedException("names threw", e); } } } diff --git a/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/core/Http2ServerRequestInstrumentation.java b/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/core/Http2ServerRequestInstrumentation.java index 2f5d1d2dc71..9e648c95cb0 100644 --- a/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/core/Http2ServerRequestInstrumentation.java +++ b/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/core/Http2ServerRequestInstrumentation.java @@ -45,7 +45,7 @@ public static void onEnter( beforeHeaders = headersMap; } - @Advice.OnMethodExit + @Advice.OnMethodExit(suppress = Throwable.class) @Source(SourceTypes.REQUEST_HEADER_VALUE) public static void onExit( @Advice.Local("beforeHeaders") final Object beforeHeaders, @@ -54,11 +54,7 @@ public static void onExit( if (beforeHeaders != multiMap) { final PropagationModule module = InstrumentationBridge.PROPAGATION; if (module != null) { - try { - module.taint(multiMap, SourceTypes.REQUEST_HEADER_VALUE); - } catch (final Throwable e) { - module.onUnexpectedException("headers threw", e); - } + module.taint(multiMap, SourceTypes.REQUEST_HEADER_VALUE); } } } diff --git a/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/core/HttpServerRequestInstrumentation.java b/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/core/HttpServerRequestInstrumentation.java index be2c1c983bc..a2186330199 100644 --- a/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/core/HttpServerRequestInstrumentation.java +++ b/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/core/HttpServerRequestInstrumentation.java @@ -39,14 +39,14 @@ public void adviceTransformations(AdviceTransformation transformation) { public static class HeadersAdvice { - @Advice.OnMethodEnter + @Advice.OnMethodEnter(suppress = Throwable.class) public static void onEnter( @Advice.Local("beforeHeaders") Object beforeHeaders, @Advice.FieldValue("headers") final Object headers) { beforeHeaders = headers; } - @Advice.OnMethodExit() + @Advice.OnMethodExit(suppress = Throwable.class) @Source(SourceTypes.REQUEST_HEADER_VALUE) public static void onExit( @Advice.Local("beforeHeaders") final Object beforeHeaders, @@ -55,11 +55,7 @@ public static void onExit( if (beforeHeaders != multiMap) { final PropagationModule module = InstrumentationBridge.PROPAGATION; if (module != null) { - try { - module.taint(multiMap, SourceTypes.REQUEST_HEADER_VALUE); - } catch (final Throwable e) { - module.onUnexpectedException("headers threw", e); - } + module.taint(multiMap, SourceTypes.REQUEST_HEADER_VALUE); } } } diff --git a/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/CookieImplInstrumentation.java b/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/CookieImplInstrumentation.java index 671e2d76349..b06d2b56ffd 100644 --- a/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/CookieImplInstrumentation.java +++ b/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/CookieImplInstrumentation.java @@ -51,35 +51,27 @@ public AdviceTransformer transformer() { } public static class GetNameAdvice { - @Advice.OnMethodExit + @Advice.OnMethodExit(suppress = Throwable.class) @Source(SourceTypes.REQUEST_COOKIE_NAME) public static void afterGetName( @Advice.This final Cookie self, @Advice.Return final String result) { final PropagationModule module = InstrumentationBridge.PROPAGATION; if (module != null) { - try { - module.taintIfTainted(result, self, SourceTypes.REQUEST_COOKIE_NAME, result); - } catch (final Throwable e) { - module.onUnexpectedException("getName threw", e); - } + module.taintIfTainted(result, self, SourceTypes.REQUEST_COOKIE_NAME, result); } } } public static class GetValueAdvice { - @Advice.OnMethodExit + @Advice.OnMethodExit(suppress = Throwable.class) @Source(SourceTypes.REQUEST_COOKIE_VALUE) public static void afterGetValue( @Advice.This final Cookie self, @Advice.Return final String result) { final PropagationModule module = InstrumentationBridge.PROPAGATION; if (module != null) { - try { - // TODO calling self.getName() actually taints the name of the cookie - module.taintIfTainted(result, self, SourceTypes.REQUEST_COOKIE_VALUE, self.getName()); - } catch (final Throwable e) { - module.onUnexpectedException("getValue threw", e); - } + // TODO calling self.getName() actually taints the name of the cookie + module.taintIfTainted(result, self, SourceTypes.REQUEST_COOKIE_VALUE, self.getName()); } } } diff --git a/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/IastRoutingContextImplInstrumentation.java b/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/IastRoutingContextImplInstrumentation.java index 87af498ce3f..cd685575017 100644 --- a/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/IastRoutingContextImplInstrumentation.java +++ b/dd-java-agent/instrumentation/vertx-web-3.4/src/main/java/datadog/trace/instrumentation/vertx_3_4/server/IastRoutingContextImplInstrumentation.java @@ -53,18 +53,14 @@ public void adviceTransformations(final AdviceTransformation transformation) { } public static class CookiesAdvice { - @Advice.OnMethodExit + @Advice.OnMethodExit(suppress = Throwable.class) @Source(SourceTypes.REQUEST_COOKIE_VALUE) public static void onCookies(@Advice.Return final Set cookies) { final PropagationModule module = InstrumentationBridge.PROPAGATION; if (module != null && cookies != null && !cookies.isEmpty()) { - try { - final IastContext ctx = IastContext.Provider.get(); - for (final Object cookie : cookies) { - module.taint(ctx, cookie, SourceTypes.REQUEST_COOKIE_VALUE); - } - } catch (final Throwable e) { - module.onUnexpectedException("cookies threw", e); + final IastContext ctx = IastContext.Provider.get(); + for (final Object cookie : cookies) { + module.taint(ctx, cookie, SourceTypes.REQUEST_COOKIE_VALUE); } } } diff --git a/dd-java-agent/instrumentation/vertx-web-3.5/src/main/java/datadog/trace/instrumentation/vertx_3_5/core/VertxHttpHeadersInstrumentation.java b/dd-java-agent/instrumentation/vertx-web-3.5/src/main/java/datadog/trace/instrumentation/vertx_3_5/core/VertxHttpHeadersInstrumentation.java index d13ac8c4d4f..7337d7a9422 100644 --- a/dd-java-agent/instrumentation/vertx-web-3.5/src/main/java/datadog/trace/instrumentation/vertx_3_5/core/VertxHttpHeadersInstrumentation.java +++ b/dd-java-agent/instrumentation/vertx-web-3.5/src/main/java/datadog/trace/instrumentation/vertx_3_5/core/VertxHttpHeadersInstrumentation.java @@ -73,7 +73,7 @@ public AdviceTransformer transformer() { } public static class GetAdvice { - @Advice.OnMethodExit + @Advice.OnMethodExit(suppress = Throwable.class) @Source(SourceTypes.REQUEST_HEADER_VALUE) public static void afterGet( @Advice.This final Object self, @@ -81,17 +81,13 @@ public static void afterGet( @Advice.Return final String result) { final PropagationModule propagation = InstrumentationBridge.PROPAGATION; if (propagation != null) { - try { - propagation.taintIfTainted(result, self, SourceTypes.REQUEST_HEADER_VALUE, name); - } catch (final Throwable e) { - propagation.onUnexpectedException("get threw", e); - } + propagation.taintIfTainted(result, self, SourceTypes.REQUEST_HEADER_VALUE, name); } } } public static class GetAllAdvice { - @Advice.OnMethodExit + @Advice.OnMethodExit(suppress = Throwable.class) @Source(SourceTypes.REQUEST_HEADER_VALUE) public static void afterGetAll( @Advice.This final Object self, @@ -99,64 +95,52 @@ public static void afterGetAll( @Advice.Return final Collection result) { final PropagationModule propagation = InstrumentationBridge.PROPAGATION; if (propagation != null && result != null && !result.isEmpty()) { - try { - if (propagation.isTainted(self)) { - final IastContext ctx = IastContext.Provider.get(); - for (final String value : result) { - propagation.taint(ctx, value, SourceTypes.REQUEST_HEADER_VALUE, name); - } + if (propagation.isTainted(self)) { + final IastContext ctx = IastContext.Provider.get(); + for (final String value : result) { + propagation.taint(ctx, value, SourceTypes.REQUEST_HEADER_VALUE, name); } - } catch (final Throwable e) { - propagation.onUnexpectedException("getAll threw", e); } } } } public static class EntriesAdvice { - @Advice.OnMethodExit + @Advice.OnMethodExit(suppress = Throwable.class) @Source(SourceTypes.REQUEST_HEADER_VALUE) public static void afterEntries( @Advice.This final Object self, @Advice.Return final List> result) { final PropagationModule propagation = InstrumentationBridge.PROPAGATION; if (propagation != null && result != null && !result.isEmpty()) { - try { - if (propagation.isTainted(self)) { - final IastContext ctx = IastContext.Provider.get(); - final Set names = new HashSet<>(); - for (final Map.Entry entry : result) { - final String name = entry.getKey(); - final String value = entry.getValue(); - if (names.add(name)) { - propagation.taint(ctx, name, SourceTypes.REQUEST_HEADER_NAME, name); - } - propagation.taint(ctx, value, SourceTypes.REQUEST_HEADER_VALUE, name); + if (propagation.isTainted(self)) { + final IastContext ctx = IastContext.Provider.get(); + final Set names = new HashSet<>(); + for (final Map.Entry entry : result) { + final String name = entry.getKey(); + final String value = entry.getValue(); + if (names.add(name)) { + propagation.taint(ctx, name, SourceTypes.REQUEST_HEADER_NAME, name); } + propagation.taint(ctx, value, SourceTypes.REQUEST_HEADER_VALUE, name); } - } catch (final Throwable e) { - propagation.onUnexpectedException("entries threw", e); } } } } public static class NamesAdvice { - @Advice.OnMethodExit + @Advice.OnMethodExit(suppress = Throwable.class) @Source(SourceTypes.REQUEST_HEADER_NAME) public static void afterNames( @Advice.This final Object self, @Advice.Return final Set result) { final PropagationModule propagation = InstrumentationBridge.PROPAGATION; if (propagation != null && result != null && !result.isEmpty()) { - try { - if (propagation.isTainted(self)) { - final IastContext ctx = IastContext.Provider.get(); - for (final String name : result) { - propagation.taint(ctx, name, SourceTypes.REQUEST_HEADER_NAME, name); - } + if (propagation.isTainted(self)) { + final IastContext ctx = IastContext.Provider.get(); + for (final String name : result) { + propagation.taint(ctx, name, SourceTypes.REQUEST_HEADER_NAME, name); } - } catch (final Throwable e) { - propagation.onUnexpectedException("names threw", e); } } }