Stacktrace leak protection for Tomcat 7

dd-java-agent/agent-iast/src/main/java/com/datadog/iast/IastSystem.java

@@ -14,6 +14,7 @@
 import com.datadog.iast.sink.PathTraversalModuleImpl;
 import com.datadog.iast.sink.SqlInjectionModuleImpl;
 import com.datadog.iast.sink.SsrfModuleImpl;
+import com.datadog.iast.sink.StacktraceLeakModuleImpl;
 import com.datadog.iast.sink.TrustBoundaryViolationModuleImpl;
 import com.datadog.iast.sink.UnvalidatedRedirectModuleImpl;
 import com.datadog.iast.sink.WeakCipherModuleImpl;
@@ -100,7 +101,8 @@ private static Stream<IastModule> iastModules(final Dependencies dependencies) {
         new WeakRandomnessModuleImpl(dependencies),
         new XPathInjectionModuleImpl(dependencies),
         new TrustBoundaryViolationModuleImpl(dependencies),
-        new XssModuleImpl(dependencies));
+        new XssModuleImpl(dependencies),
+        new StacktraceLeakModuleImpl(dependencies));
   }
 
   private static void registerRequestStartedCallback(

dd-java-agent/agent-iast/src/main/java/com/datadog/iast/model/VulnerabilityType.java

@@ -65,6 +65,9 @@ public interface VulnerabilityType {
   InjectionType XSS =
       new InjectionTypeImpl(VulnerabilityTypes.XSS_STRING, VulnerabilityMarks.XSS_MARK, ' ');
 
+  VulnerabilityType STACKTRACE_LEAK =
+      new VulnerabilityTypeImpl(VulnerabilityTypes.STACKTRACE_LEAK_STRING, NOT_MARKED);
+
   String name();
 
   /** A bit flag to ignore tainted ranges for this vulnerability. Set to 0 if none. */

dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/StacktraceLeakModuleImpl.java

@@ -0,0 +1,37 @@
+package com.datadog.iast.sink;
+
+import com.datadog.iast.Dependencies;
+import com.datadog.iast.model.Evidence;
+import com.datadog.iast.model.Location;
+import com.datadog.iast.model.Vulnerability;
+import com.datadog.iast.model.VulnerabilityType;
+import datadog.trace.api.iast.sink.StacktraceLeakModule;
+import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
+import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
+import org.jetbrains.annotations.NotNull;
+
+public class StacktraceLeakModuleImpl extends SinkModuleBase implements StacktraceLeakModule {
+
+  public StacktraceLeakModuleImpl(@NotNull Dependencies dependencies) {
+    super(dependencies);
+  }
+
+  @Override
+  public void onStacktraceLeak(
+      Throwable throwable, String moduleName, String className, String methodName) {
+    if (throwable != null) {
+      final AgentSpan span = AgentTracer.activeSpan();
+
+      Evidence evidence =
+          new Evidence(
+              "ExceptionHandler in "
+                  + moduleName
+                  + " \r\nthrown "
+                  + throwable.getClass().getName());
+      Location location = Location.forSpanAndClassAndMethod(span, className, methodName);
+
+      reporter.report(
+          span, new Vulnerability(VulnerabilityType.STACKTRACE_LEAK, location, evidence));
+    }
+  }
+}

dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/StacktraceLeakModuleTest.groovy

@@ -0,0 +1,51 @@
+package com.datadog.iast.sink
+
+import com.datadog.iast.IastModuleImplTestBase
+import com.datadog.iast.model.Evidence
+import com.datadog.iast.model.Vulnerability
+import com.datadog.iast.model.VulnerabilityType
+import datadog.trace.api.iast.sink.StacktraceLeakModule
+import datadog.trace.bootstrap.instrumentation.api.AgentSpan
+
+class StacktraceLeakModuleTest extends IastModuleImplTestBase {
+  private StacktraceLeakModule module
+
+  def setup() {
+    module = new StacktraceLeakModuleImpl(dependencies)
+  }
+
+  void 'iast stacktrace leak module'() {
+    given:
+    final spanId = 123456
+    final span = Mock(AgentSpan)
+
+    def throwable = new Exception('some exception')
+    def moduleName = 'moduleName'
+    def className = 'className'
+    def methodName = 'methodName'
+
+    when:
+    module.onStacktraceLeak(throwable, moduleName, className, methodName)
+
+    then:
+    1 * tracer.activeSpan() >> span
+    1 * span.getSpanId() >> spanId
+    1 * span.getServiceName()
+    1 * reporter.report(_, _) >> { args ->
+      Vulnerability vuln = args[1] as Vulnerability
+      assert vuln != null
+      assert vuln.getType() == VulnerabilityType.STACKTRACE_LEAK
+      assert vuln.getEvidence() == new Evidence('ExceptionHandler in moduleName \r\nthrown java.lang.Exception')
+      assert vuln.getLocation() != null
+    }
+    0 * _
+  }
+
+  void 'iast stacktrace leak no exception'() {
+    when:
+    module.onStacktraceLeak(null, null, null, null)
+
+    then:
+    0 * _
+  }
+}

dd-java-agent/instrumentation/tomcat-appsec-7/src/main/java/datadog/trace/instrumentation/tomcat7/ErrorReportValueAdvice.java

@@ -0,0 +1,78 @@
+package datadog.trace.instrumentation.tomcat7;
+
+import static datadog.trace.bootstrap.blocking.BlockingActionHelper.TemplateType.HTML;
+import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeSpan;
+
+import datadog.trace.api.Config;
+import datadog.trace.api.iast.InstrumentationBridge;
+import datadog.trace.api.iast.sink.StacktraceLeakModule;
+import datadog.trace.bootstrap.blocking.BlockingActionHelper;
+import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
+import java.io.IOException;
+import java.io.Writer;
+import java.nio.charset.StandardCharsets;
+import net.bytebuddy.asm.Advice;
+import org.apache.catalina.connector.Response;
+
+public class ErrorReportValueAdvice {
+
+  @Advice.OnMethodEnter(skipOn = Advice.OnNonDefaultValue.class)
+  public static boolean onEnter(
+      @Advice.Argument(value = 1) Response response,
+      @Advice.Argument(value = 2) Throwable throwable,
+      @Advice.Origin("#t") String className,
+      @Advice.Origin("#m") String methodName) {
+    int statusCode = response.getStatus();
+
+    // Do nothing on a 1xx, 2xx, 3xx and 404 status
+    // Do nothing if the response hasn't been explicitly marked as in error
+    //    and that error has not been reported.
+    if (statusCode < 400 || statusCode == 404 || !response.isError()) {
+      return true; // skip original method
+    }
+
+    final AgentSpan span = activeSpan();
+    if (span != null && throwable != null) {
+      // Report IAST
+      final StacktraceLeakModule module = InstrumentationBridge.STACKTRACE_LEAK_MODULE;
+      if (module != null) {
+        try {
+          module.onStacktraceLeak(throwable, "Tomcat 7+", className, methodName);
+        } catch (final Throwable e) {
+          module.onUnexpectedException("onResponseException threw", e);
+        }
+      }
+    }
+
+    // If we don't need to suppress stacktrace leak
+    if (!Config.get().isIastStacktraceLeakSuppress()) {
+      return false;
+    }
+
+    byte[] template = BlockingActionHelper.getTemplate(HTML);
+    if (template == null) {
+      return false;
+    }
+
+    try {
+      try {
+        String contentType = BlockingActionHelper.getContentType(HTML);
+        response.setContentType(contentType);
+      } catch (Throwable t) {
+        // Ignore
+      }
+      Writer writer = response.getReporter();
+      if (writer != null) {
+        // If writer is null, it's an indication that the response has
+        // been hard committed already, which should never happen
+        String html = new String(template, StandardCharsets.UTF_8);
+        writer.write(html);
+        response.finishResponse();
+      }
+    } catch (IOException | IllegalStateException e) {
+      // Ignore
+    }
+
+    return false;
+  }
+}

dd-java-agent/instrumentation/tomcat-appsec-7/src/main/java/datadog/trace/instrumentation/tomcat7/ErrorReportValueInstrumentation.java

@@ -0,0 +1,40 @@
+package datadog.trace.instrumentation.tomcat7;
+
+import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
+import static net.bytebuddy.matcher.ElementMatchers.isMethod;
+import static net.bytebuddy.matcher.ElementMatchers.isProtected;
+import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
+
+import com.google.auto.service.AutoService;
+import datadog.trace.agent.tooling.Instrumenter;
+
+@AutoService(Instrumenter.class)
+public class ErrorReportValueInstrumentation extends Instrumenter.Iast
+    implements Instrumenter.ForSingleType {
+
+  public ErrorReportValueInstrumentation() {
+    super("tomcat");
+  }
+
+  @Override
+  public String muzzleDirective() {
+    return "from7";
+  }
+
+  @Override
+  public String instrumentedType() {
+    return "org.apache.catalina.valves.ErrorReportValve";
+  }
+
+  @Override
+  public void adviceTransformations(AdviceTransformation transformation) {
+    transformation.applyAdvice(
+        isMethod()
+            .and(named("report"))
+            .and(takesArgument(0, named("org.apache.catalina.connector.Request")))
+            .and(takesArgument(1, named("org.apache.catalina.connector.Response")))
+            .and(takesArgument(2, Throwable.class))
+            .and(isProtected()),
+        packageName + ".ErrorReportValueAdvice");
+  }
+}

dd-smoke-tests/appsec/spring-tomcat7/build.gradle

@@ -0,0 +1,33 @@
+plugins {
+  id "com.github.johnrengelman.shadow"
+}
+
+apply from: "$rootDir/gradle/java.gradle"
+description = 'Spring Tomcat7 Smoke Tests.'
+
+jar {
+  manifest {
+    attributes('Main-Class': 'datadog.smoketest.appsec.springtomcat7.Main')
+  }
+}
+
+dependencies {
+  implementation group: 'org.apache.tomcat.embed', name: 'tomcat-embed-jasper', version: '7.0.47'
+  implementation group: 'org.apache.tomcat.embed', name: 'tomcat-embed-core', version: '7.0.47'
+  implementation group: 'org.apache.tomcat', name: 'tomcat-juli', version: '7.0.47'
+  implementation group: 'org.springframework', name: 'spring-webmvc', version: '4.0.0.RELEASE'
+
+  testImplementation project(':dd-smoke-tests:appsec')
+}
+
+tasks.withType(Test).configureEach {
+  dependsOn "shadowJar"
+
+  jvmArgs "-Ddatadog.smoketest.appsec.springtomcat7.shadowJar.path=${tasks.shadowJar.archiveFile.get()}"
+}
+
+task testRuntimeActivation(type: Test) {
+  jvmArgs '-Dsmoke_test.appsec.enabled=inactive',
+    "-Ddatadog.smoketest.appsec.springtomcat7.shadowJar.path=${tasks.shadowJar.archiveFile.get()}"
+}
+tasks['check'].dependsOn(testRuntimeActivation)

dd-smoke-tests/appsec/spring-tomcat7/src/main/java/datadog/smoketest/appsec/springtomcat7/AppConfigurer.java

@@ -0,0 +1,11 @@
+package datadog.smoketest.appsec.springtomcat7;
+
+import org.springframework.context.annotation.ComponentScan;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.web.servlet.config.annotation.EnableWebMvc;
+import org.springframework.web.servlet.config.annotation.WebMvcConfigurerAdapter;
+
+@Configuration
+@EnableWebMvc
+@ComponentScan(basePackages = {"datadog.smoketest.appsec.springtomcat7"})
+public class AppConfigurer extends WebMvcConfigurerAdapter {}

dd-smoke-tests/appsec/spring-tomcat7/src/main/java/datadog/smoketest/appsec/springtomcat7/Controller.java

@@ -0,0 +1,18 @@
+package datadog.smoketest.appsec.springtomcat7;
+
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RestController;
+
+@RestController
+public class Controller {
+
+  @RequestMapping("/")
+  public String htmlString() {
+    return "Hello world!";
+  }
+
+  @RequestMapping("/exception")
+  public void exceptionMethod() throws Throwable {
+    throw new Throwable("hello");
+  }
+}

dd-smoke-tests/appsec/spring-tomcat7/src/main/java/datadog/smoketest/appsec/springtomcat7/Main.java

@@ -0,0 +1,56 @@
+package datadog.smoketest.appsec.springtomcat7;
+
+import de.thetaphi.forbiddenapis.SuppressForbidden;
+import java.io.File;
+import org.apache.catalina.Context;
+import org.apache.catalina.startup.Tomcat;
+import org.springframework.web.context.support.AnnotationConfigWebApplicationContext;
+import org.springframework.web.servlet.DispatcherServlet;
+
+public class Main {
+
+  private static final String ROOT = "/";
+  private static final String SERVLET = "dispatcherServlet";
+
+  @SuppressForbidden
+  public static void main(String[] args) throws Exception {
+    int port = 8080;
+    for (String arg : args) {
+      if (arg.contains("=")) {
+        String[] kv = arg.split("=");
+        if (kv.length == 2) {
+          if ("--server.port".equalsIgnoreCase(kv[0])) {
+            try {
+              port = Integer.parseInt(kv[1]);
+            } catch (NumberFormatException e) {
+              System.out.println(
+                  "--server.port '"
+                      + kv[1]
+                      + "' is not valid port. Will be used default port "
+                      + port);
+            }
+          }
+        }
+      }
+    }
+
+    Tomcat tomcat = new Tomcat();
+    tomcat.setPort(port);
+
+    Context context = tomcat.addContext(ROOT, new File(".").getAbsolutePath());
+
+    Tomcat.addServlet(
+        context,
+        SERVLET,
+        new DispatcherServlet(
+            new AnnotationConfigWebApplicationContext() {
+              {
+                register(AppConfigurer.class);
+              }
+            }));
+    context.addServletMapping(ROOT, SERVLET);
+
+    tomcat.start();
+    tomcat.getServer().await();
+  }
+}

dd-smoke-tests/appsec/spring-tomcat7/src/test/groovy/datadog/smoketest/appsec/SpringTomcatSmokeTest.groovy

@@ -0,0 +1,36 @@
+package datadog.smoketest.appsec
+
+import okhttp3.Request
+
+class SpringTomcatSmokeTest extends AbstractAppSecServerSmokeTest {
+
+  @Override
+  ProcessBuilder createProcessBuilder() {
+    String springBootShadowJar = System.getProperty("datadog.smoketest.appsec.springtomcat7.shadowJar.path")
+
+    List<String> command = new ArrayList<>()
+    command.add(javaPath())
+    command.addAll(defaultJavaProperties)
+    command.add("-Ddd.iast.enabled=true")
+    command.add("-Ddd.iast.stacktrace-leak.suppress=true")
+    command.addAll((String[]) ["-jar", springBootShadowJar, "--server.port=${httpPort}"])
+
+    ProcessBuilder processBuilder = new ProcessBuilder(command)
+    processBuilder.directory(new File(buildDirectory))
+  }
+
+  def "suppress exception stacktrace"() {
+    when:
+    String url = "http://localhost:${httpPort}/exception"
+    def request = new Request.Builder()
+      .url(url)
+      .build()
+    def response = client.newCall(request).execute()
+    def responseBodyStr = response.body().string()
+    waitForTraceCount 1
+
+    then:
+    responseBodyStr.contains('Sorry, you cannot access this page. Please contact the customer service team.')
+    response.code() == 500
+  }
+}