Skip to content

Commit 3f5051e

Browse files
MariovidoMariovido
authored
Add XSS support for Velocity (#7546)
1 parent e7631e6 commit 3f5051e

File tree

16 files changed

+433
-0
lines changed

16 files changed

+433
-0
lines changed

‎.github/CODEOWNERS‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ dd-java-agent/instrumentation/*iast* @DataDog/asm-java
4545
dd-java-agent/instrumentation/*appsec* @DataDog/asm-java
4646
dd-java-agent/instrumentation/json/ @DataDog/asm-java
4747
dd-java-agent/instrumentation/snakeyaml/ @DataDog/asm-java
48+
dd-java-agent/instrumentation/velocity/ @DataDog/asm-java
4849
dd-java-agent/instrumentation/freemarker/ @DataDog/asm-java
4950
dd-smoke-tests/iast-util/ @DataDog/asm-java
5051
dd-smoke-tests/spring-security/ @DataDog/asm-java

‎dd-java-agent/instrumentation/velocity/build.gradle‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
2+
apply from: "$rootDir/gradle/java.gradle"
3+
apply plugin: 'call-site-instrumentation'
4+
addTestSuiteForDir('latestDepTest', 'test')
5+
6+
dependencies {
7+
compileOnly group: 'org.apache.velocity', name: 'velocity', version: '1.5'
8+
compileOnly group: 'org.apache.velocity', name: 'velocity-tools', version: '1.3'
9+
testImplementation group: 'org.apache.velocity', name: 'velocity', version: '1.5'
10+
testImplementation group: 'org.apache.velocity', name: 'velocity-tools', version: '1.3'
11+
12+
testRuntimeOnly project(':dd-java-agent:instrumentation:iast-instrumenter')
13+
14+
latestDepTestImplementation group: 'org.apache.velocity', name: 'velocity', version: '+'
15+
latestDepTestImplementation group: 'org.apache.velocity', name: 'velocity-tools', version: '+'
16+
}

‎dd-java-agent/instrumentation/velocity/src/main/java/datadog/trace/instrumentation/velocity/ASTReferenceInstrumentation.java‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
package datadog.trace.instrumentation.velocity;
2+
3+
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
4+
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
5+
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
6+
7+
import com.google.auto.service.AutoService;
8+
import datadog.trace.agent.tooling.Instrumenter;
9+
import datadog.trace.agent.tooling.InstrumenterModule;
10+
import datadog.trace.api.iast.InstrumentationBridge;
11+
import datadog.trace.api.iast.Sink;
12+
import datadog.trace.api.iast.VulnerabilityTypes;
13+
import datadog.trace.api.iast.sink.XssModule;
14+
import net.bytebuddy.asm.Advice;
15+
import org.apache.velocity.context.InternalContextAdapter;
16+
import org.apache.velocity.runtime.parser.node.ASTReference;
17+
18+
@AutoService(InstrumenterModule.class)
19+
public class ASTReferenceInstrumentation extends InstrumenterModule.Iast
20+
implements Instrumenter.ForSingleType {
21+
public ASTReferenceInstrumentation() {
22+
super("velocity");
23+
}
24+
25+
@Override
26+
public String instrumentedType() {
27+
return "org.apache.velocity.runtime.parser.node.ASTReference";
28+
}
29+
30+
@Override
31+
public void methodAdvice(MethodTransformer transformer) {
32+
transformer.applyAdvice(
33+
named("render")
34+
.and(isMethod())
35+
.and(
36+
takesArgument(0, named("org.apache.velocity.context.InternalContextAdapter"))
37+
.and(takesArgument(1, named("java.io.Writer")))),
38+
ASTReferenceInstrumentation.class.getName() + "$ASTReferenceAdvice");
39+
}
40+
41+
public static class ASTReferenceAdvice {
42+
43+
@Advice.OnMethodEnter(suppress = Throwable.class)
44+
@Sink(VulnerabilityTypes.XSS)
45+
public static void onEnter(
46+
@Advice.Argument(0) final InternalContextAdapter context,
47+
@Advice.This final ASTReference self) {
48+
if (self == null) {
49+
return;
50+
}
51+
final XssModule xssModule = InstrumentationBridge.XSS;
52+
if (xssModule == null) {
53+
return;
54+
}
55+
Object variable = self.getVariableValue(context, self.getRootString());
56+
// For cases when you have a variable that is not a string such as the EscapeTool variable
57+
if (!(variable instanceof String)) {
58+
return;
59+
}
60+
final String charSec = (String) variable;
61+
final String file = context.getCurrentTemplateName();
62+
final int line = self.getLine();
63+
xssModule.onXss(charSec, file, line);
64+
}
65+
}
66+
}

‎dd-java-agent/instrumentation/velocity/src/main/java/datadog/trace/instrumentation/velocity/EscapeToolCallSite.java‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
package datadog.trace.instrumentation.velocity;
2+
3+
import datadog.trace.agent.tooling.csi.CallSite;
4+
import datadog.trace.api.iast.IastCallSites;
5+
import datadog.trace.api.iast.InstrumentationBridge;
6+
import datadog.trace.api.iast.Propagation;
7+
import datadog.trace.api.iast.VulnerabilityMarks;
8+
import datadog.trace.api.iast.propagation.PropagationModule;
9+
import javax.annotation.Nullable;
10+
import org.apache.velocity.tools.generic.EscapeTool;
11+
12+
@Propagation
13+
@CallSite(spi = IastCallSites.class)
14+
public class EscapeToolCallSite {
15+
16+
@CallSite.After(
17+
"java.lang.String org.apache.velocity.tools.generic.EscapeTool.html(java.lang.Object)")
18+
@CallSite.After(
19+
"java.lang.String org.apache.velocity.tools.generic.EscapeTool.javascript(java.lang.Object)")
20+
@CallSite.After(
21+
"java.lang.String org.apache.velocity.tools.generic.EscapeTool.url(java.lang.Object)")
22+
@CallSite.After(
23+
"java.lang.String org.apache.velocity.tools.generic.EscapeTool.xml(java.lang.Object)")
24+
public static String afterEscape(
25+
@CallSite.This final EscapeTool self,
26+
@CallSite.Argument(0) @Nullable final Object input,
27+
@CallSite.Return final String result) {
28+
final PropagationModule module = InstrumentationBridge.PROPAGATION;
29+
if (module != null) {
30+
try {
31+
module.taintStringIfTainted(result, input, false, VulnerabilityMarks.XSS_MARK);
32+
} catch (final Throwable e) {
33+
module.onUnexpectedException("afterEscape threw", e);
34+
}
35+
}
36+
return result;
37+
}
38+
39+
@CallSite.After(
40+
"java.lang.String org.apache.velocity.tools.generic.EscapeTool.sql(java.lang.Object)")
41+
public static String afterEscapeSQL(
42+
@CallSite.This final EscapeTool self,
43+
@CallSite.Argument(0) @Nullable final Object input,
44+
@CallSite.Return final String result) {
45+
final PropagationModule module = InstrumentationBridge.PROPAGATION;
46+
if (module != null) {
47+
try {
48+
module.taintStringIfTainted(result, input, false, VulnerabilityMarks.SQL_INJECTION_MARK);
49+
} catch (final Throwable e) {
50+
module.onUnexpectedException("afterEscapeSQL threw", e);
51+
}
52+
}
53+
return result;
54+
}
55+
}

‎dd-java-agent/instrumentation/velocity/src/test/groovy/datadog/trace/instrumentation/velocity/ASTReferenceInstrumentationTest.groovy‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
package datadog.trace.instrumentation.velocity
2+
3+
import datadog.trace.agent.test.AgentTestRunner
4+
import datadog.trace.api.iast.InstrumentationBridge
5+
import datadog.trace.api.iast.sink.XssModule
6+
import org.apache.velocity.Template
7+
import org.apache.velocity.VelocityContext
8+
import org.apache.velocity.app.VelocityEngine
9+
import org.apache.velocity.runtime.RuntimeConstants
10+
import org.apache.velocity.tools.generic.EscapeTool
11+
12+
class ASTReferenceInstrumentationTest extends AgentTestRunner {
13+
14+
@Override
15+
protected void configurePreAgent() {
16+
injectSysConfig('dd.iast.enabled', 'true')
17+
}
18+
19+
void 'test ASTReference execute (insecure)'() {
20+
given:
21+
final module = Mock(XssModule)
22+
InstrumentationBridge.registerIastModule(module)
23+
24+
VelocityEngine velocity = new VelocityEngine()
25+
velocity.setProperty(
26+
RuntimeConstants.RUNTIME_LOG_LOGSYSTEM_CLASS,
27+
"org.apache.velocity.runtime.log.NullLogChute")
28+
velocity.init()
29+
Template template = velocity.getTemplate("src/test/resources/velocity-astreference-insecure.vm")
30+
31+
VelocityContext context = new VelocityContext()
32+
context.put("param", param)
33+
34+
when:
35+
template.merge(context, Mock(FileWriter))
36+
37+
then:
38+
1 * module.onXss(_, _, _)
39+
40+
where:
41+
param << ["<script>alert(1)</script>", "name"]
42+
}
43+
44+
void 'test ASTReference execute (secure)'() {
45+
given:
46+
final module = Mock(XssModule)
47+
InstrumentationBridge.registerIastModule(module)
48+
49+
VelocityEngine velocity = new VelocityEngine()
50+
velocity.setProperty(
51+
RuntimeConstants.RUNTIME_LOG_LOGSYSTEM_CLASS,
52+
"org.apache.velocity.runtime.log.NullLogChute")
53+
velocity.init()
54+
Template template = velocity.getTemplate("src/test/resources/velocity-astreference-secure.vm")
55+
56+
VelocityContext context = new VelocityContext()
57+
context.put("esc", new EscapeTool())
58+
context.put("param", param)
59+
60+
when:
61+
template.merge(context, Mock(FileWriter))
62+
63+
then:
64+
0 * module.onXss(_, _, _)
65+
66+
where:
67+
param << ["<script>alert(1)</script>", "name"]
68+
}
69+
}

‎dd-java-agent/instrumentation/velocity/src/test/groovy/datadog/trace/instrumentation/velocity/EscapeToolCallSiteTest.groovy‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
package datadog.trace.instrumentation.velocity
2+
3+
import datadog.trace.agent.test.AgentTestRunner
4+
import datadog.trace.api.iast.InstrumentationBridge
5+
import datadog.trace.api.iast.propagation.PropagationModule
6+
import foo.bar.TestEscapeToolSuite
7+
import groovy.transform.CompileDynamic
8+
9+
import static datadog.trace.api.iast.VulnerabilityMarks.SQL_INJECTION_MARK
10+
import static datadog.trace.api.iast.VulnerabilityMarks.XSS_MARK
11+
12+
@CompileDynamic
13+
class EscapeToolCallSiteTest extends AgentTestRunner {
14+
15+
@Override
16+
protected void configurePreAgent() {
17+
injectSysConfig("dd.iast.enabled", "true")
18+
}
19+
20+
void 'test #method'() {
21+
given:
22+
final module = Mock(PropagationModule)
23+
InstrumentationBridge.registerIastModule(module)
24+
25+
when:
26+
final result = TestEscapeToolSuite.&"$method".call(args)
27+
28+
then:
29+
result == expected
30+
1 * module.taintStringIfTainted(_ as String, args[0], false, mark)
31+
0 * _
32+
33+
where:
34+
method | args | mark | expected
35+
'html' | ['Ø-This is a quote'] | XSS_MARK | '&Oslash;-This is a quote'
36+
'javascript' | ['Ø-This is a quote'] | XSS_MARK | '\\u00D8-This is a quote'
37+
'url' | ['Ø-This is a quote'] | XSS_MARK | '%C3%98-This+is+a+quote'
38+
'xml' | ['Ø-This is a quote'] | XSS_MARK | '&#216;-This is a quote'
39+
'sql' | ['Ø-This is a quote'] | SQL_INJECTION_MARK | 'Ø-This is a quote'
40+
}
41+
}

‎dd-java-agent/instrumentation/velocity/src/test/java/foo/bar/TestEscapeToolSuite.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package foo.bar;
2+
3+
import org.apache.velocity.tools.generic.EscapeTool;
4+
5+
public class TestEscapeToolSuite {
6+
7+
public static EscapeTool escapeTool = new EscapeTool();
8+
9+
public static String html(String input) {
10+
return escapeTool.html(input);
11+
}
12+
13+
public static String javascript(String input) {
14+
return escapeTool.javascript(input);
15+
}
16+
17+
public static String url(String input) {
18+
return escapeTool.url(input);
19+
}
20+
21+
public static String xml(String input) {
22+
return escapeTool.xml(input);
23+
}
24+
25+
public static String sql(String input) {
26+
return escapeTool.sql(input);
27+
}
28+
}

‎dd-java-agent/instrumentation/velocity/src/test/resources/velocity-astreference-insecure.vm‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
<html>
2+
I am vulnerable $param
3+
</html>

‎dd-java-agent/instrumentation/velocity/src/test/resources/velocity-astreference-secure.vm‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
<html>
2+
I am vulnerable $esc.html($param)
3+
</html>

‎dd-smoke-tests/springboot-velocity/build.gradle‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
plugins {
2+
id 'java'
3+
id 'org.springframework.boot' version '2.7.15'
4+
id 'io.spring.dependency-management' version '1.0.15.RELEASE'
5+
id 'java-test-fixtures'
6+
}
7+
8+
apply from: "$rootDir/gradle/java.gradle"
9+
description = 'SpringBoot Velocity Smoke Tests.'
10+
11+
dependencies {
12+
implementation group: 'org.springframework.boot', name: 'spring-boot-starter-web', version: '1.5.18.RELEASE'
13+
implementation group: 'org.apache.velocity', name: 'velocity', version: '1.5'
14+
implementation(group: 'org.apache.velocity', name: 'velocity-tools', version: '1.3') {
15+
exclude group: 'javax.servlet', module: 'servlet-api'
16+
}
17+
18+
testImplementation project(':dd-smoke-tests')
19+
testImplementation(testFixtures(project(":dd-smoke-tests:iast-util")))
20+
}
21+
22+
tasks.withType(Test).configureEach {
23+
dependsOn "bootJar"
24+
jvmArgs "-Ddatadog.smoketest.springboot.shadowJar.path=${tasks.bootJar.archiveFile.get()}"
25+
}

0 commit comments

Comments
 (0)