From 92be78dcf41516f80171a4015ef7411f70295a81 Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 17 Mar 2026 09:46:51 +0100 Subject: [PATCH 1/2] fix(iast): skip XSS check for freemarker built-in escaping expressions in 2.3.24 instrumentation --- .../DollarVariableDatadogAdvice.java | 3 +++ .../core/DollarVariable24Helper.java | 21 +++++++++++++++++++ .../DollarVariableInstrumentationTest.groovy | 17 +++++++++++++++ 3 files changed, 41 insertions(+) diff --git a/dd-java-agent/instrumentation/freemarker/freemarker-2.3.24/src/main/java/datadog/trace/instrumentation/freemarker24/DollarVariableDatadogAdvice.java b/dd-java-agent/instrumentation/freemarker/freemarker-2.3.24/src/main/java/datadog/trace/instrumentation/freemarker24/DollarVariableDatadogAdvice.java index 60f39e2d6bd..794d011c27c 100644 --- a/dd-java-agent/instrumentation/freemarker/freemarker-2.3.24/src/main/java/datadog/trace/instrumentation/freemarker24/DollarVariableDatadogAdvice.java +++ b/dd-java-agent/instrumentation/freemarker/freemarker-2.3.24/src/main/java/datadog/trace/instrumentation/freemarker24/DollarVariableDatadogAdvice.java @@ -27,6 +27,9 @@ public static void onEnter( return; } String charSec = DollarVariable24Helper.fetchCharSec(self, environment); + if (charSec == null) { + return; + } final String templateName = environment.getMainTemplate().getName(); final int line = DollarVariable24Helper.fetchBeginLine(self); xssModule.onXss(charSec, templateName, line); diff --git a/dd-java-agent/instrumentation/freemarker/freemarker-2.3.24/src/main/java/freemarker/core/DollarVariable24Helper.java b/dd-java-agent/instrumentation/freemarker/freemarker-2.3.24/src/main/java/freemarker/core/DollarVariable24Helper.java index 93d7f204e9a..3d608a5337b 100644 --- a/dd-java-agent/instrumentation/freemarker/freemarker-2.3.24/src/main/java/freemarker/core/DollarVariable24Helper.java +++ b/dd-java-agent/instrumentation/freemarker/freemarker-2.3.24/src/main/java/freemarker/core/DollarVariable24Helper.java @@ -12,6 +12,7 @@ private DollarVariable24Helper() {} private static final Logger log = LoggerFactory.getLogger(DollarVariable24Helper.class); private static final Field AUTO_ESCAPE = prepareAutoEscape(); + private static final Field ESCAPED_EXPRESSION = prepareEscapedExpression(); private static Field prepareAutoEscape() { Field autoEscape = null; @@ -25,6 +26,17 @@ private static Field prepareAutoEscape() { return autoEscape; } + private static Field prepareEscapedExpression() { + try { + Field escapedExpression = DollarVariable.class.getDeclaredField("escapedExpression"); + escapedExpression.setAccessible(true); + return escapedExpression; + } catch (Throwable e) { + log.debug("Failed to get DollarVariable escapedExpression", e); + return null; + } + } + public static boolean fetchAutoEscape(Object dollarVariable) { if (AUTO_ESCAPE == null || !(dollarVariable instanceof DollarVariable)) { return true; @@ -40,6 +52,15 @@ public static String fetchCharSec(Object object, Environment environment) { if (!(object instanceof DollarVariable)) { return null; } + if (ESCAPED_EXPRESSION != null) { + try { + if (ESCAPED_EXPRESSION.get(object) instanceof BuiltIn) { + return null; + } + } catch (IllegalAccessException e) { + throw new UndeclaredThrowableException(e); + } + } try { return (String) ((DollarVariable) object).calculateInterpolatedStringOrMarkup(environment); } catch (TemplateException e) { diff --git a/dd-java-agent/instrumentation/freemarker/freemarker-2.3.24/src/test/groovy/datadog/trace/instrumentation/freemarker24/DollarVariableInstrumentationTest.groovy b/dd-java-agent/instrumentation/freemarker/freemarker-2.3.24/src/test/groovy/datadog/trace/instrumentation/freemarker24/DollarVariableInstrumentationTest.groovy index e516aefa5e3..b8cca5c2195 100644 --- a/dd-java-agent/instrumentation/freemarker/freemarker-2.3.24/src/test/groovy/datadog/trace/instrumentation/freemarker24/DollarVariableInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/freemarker/freemarker-2.3.24/src/test/groovy/datadog/trace/instrumentation/freemarker24/DollarVariableInstrumentationTest.groovy @@ -35,4 +35,21 @@ class DollarVariableInstrumentationTest extends InstrumentationSpecification { stringExpression | expression "test" | "test" } + + void 'test freemarker process with built-in escaping does not report xss'() { + given: + final module = Mock(XssModule) + InstrumentationBridge.registerIastModule(module) + + final Configuration cfg = new Configuration() + final Template template = new Template("test", new StringReader('test ${name?html}'), cfg) + final TemplateHashModel rootDataModel = new SimpleHash(cfg.getObjectWrapper()) + rootDataModel.put("name", "") + + when: + template.process(rootDataModel, Mock(FileWriter)) + + then: + 0 * module.onXss(_, _, _) + } } From f746e26f6e98da35b95f6819ccf5ea6ca8cae4af Mon Sep 17 00:00:00 2001 From: "alejandro.gonzalez" Date: Tue, 17 Mar 2026 15:30:21 +0100 Subject: [PATCH 2/2] Fixed. Replaced instanceof BuiltIn with instanceof BuiltInForLegacyEscaping, the abstract base class that covers exactly ?html, ?xml, and ?xhtml in freemarker 2.3.24. Non-escaping built-ins like ?upper_case and ?js_string extend different base classes and are unaffected. Added parameterized unit tests to cover all cases. --- .../core/DollarVariable24Helper.java | 2 +- .../DollarVariableInstrumentationTest.groovy | 33 +++++++++++++++---- 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/dd-java-agent/instrumentation/freemarker/freemarker-2.3.24/src/main/java/freemarker/core/DollarVariable24Helper.java b/dd-java-agent/instrumentation/freemarker/freemarker-2.3.24/src/main/java/freemarker/core/DollarVariable24Helper.java index 3d608a5337b..84c936f02c6 100644 --- a/dd-java-agent/instrumentation/freemarker/freemarker-2.3.24/src/main/java/freemarker/core/DollarVariable24Helper.java +++ b/dd-java-agent/instrumentation/freemarker/freemarker-2.3.24/src/main/java/freemarker/core/DollarVariable24Helper.java @@ -54,7 +54,7 @@ public static String fetchCharSec(Object object, Environment environment) { } if (ESCAPED_EXPRESSION != null) { try { - if (ESCAPED_EXPRESSION.get(object) instanceof BuiltIn) { + if (ESCAPED_EXPRESSION.get(object) instanceof BuiltInForLegacyEscaping) { return null; } } catch (IllegalAccessException e) { diff --git a/dd-java-agent/instrumentation/freemarker/freemarker-2.3.24/src/test/groovy/datadog/trace/instrumentation/freemarker24/DollarVariableInstrumentationTest.groovy b/dd-java-agent/instrumentation/freemarker/freemarker-2.3.24/src/test/groovy/datadog/trace/instrumentation/freemarker24/DollarVariableInstrumentationTest.groovy index b8cca5c2195..6fab3b39ccb 100644 --- a/dd-java-agent/instrumentation/freemarker/freemarker-2.3.24/src/test/groovy/datadog/trace/instrumentation/freemarker24/DollarVariableInstrumentationTest.groovy +++ b/dd-java-agent/instrumentation/freemarker/freemarker-2.3.24/src/test/groovy/datadog/trace/instrumentation/freemarker24/DollarVariableInstrumentationTest.groovy @@ -15,15 +15,32 @@ class DollarVariableInstrumentationTest extends InstrumentationSpecification { injectSysConfig('dd.iast.enabled', 'true') } - void 'test freemarker process'() { + void 'plain interpolation reports xss'() { given: final module = Mock(XssModule) InstrumentationBridge.registerIastModule(module) final Configuration cfg = new Configuration() - final Template template = new Template("test", new StringReader("test \${$stringExpression}"), cfg) + final Template template = new Template("test", new StringReader('test ${name}'), cfg) final TemplateHashModel rootDataModel = new SimpleHash(cfg.getObjectWrapper()) - rootDataModel.put(stringExpression, expression) + rootDataModel.put("name", "") + + when: + template.process(rootDataModel, Mock(FileWriter)) + + then: + 1 * module.onXss(_, _, _) + } + + void 'non-escaping built-in still reports xss'() { + given: + final module = Mock(XssModule) + InstrumentationBridge.registerIastModule(module) + + final Configuration cfg = new Configuration() + final Template template = new Template("test", new StringReader("test \${name?$builtIn}"), cfg) + final TemplateHashModel rootDataModel = new SimpleHash(cfg.getObjectWrapper()) + rootDataModel.put("name", "") when: template.process(rootDataModel, Mock(FileWriter)) @@ -32,17 +49,16 @@ class DollarVariableInstrumentationTest extends InstrumentationSpecification { 1 * module.onXss(_, _, _) where: - stringExpression | expression - "test" | "test" + builtIn << ['upper_case', 'js_string', 'j_string'] } - void 'test freemarker process with built-in escaping does not report xss'() { + void 'html/xml escaping built-ins do not report xss'() { given: final module = Mock(XssModule) InstrumentationBridge.registerIastModule(module) final Configuration cfg = new Configuration() - final Template template = new Template("test", new StringReader('test ${name?html}'), cfg) + final Template template = new Template("test", new StringReader("test \${name?$builtIn}"), cfg) final TemplateHashModel rootDataModel = new SimpleHash(cfg.getObjectWrapper()) rootDataModel.put("name", "") @@ -51,5 +67,8 @@ class DollarVariableInstrumentationTest extends InstrumentationSpecification { then: 0 * module.onXss(_, _, _) + + where: + builtIn << ['html', 'xml', 'xhtml'] } }