From 7581abc3f776b23b1bab7e80328fc482c041cc2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Gonz=C3=A1lez=20Garc=C3=ADa?= Date: Thu, 3 Jul 2025 13:21:17 +0200 Subject: [PATCH] Limit the maximum size of the location path in IAST vulnerabilities (#9028) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 b3e2ecda54eb3fa33c12935ec794f12966331150) --- .../com/datadog/iast/sink/XssModuleImpl.java | 13 ++++++- .../datadog/iast/sink/XssModuleTest.groovy | 39 +++++++++++++++++++ .../smoketest/springboot/XssController.java | 29 ++++++++++++++ .../IastSpringBootThymeleafSmokeTest.groovy | 39 ++++++++++++++++++- 4 files changed, 117 insertions(+), 3 deletions(-) diff --git a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/XssModuleImpl.java b/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/XssModuleImpl.java index b417ca13031..e0becd04810 100644 --- a/dd-java-agent/agent-iast/src/main/java/com/datadog/iast/sink/XssModuleImpl.java +++ b/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); } } } diff --git a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/XssModuleTest.groovy b/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/XssModuleTest.groovy index 19006a4ee60..2032f6feb9c 100644 --- a/dd-java-agent/agent-iast/src/test/groovy/com/datadog/iast/sink/XssModuleTest.groovy +++ b/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) diff --git a/dd-smoke-tests/springboot-thymeleaf/src/main/java/datadog/smoketest/springboot/XssController.java b/dd-smoke-tests/springboot-thymeleaf/src/main/java/datadog/smoketest/springboot/XssController.java index bd1ab538931..fa8b534c1b6 100644 --- a/dd-smoke-tests/springboot-thymeleaf/src/main/java/datadog/smoketest/springboot/XssController.java +++ b/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 = "

Test!

"; + + private static final String BIG_TEMPLATE = + new String(new char[500]).replace('\0', 'A') + "

Test!

"; + @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); + } } diff --git a/dd-smoke-tests/springboot-thymeleaf/src/test/groovy/datadog/smoketest/springboot/IastSpringBootThymeleafSmokeTest.groovy b/dd-smoke-tests/springboot-thymeleaf/src/test/groovy/datadog/smoketest/springboot/IastSpringBootThymeleafSmokeTest.groovy index 4fec5e7061a..219c8cac4a6 100644 --- a/dd-smoke-tests/springboot-thymeleaf/src/test/groovy/datadog/smoketest/springboot/IastSpringBootThymeleafSmokeTest.groovy +++ b/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 = '