Skip to content

Commit 0720a77

Browse files
MariovidoMariovido
authored
Add detection of untrusted deserialization in snakeyaml library (#7406)
Adds instrumentation for snakeyaml library versions prior to 2.0
1 parent 6a34b61 commit 0720a77

10 files changed

Lines changed: 233 additions & 1 deletion

File tree

‎.github/CODEOWNERS‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ dd-java-agent/agent-iast/ @DataDog/asm-java
4343
dd-java-agent/instrumentation/*iast* @DataDog/asm-java
4444
dd-java-agent/instrumentation/*appsec* @DataDog/asm-java
4545
dd-java-agent/instrumentation/json/ @DataDog/asm-java
46+
dd-java-agent/instrumentation/snakeyaml/ @DataDog/asm-java
4647
dd-smoke-tests/iast-util/ @DataDog/asm-java
4748
dd-smoke-tests/spring-security/ @DataDog/asm-java
4849
dd-java-agent/instrumentation/commons-fileupload/ @DataDog/asm-java
@@ -54,4 +55,4 @@ dd-java-agent/instrumentation/spring-security-5/ @DataDog/asm-java
5455

5556
# @DataDog/data-jobs-monitoring
5657
dd-java-agent/instrumentation/spark/ @DataDog/data-jobs-monitoring
57-
dd-java-agent/instrumentation/spark-executor/ @DataDog/data-jobs-monitoring
58+
dd-java-agent/instrumentation/spark-executor/ @DataDog/data-jobs-monitoring

‎dd-java-agent/agent-tooling/src/main/resources/datadog/trace/agent/tooling/bytebuddy/matcher/ignored_class_name.trie‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -354,6 +354,8 @@
354354
0 org.springframework.web.method.support.HandlerMethodReturnValueHandlerComposite
355355
2 org.xml.*
356356
2 org.yaml.snakeyaml.*
357+
# Need for IAST sink
358+
0 org.yaml.snakeyaml.Yaml
357359
# saves ~0.5s skipping instrumentation of almost ~470 classes
358360
2 scala.collection.*
359361

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

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
muzzle {
2+
pass {
3+
name = 'snakeyaml-1.x'
4+
group = "org.yaml"
5+
module = "snakeyaml"
6+
versions = "[1.4, 2.0)"
7+
assertInverse = true
8+
}
9+
fail {
10+
group = "org.yaml"
11+
module = 'snakeyaml'
12+
versions = "[2.0,)"
13+
}
14+
}
15+
16+
apply from: "$rootDir/gradle/java.gradle"
17+
addTestSuiteForDir('latestDepTest', 'test')
18+
19+
dependencies {
20+
compileOnly group: 'org.yaml', name: 'snakeyaml', version: '1.33'
21+
testImplementation group: 'org.yaml', name: 'snakeyaml', version: '1.33'
22+
23+
latestDepTestImplementation group: 'org.yaml', name: 'snakeyaml', version: '1.+'
24+
}

‎dd-java-agent/instrumentation/snakeyaml/src/main/java/datadog/trace/instrumentation/snakeyaml/SnakeYamlHelper.java‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
package datadog.trace.instrumentation.snakeyaml;
2+
3+
import java.lang.reflect.Field;
4+
import java.lang.reflect.UndeclaredThrowableException;
5+
import org.slf4j.Logger;
6+
import org.slf4j.LoggerFactory;
7+
import org.yaml.snakeyaml.Yaml;
8+
import org.yaml.snakeyaml.constructor.BaseConstructor;
9+
10+
public final class SnakeYamlHelper {
11+
private SnakeYamlHelper() {}
12+
13+
private static final Logger log = LoggerFactory.getLogger(SnakeYamlHelper.class);
14+
15+
private static final Field CONSTRUCTOR = prepareConstructor();
16+
17+
private static Field prepareConstructor() {
18+
Field constructor = null;
19+
try {
20+
constructor = Yaml.class.getDeclaredField("constructor");
21+
constructor.setAccessible(true);
22+
} catch (Throwable e) {
23+
log.debug("Failed to get Yaml constructor", e);
24+
return null;
25+
}
26+
return constructor;
27+
}
28+
29+
public static BaseConstructor fetchConstructor(Yaml yaml) {
30+
if (CONSTRUCTOR == null) {
31+
return null;
32+
}
33+
try {
34+
return (BaseConstructor) CONSTRUCTOR.get(yaml);
35+
} catch (IllegalAccessException e) {
36+
throw new UndeclaredThrowableException(e);
37+
}
38+
}
39+
}

‎dd-java-agent/instrumentation/snakeyaml/src/main/java/datadog/trace/instrumentation/snakeyaml/SnakeYamlInstrumenter.java‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
package datadog.trace.instrumentation.snakeyaml;
2+
3+
import static datadog.trace.agent.tooling.bytebuddy.matcher.ClassLoaderMatchers.hasClassNamed;
4+
import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named;
5+
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
6+
import static net.bytebuddy.matcher.ElementMatchers.not;
7+
import static net.bytebuddy.matcher.ElementMatchers.takesArguments;
8+
9+
import com.google.auto.service.AutoService;
10+
import datadog.trace.agent.tooling.Instrumenter;
11+
import datadog.trace.agent.tooling.InstrumenterModule;
12+
import datadog.trace.api.iast.InstrumentationBridge;
13+
import datadog.trace.api.iast.Sink;
14+
import datadog.trace.api.iast.VulnerabilityTypes;
15+
import datadog.trace.api.iast.sink.UntrustedDeserializationModule;
16+
import java.io.InputStream;
17+
import java.io.Reader;
18+
import net.bytebuddy.asm.Advice;
19+
import net.bytebuddy.matcher.ElementMatcher;
20+
import org.yaml.snakeyaml.Yaml;
21+
import org.yaml.snakeyaml.constructor.BaseConstructor;
22+
import org.yaml.snakeyaml.constructor.Constructor;
23+
24+
@AutoService(InstrumenterModule.class)
25+
public class SnakeYamlInstrumenter extends InstrumenterModule.Iast
26+
implements Instrumenter.ForSingleType {
27+
28+
public SnakeYamlInstrumenter() {
29+
super("snakeyaml", "snakeyaml");
30+
}
31+
32+
@Override
33+
public String muzzleDirective() {
34+
return "snakeyaml-1.x";
35+
}
36+
37+
static final ElementMatcher.Junction<ClassLoader> NOT_SNAKEYAML_2 =
38+
not(hasClassNamed("org.yaml.snakeyaml.inspector.TagInspector"));
39+
40+
@Override
41+
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
42+
return NOT_SNAKEYAML_2;
43+
}
44+
45+
@Override
46+
public String[] helperClassNames() {
47+
return new String[] {
48+
packageName + ".SnakeYamlHelper",
49+
};
50+
}
51+
52+
@Override
53+
public String instrumentedType() {
54+
return "org.yaml.snakeyaml.Yaml";
55+
}
56+
57+
@Override
58+
public void methodAdvice(MethodTransformer transformer) {
59+
transformer.applyAdvice(
60+
named("load")
61+
.and(isMethod())
62+
.and(
63+
takesArguments(String.class)
64+
.or(takesArguments(InputStream.class))
65+
.or(takesArguments(Reader.class))),
66+
SnakeYamlInstrumenter.class.getName() + "$LoadAdvice");
67+
}
68+
69+
public static class LoadAdvice {
70+
71+
@Advice.OnMethodEnter(suppress = Throwable.class)
72+
@Sink(VulnerabilityTypes.UNTRUSTED_DESERIALIZATION)
73+
public static void onEnter(
74+
@Advice.Argument(0) final Object data, @Advice.This final Yaml self) {
75+
if (data == null) {
76+
return;
77+
}
78+
final UntrustedDeserializationModule untrustedDeserialization =
79+
InstrumentationBridge.UNTRUSTED_DESERIALIZATION;
80+
if (untrustedDeserialization == null) {
81+
return;
82+
}
83+
final BaseConstructor constructor = SnakeYamlHelper.fetchConstructor(self);
84+
// For versions prior to 1.7 (not included), the constructor field is null
85+
if (constructor instanceof Constructor || constructor == null) {
86+
untrustedDeserialization.onObject(data);
87+
}
88+
}
89+
}
90+
}

‎dd-java-agent/instrumentation/snakeyaml/src/test/groovy/SnakeYamlInstrumenterTest.groovy‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import datadog.trace.agent.test.AgentTestRunner
2+
import datadog.trace.api.iast.InstrumentationBridge
3+
import datadog.trace.api.iast.sink.UntrustedDeserializationModule
4+
import org.yaml.snakeyaml.Yaml
5+
6+
class SnakeYamlInstrumenterTest extends AgentTestRunner {
7+
8+
@Override
9+
protected void configurePreAgent() {
10+
injectSysConfig('dd.iast.enabled', 'true')
11+
}
12+
13+
void 'test snakeyaml load with an input stream'() {
14+
given:
15+
final module = Mock(UntrustedDeserializationModule)
16+
InstrumentationBridge.registerIastModule(module)
17+
18+
final InputStream inputStream = new ByteArrayInputStream("test".getBytes())
19+
20+
when:
21+
new Yaml().load(inputStream)
22+
23+
then:
24+
1 * module.onObject(_)
25+
}
26+
27+
void 'test snakeyaml load with a reader'() {
28+
given:
29+
final module = Mock(UntrustedDeserializationModule)
30+
InstrumentationBridge.registerIastModule(module)
31+
32+
final Reader reader = new StringReader("test")
33+
34+
when:
35+
new Yaml().load(reader)
36+
37+
then:
38+
1 * module.onObject(_)
39+
}
40+
41+
void 'test snakeyaml load with a string'() {
42+
given:
43+
final module = Mock(UntrustedDeserializationModule)
44+
InstrumentationBridge.registerIastModule(module)
45+
46+
final String string = "test"
47+
48+
when:
49+
new Yaml().load(string)
50+
51+
then:
52+
1 * module.onObject(_)
53+
}
54+
}

‎dd-smoke-tests/iast-util/build.gradle‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,5 @@ dependencies {
1010
api project(':dd-smoke-tests')
1111
compileOnly group: 'org.springframework.boot', name: 'spring-boot-starter-web', version: '1.5.18.RELEASE'
1212
compileOnly group: 'com.google.code.gson', name: 'gson', version: '2.10'
13+
compileOnly group: 'org.yaml', name: 'snakeyaml', version: '1.33'
1314
}

‎dd-smoke-tests/iast-util/src/main/java/datadog/smoketest/springboot/controller/IastWebController.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
import org.springframework.web.servlet.view.UrlBasedViewResolver;
5454
import org.w3c.dom.Document;
5555
import org.xml.sax.SAXException;
56+
import org.yaml.snakeyaml.Yaml;
5657

5758
@RestController
5859
public class IastWebController {
@@ -421,6 +422,12 @@ public String untrustedDeserializationParts(HttpServletRequest request)
421422
return "OK";
422423
}
423424

425+
@GetMapping("/untrusted_deserialization/snakeyaml")
426+
public String untrustedDeserializationSnakeYaml(@RequestParam("yaml") String param) {
427+
new Yaml().load(param);
428+
return "OK";
429+
}
430+
424431
private void withProcess(final Operation<Process> op) {
425432
Process process = null;
426433
try {

‎dd-smoke-tests/iast-util/src/testFixtures/groovy/datadog/smoketest/AbstractIastSpringBootTest.groovy‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1122,5 +1122,18 @@ abstract class AbstractIastSpringBootTest extends AbstractIastServerSmokeTest {
11221122
hasVulnerability { vul -> vul.type == 'UNTRUSTED_DESERIALIZATION' }
11231123
}
11241124

1125+
void 'untrusted deserialization for snakeyaml with a string'() {
1126+
setup:
1127+
final String yaml = "test"
1128+
final url = "http://localhost:${httpPort}/untrusted_deserialization/snakeyaml?yaml=${yaml}"
1129+
final request = new Request.Builder().url(url).get().build()
1130+
1131+
when:
1132+
client.newCall(request).execute()
1133+
1134+
then:
1135+
hasVulnerability { vul -> vul.type == 'UNTRUSTED_DESERIALIZATION' }
1136+
}
1137+
11251138

11261139
}

‎settings.gradle‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -410,6 +410,7 @@ include ':dd-java-agent:instrumentation:servlet:request-3'
410410
include ':dd-java-agent:instrumentation:servlet:request-5'
411411
include ':dd-java-agent:instrumentation:shutdown'
412412
include ':dd-java-agent:instrumentation:slick'
413+
include ':dd-java-agent:instrumentation:snakeyaml'
413414
include ':dd-java-agent:instrumentation:span-origin'
414415
include ':dd-java-agent:instrumentation:spark'
415416
include ':dd-java-agent:instrumentation:spark:spark_2.12'

0 commit comments

Comments
 (0)