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']
}
}