Skip to content

Commit

Permalink
Add XSS support for Freemarker post 2.3.24-incubating (#7532)
Browse files Browse the repository at this point in the history
  • Loading branch information
Mariovido authored Sep 3, 2024
1 parent 7b95d60 commit ff8dcc7
Show file tree
Hide file tree
Showing 16 changed files with 494 additions and 4 deletions.
9 changes: 5 additions & 4 deletions dd-java-agent/instrumentation/freemarker-2.3.24/build.gradle
Original file line number Diff line number Diff line change
@@ -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
}
}
Expand All @@ -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.+'
}
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
Original file line number Diff line number Diff line change
@@ -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");
}
}
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -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"
}
}
Original file line number Diff line number Diff line change
@@ -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()
}
}
}
30 changes: 30 additions & 0 deletions dd-smoke-tests/springboot-freemarker/build.gradle
Original file line number Diff line number Diff line change
@@ -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()}"
}
Original file line number Diff line number Diff line change
@@ -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);
}
}
Loading

0 comments on commit ff8dcc7

Please sign in to comment.