Limit the maximum size of the location path in IAST vulnerabilities (#9028)

What Does This Do
Add truncation to path, class and method if it's necessary for LocationSuppliers to report XSS vulnerabilities

Motivation
incident-39654

In this incident, it was reported that the location.path field of an IAST vulnerability was populated with a large HTML payload, which caused a backend error and prevented the vulnerability from being reported.

This occurred specifically with an XSS vulnerability located in a Thymeleaf template.

Normally, the location.path is extracted from the stacktrace, so this kind of behavior is unusual. However, in cases where vulnerabilities occur in template-based frameworks, we use a different approach to improve precision — specifying the template name instead of the compiled class in the vulnerability location.

In Thymeleaf, the instrumented method getTemplateName may return a full HTML document instead of just the template name, as originally expected.

To guard against these cases, we’ve decided to truncate the values of path, class, and method when they are generated using suppliers rather than stacktrace-based extraction.

(cherry picked from commit b3e2ecd)

Author: jandro996

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

@@ -13,6 +13,8 @@
 
 public class XssModuleImpl extends SinkModuleBase implements XssModule {
 
+  private static final int MAX_LENGTH = 500;
+
   public XssModuleImpl(final Dependencies dependencies) {
     super(dependencies);
   }
@@ -61,6 +63,13 @@ public void onXss(@Nonnull CharSequence s, @Nullable String file, int line) {
     checkInjection(VulnerabilityType.XSS, s, new FileAndLineLocationSupplier(file, line));
   }
 
+  private static String truncate(final String s) {
+    if (s == null || s.length() <= MAX_LENGTH) {
+      return s;
+    }
+    return s.substring(0, MAX_LENGTH);
+  }
+
   private static class ClassMethodLocationSupplier implements LocationSupplier {
     private final String clazz;
     private final String method;
@@ -72,7 +81,7 @@ private ClassMethodLocationSupplier(final String clazz, final String method) {
 
     @Override
     public Location build(final @Nullable AgentSpan span) {
-      return Location.forSpanAndClassAndMethod(span, clazz, method);
+      return Location.forSpanAndClassAndMethod(span, truncate(clazz), truncate(method));
     }
   }
 
@@ -87,7 +96,7 @@ private FileAndLineLocationSupplier(final String file, final int line) {
 
     @Override
     public Location build(@Nullable final AgentSpan span) {
-      return Location.forSpanAndFileAndLine(span, file, line);
+      return Location.forSpanAndFileAndLine(span, truncate(file), line);
     }
   }
 }

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

@@ -155,6 +155,45 @@ class XssModuleTest extends IastModuleImplTestBase {
     '/==>var<==' | VulnerabilityMarks.SQL_INJECTION_MARK| "/==>var<=="
   }
 
+  void 'class and method names are truncated when exceeding max length'() {
+    setup:
+    final param = mapTainted('/==>value<==', NOT_MARKED)
+    final clazz = 'c' * 600
+    final method = 'm' * 600
+
+    when:
+    module.onXss(param, clazz, method)
+
+    then:
+    1 * reporter.report(_, _) >> { args ->
+      final vuln = args[1] as Vulnerability
+      assertEvidence(vuln, '/==>value<==')
+      assert vuln.location.path.length() == 500
+      assert vuln.location.method.length() == 500
+      assert vuln.location.path == clazz.substring(0, 500)
+      assert vuln.location.method == method.substring(0, 500)
+    }
+  }
+
+  void 'file name is truncated when exceeding max length'() {
+    setup:
+    final param = mapTainted('/==>value<==', NOT_MARKED)
+    final file = 'f' * 600
+    final line = 42
+
+    when:
+    module.onXss(param as CharSequence, file, line)
+
+    then:
+    1 * reporter.report(_, _) >> { args ->
+      final vuln = args[1] as Vulnerability
+      assertEvidence(vuln, '/==>value<==')
+      assert vuln.location.path.length() == 500
+      assert vuln.location.line == line
+      assert vuln.location.path == file.substring(0, 500)
+    }
+  }
+
 
   private String mapTainted(final String value, final int mark) {
     final result = addFromTaintFormat(ctx.taintedObjects, value, mark)

dd-smoke-tests/springboot-thymeleaf/src/main/java/datadog/smoketest/springboot/XssController.java

@@ -5,14 +5,43 @@
 import org.springframework.web.bind.annotation.GetMapping;
 import org.springframework.web.bind.annotation.RequestMapping;
 import org.springframework.web.bind.annotation.RequestParam;
+import org.springframework.web.bind.annotation.ResponseBody;
+import org.thymeleaf.context.Context;
+import org.thymeleaf.spring5.SpringTemplateEngine;
+import org.thymeleaf.templateresolver.StringTemplateResolver;
 
 @Controller
 @RequestMapping("/xss")
 public class XssController {
 
+  private static final String TEMPLATE = "<p th:utext=\"${xss}\">Test!</p>";
+
+  private static final String BIG_TEMPLATE =
+      new String(new char[500]).replace('\0', 'A') + "<p th:utext=\"${xss}\">Test!</p>";
+
   @GetMapping("/utext")
   public String utext(@RequestParam(name = "string") String name, Model model) {
     model.addAttribute("xss", name);
     return "utext";
   }
+
+  @GetMapping(value = "/string-template", produces = "text/html")
+  @ResponseBody
+  public String stringTemplate(@RequestParam(name = "string") String name) {
+    SpringTemplateEngine engine = new SpringTemplateEngine();
+    engine.setTemplateResolver(new StringTemplateResolver());
+    Context context = new Context();
+    context.setVariable("xss", name);
+    return engine.process(TEMPLATE, context);
+  }
+
+  @GetMapping(value = "/big-string-template", produces = "text/html")
+  @ResponseBody
+  public String bigStringTemplate(@RequestParam(name = "string") String name) {
+    SpringTemplateEngine engine = new SpringTemplateEngine();
+    engine.setTemplateResolver(new StringTemplateResolver());
+    Context context = new Context();
+    context.setVariable("xss", name);
+    return engine.process(BIG_TEMPLATE, context);
+  }
 }

dd-smoke-tests/springboot-thymeleaf/src/test/groovy/datadog/smoketest/springboot/IastSpringBootThymeleafSmokeTest.groovy

@@ -37,13 +37,50 @@ class IastSpringBootThymeleafSmokeTest extends AbstractIastServerSmokeTest {
     final request = new Request.Builder().url(url).get().build()
 
     when:
-    client.newCall(request).execute()
+    def response = client.newCall(request).execute()
 
     then:
+    response.code() == 200
     hasVulnerability { vul -> vul.type == 'XSS' && vul.location.path == templateName && vul.location.line == line }
 
     where:
     method    | param |templateName| line
     'utext'    | 'test' | 'utext' | 12
   }
+
+  void 'xss with string template returns html as template name'() {
+    setup:
+    final param = '<script>'
+    final encoded = URLEncoder.encode(param, 'UTF-8')
+    final url = "http://localhost:${httpPort}/xss/string-template?string=${encoded}"
+    final request = new Request.Builder().url(url).get().build()
+
+    when:
+    client.newCall(request).execute()
+
+    then:
+    hasVulnerability { vul ->
+      vul.type == 'XSS' &&
+        vul.location.path == '<p th:utext="${xss}">Test!</p>' &&
+        vul.location.line == 1
+    }
+  }
+
+  void 'xss with string template returns html as template name - truncated'() {
+    setup:
+    final param = '<script>'
+    final encoded = URLEncoder.encode(param, 'UTF-8')
+    final url = "http://localhost:${httpPort}/xss/big-string-template?string=${encoded}"
+    final request = new Request.Builder().url(url).get().build()
+
+    when:
+    client.newCall(request).execute()
+
+    then:
+    hasVulnerability { vul ->
+      vul.type == 'XSS' &&
+        vul.location.path == 'A'*500 &&
+        vul.location.line == 1
+    }
+  }
 }