CVE-2022-21724: Improper Initialization (CWE-665)
value: pgjdbc is the offical PostgreSQL JDBC Driver. A security hole was found in the jdbc driver for postgresql database while doing security research. The system using the postgresql library will be attacked when attacker control the jdbc url or properties. pgjdbc instantiates plugin instances based on class names provided via `authenticationPluginClassName`, `sslhostnameverifier`, `socketFactory`, `sslfactory`, `sslpasswordcallback` connection properties. However, the driver did not verify if the class implements the expected interface before instantiating the class. This can lead to code execution loaded via arbitrary classes. Users using plugins are advised to upgrade. There are no known workarounds for this issue.
CWE description: The product does not initialize or incorrectly initializes a resource, which might leave the resource in an unexpected state when it is accessed or used.
CVSS v3 base score: 9.8
Published: 2022-02-02T12:15Z
Repository: https://github.com/pgjdbc/pgjdbc
Language: Java
Files changed: pgjdbc/src/main/java/org/postgresql/core/SocketFactoryFactory.java, pgjdbc/src/main/java/org/postgresql/core/v3/AuthenticationPluginManager.java, pgjdbc/src/main/java/org/postgresql/ssl/LibPQFactory.java, pgjdbc/src/main/java/org/postgresql/ssl/MakeSSL.java, pgjdbc/src/main/java/org/postgresql/util/ObjectFactory.java, pgjdbc/src/test/java/org/postgresql/test/util/ObjectFactoryTest.java
Commit date: 2022-02-01 07:27:13 -0500
Commit message: Merge pull request from GHSA-v7wg-cpwc-24m4

This ensures arbitrary classes can't be passed instead of
AuthenticationPlugin, SocketFactory, SSLSocketFactory, CallbackHandler, HostnameVerifier
Diff: diff --git a/pgjdbc/src/main/java/org/postgresql/core/SocketFactoryFactory.java b/pgjdbc/src/main/java/org/postgresql/core/SocketFactoryFactory.java
index 09efa75f0..fe56354a2 100644
--- a/pgjdbc/src/main/java/org/postgresql/core/SocketFactoryFactory.java
+++ b/pgjdbc/src/main/java/org/postgresql/core/SocketFactoryFactory.java
@@ -34,11 +34,11 @@ public class SocketFactoryFactory {
     String socketFactoryClassName = PGProperty.SOCKET_FACTORY.get(info);
     if (socketFactoryClassName == null) {
       return SocketFactory.getDefault();
     }
     try {
-      return (SocketFactory) ObjectFactory.instantiate(socketFactoryClassName, info, true,
+      return ObjectFactory.instantiate(SocketFactory.class, socketFactoryClassName, info, true,
           PGProperty.SOCKET_FACTORY_ARG.get(info));
     } catch (Exception e) {
       throw new PSQLException(
           GT.tr("The SocketFactory class provided {0} could not be instantiated.",
               socketFactoryClassName),
@@ -59,11 +59,11 @@ public class SocketFactoryFactory {
         || "org.postgresql.ssl.jdbc4.LibPQFactory".equals(classname)
         || "org.postgresql.ssl.LibPQFactory".equals(classname)) {
       return new LibPQFactory(info);
     }
     try {
-      return (SSLSocketFactory) ObjectFactory.instantiate(classname, info, true,
+      return ObjectFactory.instantiate(SSLSocketFactory.class, classname, info, true,
           PGProperty.SSL_FACTORY_ARG.get(info));
     } catch (Exception e) {
       throw new PSQLException(
           GT.tr("The SSLSocketFactory class provided {0} could not be instantiated.", classname),
           PSQLState.CONNECTION_FAILURE, e);
diff --git a/pgjdbc/src/main/java/org/postgresql/core/v3/AuthenticationPluginManager.java b/pgjdbc/src/main/java/org/postgresql/core/v3/AuthenticationPluginManager.java
index 938f24632..bcc8b3a9b 100644
--- a/pgjdbc/src/main/java/org/postgresql/core/v3/AuthenticationPluginManager.java
+++ b/pgjdbc/src/main/java/org/postgresql/core/v3/AuthenticationPluginManager.java
@@ -64,15 +64,16 @@ class AuthenticationPluginManager {
         password = passwordText.toCharArray();
       }
     } else {
       AuthenticationPlugin authPlugin;
       try {
-        authPlugin = (AuthenticationPlugin) ObjectFactory.instantiate(authPluginClassName, info,
+        authPlugin = ObjectFactory.instantiate(AuthenticationPlugin.class, authPluginClassName, info,
             false, null);
       } catch (Exception ex) {
-        LOGGER.log(Level.FINE, "Unable to load Authentication Plugin " + ex.toString());
-        throw new PSQLException(ex.getMessage(), PSQLState.UNEXPECTED_ERROR);
+        String msg = GT.tr("Unable to load Authentication Plugin {0}", authPluginClassName);
+        LOGGER.log(Level.FINE, msg, ex);
+        throw new PSQLException(msg, PSQLState.INVALID_PARAMETER_VALUE, ex);
       }
 
       password = authPlugin.getPassword(type);
     }
 
@@ -104,11 +105,12 @@ class AuthenticationPluginManager {
   public static <T> T withEncodedPassword(AuthenticationRequestType type, Properties info,
       PasswordAction<byte[], T> action) throws PSQLException, IOException {
     byte[] encodedPassword = withPassword(type, info, password -> {
       if (password == null) {
         throw new PSQLException(
-            GT.tr("The server requested password-based authentication, but no password was provided."),
+            GT.tr("The server requested password-based authentication, but no password was provided by plugin {0}",
+                PGProperty.AUTHENTICATION_PLUGIN_CLASS_NAME.get(info)),
             PSQLState.CONNECTION_REJECTED);
       }
       ByteBuffer buf = StandardCharsets.UTF_8.encode(CharBuffer.wrap(password));
       byte[] bytes = new byte[buf.limit()];
       buf.get(bytes);
diff --git a/pgjdbc/src/main/java/org/postgresql/ssl/LibPQFactory.java b/pgjdbc/src/main/java/org/postgresql/ssl/LibPQFactory.java
index 67e1196b1..4249dcda7 100644
--- a/pgjdbc/src/main/java/org/postgresql/ssl/LibPQFactory.java
+++ b/pgjdbc/src/main/java/org/postgresql/ssl/LibPQFactory.java
@@ -54,11 +54,11 @@ public class LibPQFactory extends WrappedFactory {
     // Determine the callback handler
     CallbackHandler cbh;
     String sslpasswordcallback = PGProperty.SSL_PASSWORD_CALLBACK.get(info);
     if (sslpasswordcallback != null) {
       try {
-        cbh = (CallbackHandler) ObjectFactory.instantiate(sslpasswordcallback, info, false, null);
+        cbh = ObjectFactory.instantiate(CallbackHandler.class, sslpasswordcallback, info, false, null);
       } catch (Exception e) {
         throw new PSQLException(
           GT.tr("The password callback class provided {0} could not be instantiated.",
             sslpasswordcallback),
           PSQLState.CONNECTION_FAILURE, e);
diff --git a/pgjdbc/src/main/java/org/postgresql/ssl/MakeSSL.java b/pgjdbc/src/main/java/org/postgresql/ssl/MakeSSL.java
index bf64673f4..849d10732 100644
--- a/pgjdbc/src/main/java/org/postgresql/ssl/MakeSSL.java
+++ b/pgjdbc/src/main/java/org/postgresql/ssl/MakeSSL.java
@@ -62,11 +62,11 @@ public class MakeSSL extends ObjectFactory {
     if (sslhostnameverifier == null) {
       hvn = PGjdbcHostnameVerifier.INSTANCE;
       sslhostnameverifier = "PgjdbcHostnameVerifier";
     } else {
       try {
-        hvn = (HostnameVerifier) instantiate(sslhostnameverifier, info, false, null);
+        hvn = instantiate(HostnameVerifier.class, sslhostnameverifier, info, false, null);
       } catch (Exception e) {
         throw new PSQLException(
             GT.tr("The HostnameVerifier class provided {0} could not be instantiated.",
                 sslhostnameverifier),
             PSQLState.CONNECTION_FAILURE, e);
diff --git a/pgjdbc/src/main/java/org/postgresql/util/ObjectFactory.java b/pgjdbc/src/main/java/org/postgresql/util/ObjectFactory.java
index ef24770e6..e4fec28fc 100644
--- a/pgjdbc/src/main/java/org/postgresql/util/ObjectFactory.java
+++ b/pgjdbc/src/main/java/org/postgresql/util/ObjectFactory.java
@@ -34,18 +34,19 @@ public class ObjectFactory {
    * @throws IllegalArgumentException if something goes wrong
    * @throws InstantiationException if something goes wrong
    * @throws IllegalAccessException if something goes wrong
    * @throws InvocationTargetException if something goes wrong
    */
-  public static Object instantiate(String classname, Properties info, boolean tryString,
+  public static <T> T instantiate(Class<T> expectedClass, String classname, Properties info,
+      boolean tryString,
       @Nullable String stringarg)
       throws ClassNotFoundException, SecurityException, NoSuchMethodException,
           IllegalArgumentException, InstantiationException, IllegalAccessException,
           InvocationTargetException {
     @Nullable Object[] args = {info};
-    Constructor<?> ctor = null;
-    Class<?> cls = Class.forName(classname);
+    Constructor<? extends T> ctor = null;
+    Class<? extends T> cls = Class.forName(classname).asSubclass(expectedClass);
     try {
       ctor = cls.getConstructor(Properties.class);
     } catch (NoSuchMethodException ignored) {
     }
     if (tryString && ctor == null) {
diff --git a/pgjdbc/src/test/java/org/postgresql/test/util/ObjectFactoryTest.java b/pgjdbc/src/test/java/org/postgresql/test/util/ObjectFactoryTest.java
new file mode 100644
index 000000000..e0a9d1f4b
--- /dev/null
+++ b/pgjdbc/src/test/java/org/postgresql/test/util/ObjectFactoryTest.java
@@ -0,0 +1,106 @@
+/*
+ * Copyright (c) 2022, PostgreSQL Global Development Group
+ * See the LICENSE file in the project root for more information.
+ */
+
+package org.postgresql.test.util;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+
+import org.postgresql.PGProperty;
+import org.postgresql.jdbc.SslMode;
+import org.postgresql.test.TestUtil;
+import org.postgresql.util.ObjectFactory;
+import org.postgresql.util.PSQLState;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.Assertions;
+import org.opentest4j.MultipleFailuresError;
+
+import java.sql.SQLException;
+import java.util.Properties;
+
+import javax.net.SocketFactory;
+
+public class ObjectFactoryTest {
+  Properties props = new Properties();
+
+  static class BadObject {
+    static boolean wasInstantiated = false;
+
+    BadObject() {
+      wasInstantiated = true;
+      throw new RuntimeException("I should not be instantiated");
+    }
+  }
+
+  private void testInvalidInstantiation(PGProperty prop, PSQLState expectedSqlState) {
+    prop.set(props, BadObject.class.getName());
+
+    BadObject.wasInstantiated = false;
+    SQLException ex = assertThrows(SQLException.class, () -> {
+      TestUtil.openDB(props);
+    });
+
+    try {
+      Assertions.assertAll(
+          () -> assertFalse(BadObject.wasInstantiated, "ObjectFactory should not have "
+              + "instantiated bad object for " + prop),
+          () -> assertEquals(expectedSqlState.getState(), ex.getSQLState(), () -> "#getSQLState()"),
+          () -> {
+            assertThrows(
+                ClassCastException.class,
+                () -> {
+                  throw ex.getCause();
+                },
+                () -> "Wrong class specified for " + prop.name()
+                    + " => ClassCastException is expected in SQLException#getCause()"
+            );
+          }
+      );
+    } catch (MultipleFailuresError e) {
+      // Add the original exception so it is easier to understand the reason for the test to fail
+      e.addSuppressed(ex);
+      throw e;
+    }
+  }
+
+  @Test
+  public void testInvalidSocketFactory() {
+    testInvalidInstantiation(PGProperty.SOCKET_FACTORY, PSQLState.CONNECTION_FAILURE);
+  }
+
+  @Test
+  public void testInvalidSSLFactory() {
+    TestUtil.assumeSslTestsEnabled();
+    // We need at least "require" to trigger SslSockerFactory instantiation
+    PGProperty.SSL_MODE.set(props, SslMode.REQUIRE.value);
+    testInvalidInstantiation(PGProperty.SSL_FACTORY, PSQLState.CONNECTION_FAILURE);
+  }
+
+  @Test
+  public void testInvalidAuthenticationPlugin() {
+    testInvalidInstantiation(PGProperty.AUTHENTICATION_PLUGIN_CLASS_NAME,
+        PSQLState.INVALID_PARAMETER_VALUE);
+  }
+
+  @Test
+  public void testInvalidSslHostnameVerifier() {
+    TestUtil.assumeSslTestsEnabled();
+    // Hostname verification is done at verify-full level only
+    PGProperty.SSL_MODE.set(props, SslMode.VERIFY_FULL.value);
+    PGProperty.SSL_ROOT_CERT.set(props, TestUtil.getSslTestCertPath("goodroot.crt"));
+    testInvalidInstantiation(PGProperty.SSL_HOSTNAME_VERIFIER, PSQLState.CONNECTION_FAILURE);
+  }
+
+  @Test
+  public void testInstantiateInvalidSocketFactory() {
+    Properties props = new Properties();
+    assertThrows(ClassCastException.class, () -> {
+      ObjectFactory.instantiate(SocketFactory.class, BadObject.class.getName(), props,
+          false, null);
+    });
+  }
+}

Vulnerable code:       return (SocketFactory) ObjectFactory.instantiate(socketFactoryClassName, info, true,
      return (SSLSocketFactory) ObjectFactory.instantiate(classname, info, true,
        authPlugin = (AuthenticationPlugin) ObjectFactory.instantiate(authPluginClassName, info,
        LOGGER.log(Level.FINE, "Unable to load Authentication Plugin " + ex.toString());
        throw new PSQLException(ex.getMessage(), PSQLState.UNEXPECTED_ERROR);
            GT.tr("The server requested password-based authentication, but no password was provided."),
        cbh = (CallbackHandler) ObjectFactory.instantiate(sslpasswordcallback, info, false, null);
        hvn = (HostnameVerifier) instantiate(sslhostnameverifier, info, false, null);
  public static Object instantiate(String classname, Properties info, boolean tryString,
    Constructor<?> ctor = null;
    Class<?> cls = Class.forName(classname);
Fixed code:       return ObjectFactory.instantiate(SocketFactory.class, socketFactoryClassName, info, true,
      return ObjectFactory.instantiate(SSLSocketFactory.class, classname, info, true,
        authPlugin = ObjectFactory.instantiate(AuthenticationPlugin.class, authPluginClassName, info,
        String msg = GT.tr("Unable to load Authentication Plugin {0}", authPluginClassName);
        LOGGER.log(Level.FINE, msg, ex);
        throw new PSQLException(msg, PSQLState.INVALID_PARAMETER_VALUE, ex);
            GT.tr("The server requested password-based authentication, but no password was provided by plugin {0}",
                PGProperty.AUTHENTICATION_PLUGIN_CLASS_NAME.get(info)),
        cbh = ObjectFactory.instantiate(CallbackHandler.class, sslpasswordcallback, info, false, null);
        hvn = instantiate(HostnameVerifier.class, sslhostnameverifier, info, false, null);
  public static <T> T instantiate(Class<T> expectedClass, String classname, Properties info,
      boolean tryString,
    Constructor<? extends T> ctor = null;
    Class<? extends T> cls = Class.forName(classname).asSubclass(expectedClass);
/*
 * Copyright (c) 2022, PostgreSQL Global Development Group
 * See the LICENSE file in the project root for more information.
 */

package org.postgresql.test.util;

import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;

import org.postgresql.PGProperty;
import org.postgresql.jdbc.SslMode;
import org.postgresql.test.TestUtil;
import org.postgresql.util.ObjectFactory;
import org.postgresql.util.PSQLState;

import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Assertions;
import org.opentest4j.MultipleFailuresError;

import java.sql.SQLException;
import java.util.Properties;

import javax.net.SocketFactory;

public class ObjectFactoryTest {
  Properties props = new Properties();

  static class BadObject {
    static boolean wasInstantiated = false;

    BadObject() {
      wasInstantiated = true;
      throw new RuntimeException("I should not be instantiated");
    }
  }

  private void testInvalidInstantiation(PGProperty prop, PSQLState expectedSqlState) {
    prop.set(props, BadObject.class.getName());

    BadObject.wasInstantiated = false;
    SQLException ex = assertThrows(SQLException.class, () -> {
      TestUtil.openDB(props);
    });

    try {
      Assertions.assertAll(
          () -> assertFalse(BadObject.wasInstantiated, "ObjectFactory should not have "
              + "instantiated bad object for " + prop),
          () -> assertEquals(expectedSqlState.getState(), ex.getSQLState(), () -> "#getSQLState()"),
          () -> {
            assertThrows(
                ClassCastException.class,
                () -> {
                  throw ex.getCause();
                },
                () -> "Wrong class specified for " + prop.name()
                    + " => ClassCastException is expected in SQLException#getCause()"
            );
          }
      );
    } catch (MultipleFailuresError e) {
      // Add the original exception so it is easier to understand the reason for the test to fail
      e.addSuppressed(ex);
      throw e;
    }
  }

  @Test
  public void testInvalidSocketFactory() {
    testInvalidInstantiation(PGProperty.SOCKET_FACTORY, PSQLState.CONNECTION_FAILURE);
  }

  @Test
  public void testInvalidSSLFactory() {
    TestUtil.assumeSslTestsEnabled();
    // We need at least "require" to trigger SslSockerFactory instantiation
    PGProperty.SSL_MODE.set(props, SslMode.REQUIRE.value);
    testInvalidInstantiation(PGProperty.SSL_FACTORY, PSQLState.CONNECTION_FAILURE);
  }

  @Test
  public void testInvalidAuthenticationPlugin() {
    testInvalidInstantiation(PGProperty.AUTHENTICATION_PLUGIN_CLASS_NAME,
        PSQLState.INVALID_PARAMETER_VALUE);
  }

  @Test
  public void testInvalidSslHostnameVerifier() {
    TestUtil.assumeSslTestsEnabled();
    // Hostname verification is done at verify-full level only
    PGProperty.SSL_MODE.set(props, SslMode.VERIFY_FULL.value);
    PGProperty.SSL_ROOT_CERT.set(props, TestUtil.getSslTestCertPath("goodroot.crt"));
    testInvalidInstantiation(PGProperty.SSL_HOSTNAME_VERIFIER, PSQLState.CONNECTION_FAILURE);
  }

  @Test
  public void testInstantiateInvalidSocketFactory() {
    Properties props = new Properties();
    assertThrows(ClassCastException.class, () -> {
      ObjectFactory.instantiate(SocketFactory.class, BadObject.class.getName(), props,
          false, null);
    });
  }
}