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..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 @@ -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 BuiltInForLegacyEscaping) { + 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..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,24 +15,60 @@ 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)) + + then: + 1 * module.onXss(_, _, _) + + where: + builtIn << ['upper_case', 'js_string', 'j_string'] + } + + 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?$builtIn}"), cfg) + final TemplateHashModel rootDataModel = new SimpleHash(cfg.getObjectWrapper()) + rootDataModel.put("name", "") + + when: + template.process(rootDataModel, Mock(FileWriter)) + + then: + 0 * module.onXss(_, _, _) where: - stringExpression | expression - "test" | "test" + builtIn << ['html', 'xml', 'xhtml'] } }