Add XSS support for Freemarker post 2.3.24-incubating

dd-java-agent/instrumentation/freemarker-2.3.24/build.gradle

@@ -1,8 +1,9 @@
 muzzle {
   pass {
+    name = 'freemarker-2.3.24'
     group = 'org.freemarker'
     module = 'freemarker'
-    versions = '[2.3.32,]'
+    versions = '[2.3.24-incubating,]'
     assertInverse = true
   }
 }
@@ -13,11 +14,11 @@ apply plugin: 'call-site-instrumentation'
 addTestSuiteForDir('latestDepTest', 'test')
 
 dependencies {
-  compileOnly group: 'org.freemarker', name: 'freemarker', version: '2.3.32'
+  compileOnly group: 'org.freemarker', name: 'freemarker', version: '2.3.24-incubating'
 
-  testImplementation group: 'org.freemarker', name: 'freemarker', version: '2.3.32'
+  testImplementation group: 'org.freemarker', name: 'freemarker', version: '2.3.24-incubating'
 
   testRuntimeOnly project(':dd-java-agent:instrumentation:iast-instrumenter')
 
-  latestDepTestImplementation group: 'org.freemarker', name: 'freemarker', version: '+'
+  latestDepTestImplementation group: 'org.freemarker', name: 'freemarker', version: '2.3.+'
 }

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

@@ -0,0 +1,35 @@
+package datadog.trace.instrumentation.freemarker24;
+
+import datadog.trace.api.iast.InstrumentationBridge;
+import datadog.trace.api.iast.Sink;
+import datadog.trace.api.iast.VulnerabilityTypes;
+import datadog.trace.api.iast.sink.XssModule;
+import freemarker.core.DollarVariable24Helper;
+import freemarker.core.Environment;
+import net.bytebuddy.asm.Advice;
+
+public final class DollarVariableDatadogAdvice {
+
+  public static class DollarVariableAdvice {
+
+    @Advice.OnMethodEnter(suppress = Throwable.class)
+    @Sink(VulnerabilityTypes.XSS)
+    public static void onEnter(
+        @Advice.Argument(0) final Environment environment, @Advice.This final Object self) {
+      if (environment == null || self == null) {
+        return;
+      }
+      final XssModule xssModule = InstrumentationBridge.XSS;
+      if (xssModule == null) {
+        return;
+      }
+      if (DollarVariable24Helper.fetchAutoEscape(self)) {
+        return;
+      }
+      String charSec = DollarVariable24Helper.fetchCharSec(self, environment);
+      final String templateName = environment.getMainTemplate().getName();
+      final int line = DollarVariable24Helper.fetchBeginLine(self);
+      xssModule.onXss(charSec, templateName, line);
+    }
+  }
+}

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

@@ -0,0 +1,55 @@
+package datadog.trace.instrumentation.freemarker24;
+
+import static datadog.trace.agent.tooling.bytebuddy.matcher.ClassLoaderMatchers.hasClassNamed;
+import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
+import static net.bytebuddy.matcher.ElementMatchers.isMethod;
+import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
+
+import com.google.auto.service.AutoService;
+import datadog.trace.agent.tooling.Instrumenter;
+import datadog.trace.agent.tooling.InstrumenterModule;
+import net.bytebuddy.matcher.ElementMatcher;
+
+@AutoService(InstrumenterModule.class)
+public class DollarVariableInstrumentation extends InstrumenterModule.Iast
+    implements Instrumenter.ForSingleType {
+  static final String FREEMARKER_CORE = "freemarker.core";
+
+  public DollarVariableInstrumentation() {
+    super("freemarker");
+  }
+
+  @Override
+  public String muzzleDirective() {
+    return "freemarker-2.3.24";
+  }
+
+  static final ElementMatcher.Junction<ClassLoader> VERSION_POST_2_3_24 =
+      hasClassNamed("freemarker.cache.ByteArrayTemplateLoader");
+
+  @Override
+  public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
+    return VERSION_POST_2_3_24;
+  }
+
+  @Override
+  public String instrumentedType() {
+    return FREEMARKER_CORE + ".DollarVariable";
+  }
+
+  @Override
+  public String[] helperClassNames() {
+    return new String[] {
+      FREEMARKER_CORE + ".DollarVariable24Helper",
+    };
+  }
+
+  @Override
+  public void methodAdvice(MethodTransformer transformer) {
+    transformer.applyAdvice(
+        named("accept")
+            .and(isMethod())
+            .and(takesArgument(0, named(FREEMARKER_CORE + ".Environment"))),
+        packageName + ".DollarVariableDatadogAdvice$DollarVariableAdvice");
+  }
+}

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

@@ -0,0 +1,66 @@
+package datadog.trace.instrumentation.freemarker24;
+
+import static datadog.trace.agent.tooling.bytebuddy.matcher.HierarchyMatchers.implementsInterface;
+import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
+import static net.bytebuddy.matcher.ElementMatchers.returns;
+import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
+
+import com.google.auto.service.AutoService;
+import datadog.trace.advice.ActiveRequestContext;
+import datadog.trace.advice.RequiresRequestContext;
+import datadog.trace.agent.tooling.Instrumenter;
+import datadog.trace.agent.tooling.InstrumenterModule;
+import datadog.trace.api.gateway.RequestContext;
+import datadog.trace.api.gateway.RequestContextSlot;
+import datadog.trace.api.iast.IastContext;
+import datadog.trace.api.iast.InstrumentationBridge;
+import datadog.trace.api.iast.Propagation;
+import datadog.trace.api.iast.propagation.PropagationModule;
+import freemarker.template.TemplateModel;
+import net.bytebuddy.asm.Advice;
+import net.bytebuddy.description.type.TypeDescription;
+import net.bytebuddy.matcher.ElementMatcher;
+
+@AutoService(InstrumenterModule.class)
+public class ObjectWrapperInstrumentation extends InstrumenterModule.Iast
+    implements Instrumenter.ForTypeHierarchy {
+
+  public ObjectWrapperInstrumentation() {
+    super("freemarker");
+  }
+
+  @Override
+  public String hierarchyMarkerType() {
+    return "freemarker.template.ObjectWrapper";
+  }
+
+  @Override
+  public ElementMatcher<TypeDescription> hierarchyMatcher() {
+    return implementsInterface(named(hierarchyMarkerType()));
+  }
+
+  @Override
+  public void methodAdvice(MethodTransformer transformer) {
+    transformer.applyAdvice(
+        named("wrap")
+            .and(takesArgument(0, named("java.lang.Object")))
+            .and(returns(named("freemarker.template.TemplateModel"))),
+        getClass().getName() + "$ObjectWrapperAdvice");
+  }
+
+  @RequiresRequestContext(RequestContextSlot.IAST)
+  public static class ObjectWrapperAdvice {
+    @Advice.OnMethodExit(suppress = Throwable.class)
+    @Propagation
+    public static void onExit(
+        @Advice.Return final TemplateModel templateModel,
+        @Advice.Argument(0) final Object object,
+        @ActiveRequestContext RequestContext reqCtx) {
+      final PropagationModule module = InstrumentationBridge.PROPAGATION;
+      if (module != null) {
+        IastContext ctx = reqCtx.getData(RequestContextSlot.IAST);
+        module.taintObjectIfTainted(ctx, templateModel, object);
+      }
+    }
+  }
+}

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

@@ -0,0 +1,56 @@
+package freemarker.core;
+
+import freemarker.template.TemplateException;
+import java.lang.reflect.Field;
+import java.lang.reflect.UndeclaredThrowableException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public final class DollarVariable24Helper {
+  private DollarVariable24Helper() {}
+
+  private static final Logger log = LoggerFactory.getLogger(DollarVariable24Helper.class);
+
+  private static final Field AUTO_ESCAPE = prepareAutoEscape();
+
+  private static Field prepareAutoEscape() {
+    Field autoEscape = null;
+    try {
+      autoEscape = DollarVariable.class.getDeclaredField("autoEscape");
+      autoEscape.setAccessible(true);
+    } catch (Throwable e) {
+      log.debug("Failed to get DollarVariable autoEscape", e);
+      return null;
+    }
+    return autoEscape;
+  }
+
+  public static boolean fetchAutoEscape(Object dollarVariable) {
+    if (AUTO_ESCAPE == null || !(dollarVariable instanceof DollarVariable)) {
+      return true;
+    }
+    try {
+      return (boolean) AUTO_ESCAPE.get(dollarVariable);
+    } catch (IllegalAccessException e) {
+      throw new UndeclaredThrowableException(e);
+    }
+  }
+
+  public static String fetchCharSec(Object object, Environment environment) {
+    if (!(object instanceof DollarVariable)) {
+      return null;
+    }
+    try {
+      return (String) ((DollarVariable) object).calculateInterpolatedStringOrMarkup(environment);
+    } catch (TemplateException e) {
+      throw new UndeclaredThrowableException(e);
+    }
+  }
+
+  public static Integer fetchBeginLine(Object object) {
+    if (!(object instanceof DollarVariable)) {
+      return null;
+    }
+    return ((DollarVariable) object).beginLine;
+  }
+}

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

@@ -0,0 +1,38 @@
+package datadog.trace.instrumentation.freemarker24
+
+import datadog.trace.agent.test.AgentTestRunner
+import datadog.trace.api.iast.InstrumentationBridge
+import datadog.trace.api.iast.sink.XssModule
+import freemarker.template.Configuration
+import freemarker.template.SimpleHash
+import freemarker.template.Template
+import freemarker.template.TemplateHashModel
+
+class DollarVariableInstrumentationTest extends AgentTestRunner {
+
+  @Override
+  protected void configurePreAgent() {
+    injectSysConfig('dd.iast.enabled', 'true')
+  }
+
+  void 'test freemarker process'() {
+    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 TemplateHashModel rootDataModel = new SimpleHash(cfg.getObjectWrapper())
+    rootDataModel.put(stringExpression, expression)
+
+    when:
+    template.process(rootDataModel, Mock(FileWriter))
+
+    then:
+    1 * module.onXss(_, _, _)
+
+    where:
+    stringExpression | expression
+    "test"           | "test"
+  }
+}

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

@@ -0,0 +1,55 @@
+package datadog.trace.instrumentation.freemarker24
+
+import datadog.trace.agent.test.AgentTestRunner
+import datadog.trace.api.iast.IastContext
+import datadog.trace.api.iast.InstrumentationBridge
+import datadog.trace.api.iast.propagation.PropagationModule
+import datadog.trace.bootstrap.instrumentation.api.AgentTracer
+import datadog.trace.bootstrap.instrumentation.api.TagContext
+import freemarker.template.Configuration
+import freemarker.template.DefaultObjectWrapper
+
+class ObjectWrapperInstrumentationTest extends  AgentTestRunner {
+
+  private Object iastCtx
+
+  @Override
+  protected void configurePreAgent() {
+    injectSysConfig('dd.iast.enabled', 'true')
+  }
+
+  @Override
+  void setup() {
+    iastCtx = Stub(IastContext)
+  }
+
+  @Override
+  void cleanup() {
+    InstrumentationBridge.clearIastModules()
+  }
+
+  void 'test freemarker ObjectWrapper wrap'() {
+    given:
+    final module = Mock(PropagationModule)
+    InstrumentationBridge.registerIastModule(module)
+    final objectWrapper = new DefaultObjectWrapper(Configuration.VERSION_2_3_32)
+    final String wrapped = "test"
+
+    when:
+    runUnderIastTrace { objectWrapper.wrap(wrapped) }
+
+    then:
+    1 * module.taintObjectIfTainted(iastCtx, _, wrapped)
+    0 * _
+  }
+
+  protected <E> E runUnderIastTrace(Closure<E> cl) {
+    final ddctx = new TagContext().withRequestContextDataIast(iastCtx)
+    final span = TEST_TRACER.startSpan("test", "test-iast-span", ddctx)
+    try {
+      return AgentTracer.activateSpan(span).withCloseable(cl)
+    } finally {
+      span.finish()
+    }
+  }
+}

dd-smoke-tests/springboot-freemarker/build.gradle

@@ -0,0 +1,30 @@
+plugins {
+  id 'java'
+  id 'org.springframework.boot' version '2.7.15'
+  id 'io.spring.dependency-management' version '1.0.15.RELEASE'
+  id 'java-test-fixtures'
+}
+
+apply from: "$rootDir/gradle/java.gradle"
+description = 'SpringBoot Freemarker Smoke Tests.'
+
+java {
+  sourceCompatibility = '1.8'
+}
+
+repositories {
+  mavenCentral()
+}
+
+dependencies {
+  implementation group: 'org.springframework.boot', name: 'spring-boot-starter-web', version: '1.5.18.RELEASE'
+  implementation group: 'org.freemarker', name: 'freemarker', version: '2.3.24-incubating'
+
+  testImplementation project(':dd-smoke-tests')
+  testImplementation(testFixtures(project(":dd-smoke-tests:iast-util")))
+}
+
+tasks.withType(Test).configureEach {
+  dependsOn "bootJar"
+  jvmArgs "-Ddatadog.smoketest.springboot.shadowJar.path=${tasks.bootJar.archiveFile.get()}"
+}

dd-smoke-tests/springboot-freemarker/src/main/java/datadog/smoketest/springboot/SpringbootApplication.java

@@ -0,0 +1,11 @@
+package datadog.smoketest.springboot;
+
+import org.springframework.boot.SpringApplication;
+import org.springframework.boot.autoconfigure.SpringBootApplication;
+
+@SpringBootApplication
+public class SpringbootApplication {
+  public static void main(String[] args) {
+    SpringApplication.run(SpringbootApplication.class, args);
+  }
+}