Skip to content

Commit

Permalink
Add detection of untrusted deserialization in snakeyaml library (#7406)
Browse files Browse the repository at this point in the history
Adds instrumentation for snakeyaml library versions prior to 2.0
  • Loading branch information
Mariovido authored Aug 23, 2024
1 parent 6a34b61 commit 0720a77
Show file tree
Hide file tree
Showing 10 changed files with 233 additions and 1 deletion.
3 changes: 2 additions & 1 deletion .github/CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ dd-java-agent/agent-iast/ @DataDog/asm-java
dd-java-agent/instrumentation/*iast* @DataDog/asm-java
dd-java-agent/instrumentation/*appsec* @DataDog/asm-java
dd-java-agent/instrumentation/json/ @DataDog/asm-java
dd-java-agent/instrumentation/snakeyaml/ @DataDog/asm-java
dd-smoke-tests/iast-util/ @DataDog/asm-java
dd-smoke-tests/spring-security/ @DataDog/asm-java
dd-java-agent/instrumentation/commons-fileupload/ @DataDog/asm-java
Expand All @@ -54,4 +55,4 @@ dd-java-agent/instrumentation/spring-security-5/ @DataDog/asm-java

# @DataDog/data-jobs-monitoring
dd-java-agent/instrumentation/spark/ @DataDog/data-jobs-monitoring
dd-java-agent/instrumentation/spark-executor/ @DataDog/data-jobs-monitoring
dd-java-agent/instrumentation/spark-executor/ @DataDog/data-jobs-monitoring
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,8 @@
0 org.springframework.web.method.support.HandlerMethodReturnValueHandlerComposite
2 org.xml.*
2 org.yaml.snakeyaml.*
# Need for IAST sink
0 org.yaml.snakeyaml.Yaml
# saves ~0.5s skipping instrumentation of almost ~470 classes
2 scala.collection.*

Expand Down
24 changes: 24 additions & 0 deletions dd-java-agent/instrumentation/snakeyaml/build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
muzzle {
pass {
name = 'snakeyaml-1.x'
group = "org.yaml"
module = "snakeyaml"
versions = "[1.4, 2.0)"
assertInverse = true
}
fail {
group = "org.yaml"
module = 'snakeyaml'
versions = "[2.0,)"
}
}

apply from: "$rootDir/gradle/java.gradle"
addTestSuiteForDir('latestDepTest', 'test')

dependencies {
compileOnly group: 'org.yaml', name: 'snakeyaml', version: '1.33'
testImplementation group: 'org.yaml', name: 'snakeyaml', version: '1.33'

latestDepTestImplementation group: 'org.yaml', name: 'snakeyaml', version: '1.+'
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package datadog.trace.instrumentation.snakeyaml;

import java.lang.reflect.Field;
import java.lang.reflect.UndeclaredThrowableException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.yaml.snakeyaml.Yaml;
import org.yaml.snakeyaml.constructor.BaseConstructor;

public final class SnakeYamlHelper {
private SnakeYamlHelper() {}

private static final Logger log = LoggerFactory.getLogger(SnakeYamlHelper.class);

private static final Field CONSTRUCTOR = prepareConstructor();

private static Field prepareConstructor() {
Field constructor = null;
try {
constructor = Yaml.class.getDeclaredField("constructor");
constructor.setAccessible(true);
} catch (Throwable e) {
log.debug("Failed to get Yaml constructor", e);
return null;
}
return constructor;
}

public static BaseConstructor fetchConstructor(Yaml yaml) {
if (CONSTRUCTOR == null) {
return null;
}
try {
return (BaseConstructor) CONSTRUCTOR.get(yaml);
} catch (IllegalAccessException e) {
throw new UndeclaredThrowableException(e);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package datadog.trace.instrumentation.snakeyaml;

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.not;
import static net.bytebuddy.matcher.ElementMatchers.takesArguments;

import com.google.auto.service.AutoService;
import datadog.trace.agent.tooling.Instrumenter;
import datadog.trace.agent.tooling.InstrumenterModule;
import datadog.trace.api.iast.InstrumentationBridge;
import datadog.trace.api.iast.Sink;
import datadog.trace.api.iast.VulnerabilityTypes;
import datadog.trace.api.iast.sink.UntrustedDeserializationModule;
import java.io.InputStream;
import java.io.Reader;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.matcher.ElementMatcher;
import org.yaml.snakeyaml.Yaml;
import org.yaml.snakeyaml.constructor.BaseConstructor;
import org.yaml.snakeyaml.constructor.Constructor;

@AutoService(InstrumenterModule.class)
public class SnakeYamlInstrumenter extends InstrumenterModule.Iast
implements Instrumenter.ForSingleType {

public SnakeYamlInstrumenter() {
super("snakeyaml", "snakeyaml");
}

@Override
public String muzzleDirective() {
return "snakeyaml-1.x";
}

static final ElementMatcher.Junction<ClassLoader> NOT_SNAKEYAML_2 =
not(hasClassNamed("org.yaml.snakeyaml.inspector.TagInspector"));

@Override
public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
return NOT_SNAKEYAML_2;
}

@Override
public String[] helperClassNames() {
return new String[] {
packageName + ".SnakeYamlHelper",
};
}

@Override
public String instrumentedType() {
return "org.yaml.snakeyaml.Yaml";
}

@Override
public void methodAdvice(MethodTransformer transformer) {
transformer.applyAdvice(
named("load")
.and(isMethod())
.and(
takesArguments(String.class)
.or(takesArguments(InputStream.class))
.or(takesArguments(Reader.class))),
SnakeYamlInstrumenter.class.getName() + "$LoadAdvice");
}

public static class LoadAdvice {

@Advice.OnMethodEnter(suppress = Throwable.class)
@Sink(VulnerabilityTypes.UNTRUSTED_DESERIALIZATION)
public static void onEnter(
@Advice.Argument(0) final Object data, @Advice.This final Yaml self) {
if (data == null) {
return;
}
final UntrustedDeserializationModule untrustedDeserialization =
InstrumentationBridge.UNTRUSTED_DESERIALIZATION;
if (untrustedDeserialization == null) {
return;
}
final BaseConstructor constructor = SnakeYamlHelper.fetchConstructor(self);
// For versions prior to 1.7 (not included), the constructor field is null
if (constructor instanceof Constructor || constructor == null) {
untrustedDeserialization.onObject(data);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import datadog.trace.agent.test.AgentTestRunner
import datadog.trace.api.iast.InstrumentationBridge
import datadog.trace.api.iast.sink.UntrustedDeserializationModule
import org.yaml.snakeyaml.Yaml

class SnakeYamlInstrumenterTest extends AgentTestRunner {

@Override
protected void configurePreAgent() {
injectSysConfig('dd.iast.enabled', 'true')
}

void 'test snakeyaml load with an input stream'() {
given:
final module = Mock(UntrustedDeserializationModule)
InstrumentationBridge.registerIastModule(module)

final InputStream inputStream = new ByteArrayInputStream("test".getBytes())

when:
new Yaml().load(inputStream)

then:
1 * module.onObject(_)
}

void 'test snakeyaml load with a reader'() {
given:
final module = Mock(UntrustedDeserializationModule)
InstrumentationBridge.registerIastModule(module)

final Reader reader = new StringReader("test")

when:
new Yaml().load(reader)

then:
1 * module.onObject(_)
}

void 'test snakeyaml load with a string'() {
given:
final module = Mock(UntrustedDeserializationModule)
InstrumentationBridge.registerIastModule(module)

final String string = "test"

when:
new Yaml().load(string)

then:
1 * module.onObject(_)
}
}
1 change: 1 addition & 0 deletions dd-smoke-tests/iast-util/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,5 @@ dependencies {
api project(':dd-smoke-tests')
compileOnly group: 'org.springframework.boot', name: 'spring-boot-starter-web', version: '1.5.18.RELEASE'
compileOnly group: 'com.google.code.gson', name: 'gson', version: '2.10'
compileOnly group: 'org.yaml', name: 'snakeyaml', version: '1.33'
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
import org.springframework.web.servlet.view.UrlBasedViewResolver;
import org.w3c.dom.Document;
import org.xml.sax.SAXException;
import org.yaml.snakeyaml.Yaml;

@RestController
public class IastWebController {
Expand Down Expand Up @@ -421,6 +422,12 @@ public String untrustedDeserializationParts(HttpServletRequest request)
return "OK";
}

@GetMapping("/untrusted_deserialization/snakeyaml")
public String untrustedDeserializationSnakeYaml(@RequestParam("yaml") String param) {
new Yaml().load(param);
return "OK";
}

private void withProcess(final Operation<Process> op) {
Process process = null;
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1122,5 +1122,18 @@ abstract class AbstractIastSpringBootTest extends AbstractIastServerSmokeTest {
hasVulnerability { vul -> vul.type == 'UNTRUSTED_DESERIALIZATION' }
}

void 'untrusted deserialization for snakeyaml with a string'() {
setup:
final String yaml = "test"
final url = "http://localhost:${httpPort}/untrusted_deserialization/snakeyaml?yaml=${yaml}"
final request = new Request.Builder().url(url).get().build()

when:
client.newCall(request).execute()

then:
hasVulnerability { vul -> vul.type == 'UNTRUSTED_DESERIALIZATION' }
}


}
1 change: 1 addition & 0 deletions settings.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,7 @@ include ':dd-java-agent:instrumentation:servlet:request-3'
include ':dd-java-agent:instrumentation:servlet:request-5'
include ':dd-java-agent:instrumentation:shutdown'
include ':dd-java-agent:instrumentation:slick'
include ':dd-java-agent:instrumentation:snakeyaml'
include ':dd-java-agent:instrumentation:span-origin'
include ':dd-java-agent:instrumentation:spark'
include ':dd-java-agent:instrumentation:spark:spark_2.12'
Expand Down

0 comments on commit 0720a77

Please sign in to comment.