From fe85a7d9e7c0eaaa7e8547d2a7e941c6e40a7783 Mon Sep 17 00:00:00 2001
From: Arnout Engelen <arnout@bzzt.net>
Date: Tue, 1 Feb 2022 14:29:45 +0100
Subject: [PATCH] Avoid instantiating arbitrary classes

This PR avoids the use of `Class#newInstance`, which is
deprecated in Java 9.

In particular, previously you could set the `config.strategy`
system to an arbitrary class that would get instantiated even
if it was not a subclass of `ConfigLoadingStrategy`. This is
now checked before instantiating the class.

The previous behavior could arguably be considered a security
concern when an attacker has write access to system properties,
though in such a scenario there are likely many other ways to
load arbitrary code.
---
 .../src/main/java/com/typesafe/config/ConfigFactory.java   | 7 ++++++-
 .../main/java/com/typesafe/config/impl/ConfigBeanImpl.java | 6 ++++--
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/config/src/main/java/com/typesafe/config/ConfigFactory.java b/config/src/main/java/com/typesafe/config/ConfigFactory.java
index 3f50ab168..c4c0ba38a 100644
--- a/config/src/main/java/com/typesafe/config/ConfigFactory.java
+++ b/config/src/main/java/com/typesafe/config/ConfigFactory.java
@@ -8,6 +8,7 @@
 
 import java.io.File;
 import java.io.Reader;
+import java.lang.reflect.InvocationTargetException;
 import java.net.MalformedURLException;
 import java.net.URL;
 import java.util.Map;
@@ -1241,7 +1242,11 @@ private static ConfigLoadingStrategy getConfigLoadingStrategy() {
 
         if (className != null) {
             try {
-                return ConfigLoadingStrategy.class.cast(Class.forName(className).newInstance());
+                return Class.forName(className).asSubclass(ConfigLoadingStrategy.class).getDeclaredConstructor().newInstance();
+            } catch (InvocationTargetException e) {
+                Throwable cause = e.getCause();
+                if (cause == null) throw new ConfigException.BugOrBroken("Failed to load strategy: " + className, e);
+                else throw new ConfigException.BugOrBroken("Failed to load strategy: " + className, cause);
             } catch (Throwable e) {
                 throw new ConfigException.BugOrBroken("Failed to load strategy: " + className, e);
             }
diff --git a/config/src/main/java/com/typesafe/config/impl/ConfigBeanImpl.java b/config/src/main/java/com/typesafe/config/impl/ConfigBeanImpl.java
index 2b9cf5c60..329c7c9c4 100644
--- a/config/src/main/java/com/typesafe/config/impl/ConfigBeanImpl.java
+++ b/config/src/main/java/com/typesafe/config/impl/ConfigBeanImpl.java
@@ -106,7 +106,7 @@ public static <T> T createInternal(Config config, Class<T> clazz) {
             }
 
             // Fill in the bean instance
-            T bean = clazz.newInstance();
+            T bean = clazz.getDeclaredConstructor().newInstance();
             for (PropertyDescriptor beanProp : beanProps) {
                 Method setter = beanProp.getWriteMethod();
                 Type parameterType = setter.getGenericParameterTypes()[0];
@@ -125,8 +125,10 @@ public static <T> T createInternal(Config config, Class<T> clazz) {
                 setter.invoke(bean, unwrapped);
             }
             return bean;
-        } catch (InstantiationException e) {
+        } catch (NoSuchMethodException e) {
             throw new ConfigException.BadBean(clazz.getName() + " needs a public no-args constructor to be used as a bean", e);
+        } catch (InstantiationException e) {
+            throw new ConfigException.BadBean(clazz.getName() + " needs to be instantiable to be used as a bean", e);
         } catch (IllegalAccessException e) {
             throw new ConfigException.BadBean(clazz.getName() + " getters and setters are not accessible, they must be for use as a bean", e);
         } catch (InvocationTargetException e) {