Skip to content

Commit 92be78d

Browse files
committed
fix(iast): skip XSS check for freemarker built-in escaping expressions in 2.3.24 instrumentation
1 parent 57fc106 commit 92be78d

3 files changed

Lines changed: 41 additions & 0 deletions

File tree

dd-java-agent/instrumentation/freemarker/freemarker-2.3.24/src/main/java/datadog/trace/instrumentation/freemarker24/DollarVariableDatadogAdvice.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ public static void onEnter(
2727
return;
2828
}
2929
String charSec = DollarVariable24Helper.fetchCharSec(self, environment);
30+
if (charSec == null) {
31+
return;
32+
}
3033
final String templateName = environment.getMainTemplate().getName();
3134
final int line = DollarVariable24Helper.fetchBeginLine(self);
3235
xssModule.onXss(charSec, templateName, line);

dd-java-agent/instrumentation/freemarker/freemarker-2.3.24/src/main/java/freemarker/core/DollarVariable24Helper.java

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ private DollarVariable24Helper() {}
1212
private static final Logger log = LoggerFactory.getLogger(DollarVariable24Helper.class);
1313

1414
private static final Field AUTO_ESCAPE = prepareAutoEscape();
15+
private static final Field ESCAPED_EXPRESSION = prepareEscapedExpression();
1516

1617
private static Field prepareAutoEscape() {
1718
Field autoEscape = null;
@@ -25,6 +26,17 @@ private static Field prepareAutoEscape() {
2526
return autoEscape;
2627
}
2728

29+
private static Field prepareEscapedExpression() {
30+
try {
31+
Field escapedExpression = DollarVariable.class.getDeclaredField("escapedExpression");
32+
escapedExpression.setAccessible(true);
33+
return escapedExpression;
34+
} catch (Throwable e) {
35+
log.debug("Failed to get DollarVariable escapedExpression", e);
36+
return null;
37+
}
38+
}
39+
2840
public static boolean fetchAutoEscape(Object dollarVariable) {
2941
if (AUTO_ESCAPE == null || !(dollarVariable instanceof DollarVariable)) {
3042
return true;
@@ -40,6 +52,15 @@ public static String fetchCharSec(Object object, Environment environment) {
4052
if (!(object instanceof DollarVariable)) {
4153
return null;
4254
}
55+
if (ESCAPED_EXPRESSION != null) {
56+
try {
57+
if (ESCAPED_EXPRESSION.get(object) instanceof BuiltIn) {
58+
return null;
59+
}
60+
} catch (IllegalAccessException e) {
61+
throw new UndeclaredThrowableException(e);
62+
}
63+
}
4364
try {
4465
return (String) ((DollarVariable) object).calculateInterpolatedStringOrMarkup(environment);
4566
} catch (TemplateException e) {

dd-java-agent/instrumentation/freemarker/freemarker-2.3.24/src/test/groovy/datadog/trace/instrumentation/freemarker24/DollarVariableInstrumentationTest.groovy

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,4 +35,21 @@ class DollarVariableInstrumentationTest extends InstrumentationSpecification {
3535
stringExpression | expression
3636
"test" | "test"
3737
}
38+
39+
void 'test freemarker process with built-in escaping does not report xss'() {
40+
given:
41+
final module = Mock(XssModule)
42+
InstrumentationBridge.registerIastModule(module)
43+
44+
final Configuration cfg = new Configuration()
45+
final Template template = new Template("test", new StringReader('test ${name?html}'), cfg)
46+
final TemplateHashModel rootDataModel = new SimpleHash(cfg.getObjectWrapper())
47+
rootDataModel.put("name", "<script>alert(1)</script>")
48+
49+
when:
50+
template.process(rootDataModel, Mock(FileWriter))
51+
52+
then:
53+
0 * module.onXss(_, _, _)
54+
}
3855
}

0 commit comments

Comments
 (0)