diff --git a/dd-java-agent/instrumentation/freemarker-2.3.24/build.gradle b/dd-java-agent/instrumentation/freemarker-2.3.24/build.gradle index f2d5ef612aa..0845f3a63b6 100644 --- a/dd-java-agent/instrumentation/freemarker-2.3.24/build.gradle +++ b/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.+' } diff --git a/dd-java-agent/instrumentation/freemarker-2.3.24/src/main/java/datadog/trace/instrumentation/freemarker24/DollarVariableDatadogAdvice.java b/dd-java-agent/instrumentation/freemarker-2.3.24/src/main/java/datadog/trace/instrumentation/freemarker24/DollarVariableDatadogAdvice.java new file mode 100644 index 00000000000..60f39e2d6bd --- /dev/null +++ b/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); + } + } +} diff --git a/dd-java-agent/instrumentation/freemarker-2.3.24/src/main/java/datadog/trace/instrumentation/freemarker24/DollarVariableInstrumentation.java b/dd-java-agent/instrumentation/freemarker-2.3.24/src/main/java/datadog/trace/instrumentation/freemarker24/DollarVariableInstrumentation.java new file mode 100644 index 00000000000..0cd06b61794 --- /dev/null +++ b/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 VERSION_POST_2_3_24 = + hasClassNamed("freemarker.cache.ByteArrayTemplateLoader"); + + @Override + public ElementMatcher.Junction 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"); + } +} diff --git a/dd-java-agent/instrumentation/freemarker-2.3.24/src/main/java/datadog/trace/instrumentation/freemarker24/ObjectWrapperInstrumentation.java b/dd-java-agent/instrumentation/freemarker-2.3.24/src/main/java/datadog/trace/instrumentation/freemarker24/ObjectWrapperInstrumentation.java new file mode 100644 index 00000000000..0e521c04d7b --- /dev/null +++ b/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 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); + } + } + } +} diff --git a/dd-java-agent/instrumentation/freemarker-2.3.24/src/main/java/freemarker/core/DollarVariable24Helper.java b/dd-java-agent/instrumentation/freemarker-2.3.24/src/main/java/freemarker/core/DollarVariable24Helper.java new file mode 100644 index 00000000000..93d7f204e9a --- /dev/null +++ b/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; + } +} diff --git a/dd-java-agent/instrumentation/freemarker-2.3.24/src/test/groovy/datadog/trace/instrumentation/freemarker24/DollarVariableInstrumentationTest.groovy b/dd-java-agent/instrumentation/freemarker-2.3.24/src/test/groovy/datadog/trace/instrumentation/freemarker24/DollarVariableInstrumentationTest.groovy new file mode 100644 index 00000000000..bb85fe021d6 --- /dev/null +++ b/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" + } +} diff --git a/dd-java-agent/instrumentation/freemarker-2.3.24/src/test/groovy/datadog/trace/instrumentation/freemarker24/ObjectWrapperInstrumentationTest.groovy b/dd-java-agent/instrumentation/freemarker-2.3.24/src/test/groovy/datadog/trace/instrumentation/freemarker24/ObjectWrapperInstrumentationTest.groovy new file mode 100644 index 00000000000..dfc3cbefe4d --- /dev/null +++ b/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 runUnderIastTrace(Closure 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() + } + } +} diff --git a/dd-smoke-tests/springboot-freemarker/build.gradle b/dd-smoke-tests/springboot-freemarker/build.gradle new file mode 100644 index 00000000000..9cbd3352d80 --- /dev/null +++ b/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()}" +} diff --git a/dd-smoke-tests/springboot-freemarker/src/main/java/datadog/smoketest/springboot/SpringbootApplication.java b/dd-smoke-tests/springboot-freemarker/src/main/java/datadog/smoketest/springboot/SpringbootApplication.java new file mode 100644 index 00000000000..2b4e4de452a --- /dev/null +++ b/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); + } +} diff --git a/dd-smoke-tests/springboot-freemarker/src/main/java/datadog/smoketest/springboot/XssController.java b/dd-smoke-tests/springboot-freemarker/src/main/java/datadog/smoketest/springboot/XssController.java new file mode 100644 index 00000000000..4ddb2ae7915 --- /dev/null +++ b/dd-smoke-tests/springboot-freemarker/src/main/java/datadog/smoketest/springboot/XssController.java @@ -0,0 +1,37 @@ +package datadog.smoketest.springboot; + +import freemarker.template.Configuration; +import freemarker.template.Template; +import freemarker.template.TemplateException; +import java.io.File; +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; +import javax.servlet.http.HttpServletResponse; +import org.springframework.stereotype.Controller; +import org.springframework.web.bind.annotation.GetMapping; +import org.springframework.web.bind.annotation.RequestMapping; +import org.springframework.web.bind.annotation.RequestParam; + +@Controller() +@RequestMapping("/xss") +public class XssController { + + private static final String DIRECTORY_TEMPLATES_TEST = "resources/main/templates"; + private static final String DIRECTORY_TEMPLATES_RUN = + "dd-smoke-tests/springboot-freemarker/src/main/resources/templates"; + + @GetMapping(value = "/freemarker") + public void freemarker( + @RequestParam(name = "name") String name, + @RequestParam(name = "templateName") String templateName, + final HttpServletResponse response) + throws IOException, TemplateException { + Configuration cfg = new Configuration(); + cfg.setDirectoryForTemplateLoading(new File(DIRECTORY_TEMPLATES_TEST)); + Template template = cfg.getTemplate(templateName); + Map root = new HashMap<>(); + root.put("name", name); + template.process(root, response.getWriter()); + } +} diff --git a/dd-smoke-tests/springboot-freemarker/src/main/resources/templates/freemarker-2.3.24-insecure.ftlh b/dd-smoke-tests/springboot-freemarker/src/main/resources/templates/freemarker-2.3.24-insecure.ftlh new file mode 100644 index 00000000000..7340e478647 --- /dev/null +++ b/dd-smoke-tests/springboot-freemarker/src/main/resources/templates/freemarker-2.3.24-insecure.ftlh @@ -0,0 +1,11 @@ +<#ftl output_format="HTML" auto_esc=false> +

Output format: ${.output_format} +

Auto-escaping: ${.auto_esc?c} + + + Welcome! + + +

Welcome ${name}!

+ + diff --git a/dd-smoke-tests/springboot-freemarker/src/main/resources/templates/freemarker-2.3.24-secure.ftlh b/dd-smoke-tests/springboot-freemarker/src/main/resources/templates/freemarker-2.3.24-secure.ftlh new file mode 100644 index 00000000000..bd6d860eaf6 --- /dev/null +++ b/dd-smoke-tests/springboot-freemarker/src/main/resources/templates/freemarker-2.3.24-secure.ftlh @@ -0,0 +1,11 @@ +<#ftl output_format="HTML" auto_esc=true> +

Output format: ${.output_format} +

Auto-escaping: ${.auto_esc?c} + + + Welcome! + + +

Welcome ${name}!

+ + diff --git a/dd-smoke-tests/springboot-freemarker/src/main/resources/templates/freemarker-2.3.9-insecure.ftlh b/dd-smoke-tests/springboot-freemarker/src/main/resources/templates/freemarker-2.3.9-insecure.ftlh new file mode 100644 index 00000000000..b946eacd5df --- /dev/null +++ b/dd-smoke-tests/springboot-freemarker/src/main/resources/templates/freemarker-2.3.9-insecure.ftlh @@ -0,0 +1,8 @@ + + + Welcome! + + +

Welcome ${name}!

+ + diff --git a/dd-smoke-tests/springboot-freemarker/src/main/resources/templates/freemarker-2.3.9-secure.ftlh b/dd-smoke-tests/springboot-freemarker/src/main/resources/templates/freemarker-2.3.9-secure.ftlh new file mode 100644 index 00000000000..6063e352022 --- /dev/null +++ b/dd-smoke-tests/springboot-freemarker/src/main/resources/templates/freemarker-2.3.9-secure.ftlh @@ -0,0 +1,8 @@ + + + Welcome! + + +

Welcome ${name?html}!

+ + diff --git a/dd-smoke-tests/springboot-freemarker/src/test/groovy/datadog/smoketest/springboot/IastSpringBootFreemarkerSmokeTest.groovy b/dd-smoke-tests/springboot-freemarker/src/test/groovy/datadog/smoketest/springboot/IastSpringBootFreemarkerSmokeTest.groovy new file mode 100644 index 00000000000..1fd1f13dbae --- /dev/null +++ b/dd-smoke-tests/springboot-freemarker/src/test/groovy/datadog/smoketest/springboot/IastSpringBootFreemarkerSmokeTest.groovy @@ -0,0 +1,67 @@ +package datadog.smoketest.springboot + +import datadog.smoketest.AbstractIastServerSmokeTest +import okhttp3.Request + +import static datadog.trace.api.config.IastConfig.IAST_DEBUG_ENABLED +import static datadog.trace.api.config.IastConfig.IAST_DETECTION_MODE +import static datadog.trace.api.config.IastConfig.IAST_ENABLED +import static datadog.trace.api.config.IastConfig.IAST_REDACTION_ENABLED + +class IastSpringBootFreemarkerSmokeTest extends AbstractIastServerSmokeTest { + + @Override + ProcessBuilder createProcessBuilder() { + String springBootShadowJar = System.getProperty('datadog.smoketest.springboot.shadowJar.path') + + List command = [] + command.add(javaPath()) + command.addAll(defaultJavaProperties) + command.addAll([ + withSystemProperty(IAST_ENABLED, true), + withSystemProperty(IAST_DETECTION_MODE, 'FULL'), + withSystemProperty(IAST_DEBUG_ENABLED, true), + withSystemProperty(IAST_REDACTION_ENABLED, false) + ]) + command.addAll((String[]) ['-jar', springBootShadowJar, "--server.port=${httpPort}"]) + ProcessBuilder processBuilder = new ProcessBuilder(command) + processBuilder.directory(new File(buildDirectory)) + // Spring will print all environment variables to the log, which may pollute it and affect log assertions. + processBuilder.environment().clear() + return processBuilder + } + + void 'xss is present'() { + setup: + final url = "http://localhost:${httpPort}/xss/freemarker?name=${param}&templateName=${templateName}" + final request = new Request.Builder().url(url).get().build() + + when: + client.newCall(request).execute() + + then: + hasVulnerability { vul -> vul.type == 'XSS' && vul.location.path == templateName && vul.location.line == line } + + where: + param | templateName | line + 'name' | 'freemarker-2.3.24-insecure.ftlh' | 9 + 'name' | 'freemarker-2.3.9-insecure.ftlh' | 6 + } + + void 'xss is not present'() { + setup: + final url = "http://localhost:${httpPort}/xss/freemarker?name=${param}&templateName=${templateName}" + final request = new Request.Builder().url(url).get().build() + + when: + client.newCall(request).execute() + + then: + noVulnerability { vul -> vul.type == 'XSS' && vul.location.path == templateName && vul.location.line == line } + + where: + param | templateName | line + 'name' | 'freemarker-2.3.24-secure.ftlh' | 9 + 'name' | 'freemarker-2.3.9-secure.ftlh' | 6 + } +} diff --git a/settings.gradle b/settings.gradle index 94b11a5ef52..d11ced5d336 100644 --- a/settings.gradle +++ b/settings.gradle @@ -138,6 +138,7 @@ include ':dd-smoke-tests:spring-boot-3.0-webmvc' include ':dd-smoke-tests:spring-boot-rabbit' include ':dd-smoke-tests:spring-security' include ':dd-smoke-tests:springboot' +include ':dd-smoke-tests:springboot-freemarker' include ':dd-smoke-tests:springboot-grpc' include ':dd-smoke-tests:springboot-jetty-jsp' include ':dd-smoke-tests:springboot-mongo'