From 587b840666e4a487b11a9d42b007aea10e9c7b50 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Wed, 27 May 2020 16:36:46 -0700 Subject: [PATCH] Restrict use of custom YAML types Update `YamlJsonParser` and `OriginTrackedYamlLoader` to ensure that custom types cannot be loaded. Closes gh-21596 --- .../boot/env/OriginTrackedYamlLoader.java | 3 +- .../boot/json/YamlJsonParser.java | 29 +++++++++++++++++-- .../env/OriginTrackedYamlLoaderTests.java | 12 ++++++++ .../boot/json/YamlJsonParserTests.java | 11 +++++++ 4 files changed, 52 insertions(+), 3 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/env/OriginTrackedYamlLoader.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/env/OriginTrackedYamlLoader.java index 7b2d260d589..1955bc5e364 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/env/OriginTrackedYamlLoader.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/env/OriginTrackedYamlLoader.java @@ -27,6 +27,7 @@ import org.yaml.snakeyaml.LoaderOptions; import org.yaml.snakeyaml.Yaml; import org.yaml.snakeyaml.constructor.BaseConstructor; import org.yaml.snakeyaml.constructor.Constructor; +import org.yaml.snakeyaml.constructor.SafeConstructor; import org.yaml.snakeyaml.error.Mark; import org.yaml.snakeyaml.nodes.MappingNode; import org.yaml.snakeyaml.nodes.Node; @@ -79,7 +80,7 @@ class OriginTrackedYamlLoader extends YamlProcessor { /** * {@link Constructor} that tracks property origins. */ - private class OriginTrackingConstructor extends Constructor { + private class OriginTrackingConstructor extends SafeConstructor { @Override protected Object constructObject(Node node) { diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/YamlJsonParser.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/YamlJsonParser.java index c46d864a068..afec8622d9f 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/YamlJsonParser.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/json/YamlJsonParser.java @@ -16,10 +16,15 @@ package org.springframework.boot.json; +import java.util.Collections; +import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Set; +import java.util.stream.Collectors; import org.yaml.snakeyaml.Yaml; +import org.yaml.snakeyaml.constructor.Constructor; /** * Thin wrapper to adapt Snake {@link Yaml} to {@link JsonParser}. @@ -31,16 +36,36 @@ import org.yaml.snakeyaml.Yaml; */ public class YamlJsonParser extends AbstractJsonParser { + private final Yaml yaml = new Yaml(new TypeLimitedConstructor()); + @Override @SuppressWarnings("unchecked") public Map parseMap(String json) { - return parseMap(json, (trimmed) -> new Yaml().loadAs(trimmed, Map.class)); + return parseMap(json, (trimmed) -> this.yaml.loadAs(trimmed, Map.class)); } @Override @SuppressWarnings("unchecked") public List parseList(String json) { - return parseList(json, (trimmed) -> new Yaml().loadAs(trimmed, List.class)); + return parseList(json, (trimmed) -> this.yaml.loadAs(trimmed, List.class)); + } + + private static class TypeLimitedConstructor extends Constructor { + + private static final Set SUPPORTED_TYPES; + static { + Set> supportedTypes = new LinkedHashSet<>(); + supportedTypes.add(List.class); + supportedTypes.add(Map.class); + SUPPORTED_TYPES = supportedTypes.stream().map(Class::getName) + .collect(Collectors.collectingAndThen(Collectors.toSet(), Collections::unmodifiableSet)); + } + + @Override + protected Class getClassForName(String name) throws ClassNotFoundException { + return (SUPPORTED_TYPES.contains(name)) ? super.getClassForName(name) : null; + } + } } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/env/OriginTrackedYamlLoaderTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/env/OriginTrackedYamlLoaderTests.java index ac536c964d7..e9873a85a7e 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/env/OriginTrackedYamlLoaderTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/env/OriginTrackedYamlLoaderTests.java @@ -16,18 +16,22 @@ package org.springframework.boot.env; +import java.nio.charset.StandardCharsets; import java.util.List; import java.util.Map; import org.junit.Before; import org.junit.Test; +import org.yaml.snakeyaml.constructor.ConstructorException; import org.springframework.boot.origin.OriginTrackedValue; import org.springframework.boot.origin.TextResourceOrigin; +import org.springframework.core.io.ByteArrayResource; import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.Resource; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; /** * Tests for {@link OriginTrackedYamlLoader}. @@ -116,6 +120,14 @@ public class OriginTrackedYamlLoaderTests { assertThat(getLocation(nullValue)).isEqualTo("28:13"); } + @Test + public void unsupportedType() throws Exception { + String yaml = "value: !!java.net.URL [!!java.lang.String [!!java.lang.StringBuilder [\"http://localhost:9000/\"]]]"; + Resource resource = new ByteArrayResource(yaml.getBytes(StandardCharsets.UTF_8)); + this.loader = new OriginTrackedYamlLoader(resource); + assertThatExceptionOfType(ConstructorException.class).isThrownBy(this.loader::load); + } + private OriginTrackedValue getValue(String name) { if (this.result == null) { this.result = this.loader.load(); diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/json/YamlJsonParserTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/json/YamlJsonParserTests.java index 26c31e2b07e..c1da2c59e2b 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/json/YamlJsonParserTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/json/YamlJsonParserTests.java @@ -16,6 +16,11 @@ package org.springframework.boot.json; +import org.junit.Test; +import org.yaml.snakeyaml.constructor.ConstructorException; + +import static org.assertj.core.api.Assertions.assertThatExceptionOfType; + /** * Tests for {@link YamlJsonParser}. * @@ -28,4 +33,10 @@ public class YamlJsonParserTests extends AbstractJsonParserTests { return new YamlJsonParser(); } + @Test + public void customTypesAreNotLoaded() throws Exception { + assertThatExceptionOfType(ConstructorException.class) + .isThrownBy(() -> getParser().parseMap("{value: !!java.net.URL [\"http://localhost:9000/\"]}")); + } + }