From 0d80f46cef37d9f4c779373ab5d099c5f478d3f3 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 31 Aug 2020 14:31:05 -0700 Subject: [PATCH] Remove node and recursive limits for YAML Update `OriginTrackedYamlLoader` to remove node limits and recursive parsing restrictions. SnakeYAML 1.26 introduced these options in order to protect against the "billion laugh attacks" but since we consider `application.yml` files to be trusted, we don't need these restrictions. Fixes gh-23096 --- .../boot/env/OriginTrackedYamlLoader.java | 16 ++++++++-- .../env/OriginTrackedYamlLoaderTests.java | 29 +++++++++++++++++++ .../springframework/boot/env/recursive.yml | 7 +++++ 3 files changed, 49 insertions(+), 3 deletions(-) create mode 100644 spring-boot-project/spring-boot/src/test/resources/org/springframework/boot/env/recursive.yml 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 ce92ea842c..39da94f7b5 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 @@ -62,12 +62,18 @@ class OriginTrackedYamlLoader extends YamlProcessor { @Override protected Yaml createYaml() { - BaseConstructor constructor = new OriginTrackingConstructor(); + LoaderOptions loaderOptions = new LoaderOptions(); + loaderOptions.setAllowDuplicateKeys(false); + loaderOptions.setMaxAliasesForCollections(Integer.MAX_VALUE); + loaderOptions.setAllowRecursiveKeys(true); + return createYaml(loaderOptions); + } + + private Yaml createYaml(LoaderOptions loaderOptions) { + BaseConstructor constructor = new OriginTrackingConstructor(loaderOptions); Representer representer = new Representer(); DumperOptions dumperOptions = new DumperOptions(); LimitedResolver resolver = new LimitedResolver(); - LoaderOptions loaderOptions = new LoaderOptions(); - loaderOptions.setAllowDuplicateKeys(false); return new Yaml(constructor, representer, dumperOptions, loaderOptions, resolver); } @@ -82,6 +88,10 @@ class OriginTrackedYamlLoader extends YamlProcessor { */ private class OriginTrackingConstructor extends SafeConstructor { + OriginTrackingConstructor(LoaderOptions loadingConfig) { + super(loadingConfig); + } + @Override protected Object constructObject(Node node) { if (node instanceof ScalarNode) { 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 480759c179..1b36ad6e3b 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 @@ -128,6 +128,35 @@ class OriginTrackedYamlLoaderTests { assertThatExceptionOfType(ConstructorException.class).isThrownBy(this.loader::load); } + @Test + void loadWhenLargeNumberOfNodesLoadsYaml() { + StringBuilder yaml = new StringBuilder(); + int size = 500; + yaml.append("defs:\n"); + for (int i = 0; i < size; i++) { + yaml.append(" - def" + i + ": &def" + i + "\n"); + yaml.append(" - value: " + i + "\n"); + } + yaml.append("refs:\n"); + for (int i = 0; i < size; i++) { + yaml.append(" ref" + i + ":\n"); + yaml.append(" - value: *def" + i + "\n"); + } + Resource resource = new ByteArrayResource(yaml.toString().getBytes(StandardCharsets.UTF_8)); + this.loader = new OriginTrackedYamlLoader(resource); + Map loaded = this.loader.load().get(0); + assertThat(loaded).hasSize(size * 2); + } + + @Test + void loadWhenRecursiveLoadsYaml() { + Resource resource = new ClassPathResource("recursive.yml", getClass()); + this.loader = new OriginTrackedYamlLoader(resource); + Map loaded = this.loader.load().get(0); + assertThat(loaded.get("test.a.spring")).hasToString("a"); + assertThat(loaded.get("test.b.boot")).hasToString("b"); + } + private OriginTrackedValue getValue(String name) { if (this.result == null) { this.result = this.loader.load(); diff --git a/spring-boot-project/spring-boot/src/test/resources/org/springframework/boot/env/recursive.yml b/spring-boot-project/spring-boot/src/test/resources/org/springframework/boot/env/recursive.yml new file mode 100644 index 0000000000..3d4ca7d182 --- /dev/null +++ b/spring-boot-project/spring-boot/src/test/resources/org/springframework/boot/env/recursive.yml @@ -0,0 +1,7 @@ +&def1 +*def1: a +test: + a: + spring: 'a' + b: + boot: 'b'