CVE-2020-1953

@@ -18,11 +18,14 @@
 package org.apache.commons.configuration2;
 
 import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.ex.ConfigurationRuntimeException;
 import org.apache.commons.configuration2.io.InputStreamSupport;
 import org.apache.commons.configuration2.tree.ImmutableNode;
 import org.yaml.snakeyaml.DumperOptions;
 import org.yaml.snakeyaml.LoaderOptions;
 import org.yaml.snakeyaml.Yaml;
+import org.yaml.snakeyaml.constructor.Constructor;
+import org.yaml.snakeyaml.representer.Representer;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -65,7 +68,7 @@ public void read(final Reader in) throws ConfigurationException
     {
         try
         {
-            final Yaml yaml = new Yaml();
+            final Yaml yaml = createYamlForReading(new LoaderOptions());
             final Map<String, Object> map = (Map) yaml.load(in);
             load(map);
         }
@@ -80,7 +83,7 @@ public void read(final Reader in, final LoaderOptions options)
     {
         try
         {
-            final Yaml yaml = new Yaml(options);
+            final Yaml yaml = createYamlForReading(options);
             final Map<String, Object> map = (Map) yaml.load(in);
             load(map);
         }
@@ -117,7 +120,7 @@ public void read(final InputStream in) throws ConfigurationException
     {
         try
         {
-            final Yaml yaml = new Yaml();
+            final Yaml yaml = createYamlForReading(new LoaderOptions());
             final Map<String, Object> map = (Map) yaml.load(in);
             load(map);
         }
@@ -132,7 +135,7 @@ public void read(final InputStream in, final LoaderOptions options)
     {
         try
         {
-            final Yaml yaml = new Yaml(options);
+            final Yaml yaml = createYamlForReading(options);
             final Map<String, Object> map = (Map) yaml.load(in);
             load(map);
         }
@@ -142,4 +145,34 @@ public void read(final InputStream in, final LoaderOptions options)
         }
     }
 
+    /**
+     * Creates a {@code Yaml} object for reading a Yaml file. The object is
+     * configured with some default settings.
+     *
+     * @param options options for loading the file
+     * @return the {@code Yaml} instance for loading a file
+     */
+    private static Yaml createYamlForReading(LoaderOptions options)
+    {
+        return new Yaml(createClassLoadingDisablingConstructor(), new Representer(), new DumperOptions(), options);
+    }
+
+    /**
+     * Returns a {@code Constructor} object for the YAML parser that prevents
+     * all classes from being loaded. This effectively disables the dynamic
+     * creation of Java objects that are declared in YAML files to be loaded.
+     *
+     * @return the {@code Constructor} preventing object creation
+     */
+    private static Constructor createClassLoadingDisablingConstructor()
+    {
+        return new Constructor()
+        {
+            @Override
+            protected Class<?> getClassForName(String name)
+            {
+                throw new ConfigurationRuntimeException("Class loading is disabled.");
+            }
+        };
+    }
 }

@@ -17,26 +17,37 @@
 
 package org.apache.commons.configuration2;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
-
+import java.io.ByteArrayInputStream;
+import java.io.File;
 import java.io.FileReader;
 import java.io.IOException;
+import java.io.StringReader;
 import java.io.StringWriter;
+import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 
 import org.apache.commons.configuration2.ex.ConfigurationException;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
 import org.yaml.snakeyaml.Yaml;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
 /**
  * Unit test for {@link YAMLConfiguration}
  */
 public class TestYAMLConfiguration
 {
+    @Rule
+    public TemporaryFolder temporaryFolder = new TemporaryFolder();
+
     /** The files that we test with. */
     private final String testYaml =
             ConfigurationAssert.getTestFile("test.yaml").getAbsolutePath();
@@ -134,4 +145,40 @@ public void testCopyConstructor()
         yamlConfiguration = new YAMLConfiguration(c);
         assertEquals("bar", yamlConfiguration.getString("foo"));
     }
+
+    @Test
+    public void testObjectCreationFromReader()
+    {
+        final File createdFile = new File(temporaryFolder.getRoot(), "data.txt");
+        final String yaml = "!!java.io.FileOutputStream [" + createdFile.getAbsolutePath() + "]";
+
+        try
+        {
+            yamlConfiguration.read(new StringReader(yaml));
+            fail("Loading configuration did not cause an exception!");
+        }
+        catch (ConfigurationException e)
+        {
+            //expected
+        }
+        assertFalse("Java object was created", createdFile.exists());
+    }
+
+    @Test
+    public void testObjectCreationFromStream()
+    {
+        final File createdFile = new File(temporaryFolder.getRoot(), "data.txt");
+        final String yaml = "!!java.io.FileOutputStream [" + createdFile.getAbsolutePath() + "]";
+
+        try
+        {
+            yamlConfiguration.read(new ByteArrayInputStream(yaml.getBytes(StandardCharsets.UTF_8)));
+            fail("Loading configuration did not cause an exception!");
+        }
+        catch (ConfigurationException e)
+        {
+            //expected
+        }
+        assertFalse("Java object was created", createdFile.exists());
+    }
 }