From: Markus Koschany Date: Sun, 3 Jul 2022 16:16:36 +0200 Subject: CVE-2022-21724 Origin: https://github.com/pgjdbc/pgjdbc/commit/f4d0ed69c0b3aae8531d83d6af4c57f22312c813 --- .../org/postgresql/core/SocketFactoryFactory.java | 4 +- .../main/java/org/postgresql/ssl/LibPQFactory.java | 2 +- .../src/main/java/org/postgresql/ssl/MakeSSL.java | 2 +- .../java/org/postgresql/util/ObjectFactory.java | 7 +- .../postgresql/test/util/ObjectFactoryTest.java | 106 +++++++++++++++++++++ 5 files changed, 114 insertions(+), 7 deletions(-) create mode 100644 pgjdbc/src/test/java/org/postgresql/test/util/ObjectFactoryTest.java diff --git a/pgjdbc/src/main/java/org/postgresql/core/SocketFactoryFactory.java b/pgjdbc/src/main/java/org/postgresql/core/SocketFactoryFactory.java index 09efa75..fe56354 100644 --- a/pgjdbc/src/main/java/org/postgresql/core/SocketFactoryFactory.java +++ b/pgjdbc/src/main/java/org/postgresql/core/SocketFactoryFactory.java @@ -36,7 +36,7 @@ public class SocketFactoryFactory { 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( @@ -61,7 +61,7 @@ public class SocketFactoryFactory { 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( diff --git a/pgjdbc/src/main/java/org/postgresql/ssl/LibPQFactory.java b/pgjdbc/src/main/java/org/postgresql/ssl/LibPQFactory.java index c0c34bd..4d4c1aa 100644 --- a/pgjdbc/src/main/java/org/postgresql/ssl/LibPQFactory.java +++ b/pgjdbc/src/main/java/org/postgresql/ssl/LibPQFactory.java @@ -77,7 +77,7 @@ public class LibPQFactory extends WrappedFactory { String sslpasswordcallback = PGProperty.SSL_PASSWORD_CALLBACK.get(info); if (sslpasswordcallback != null) { try { - cbh = (CallbackHandler) ObjectFactory.instantiate(sslpasswordcallback, info, false, null); + cbh = (CallbackHandler) MakeSSL.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.", diff --git a/pgjdbc/src/main/java/org/postgresql/ssl/MakeSSL.java b/pgjdbc/src/main/java/org/postgresql/ssl/MakeSSL.java index e09d88e..4aa20d9 100644 --- a/pgjdbc/src/main/java/org/postgresql/ssl/MakeSSL.java +++ b/pgjdbc/src/main/java/org/postgresql/ssl/MakeSSL.java @@ -63,7 +63,7 @@ public class MakeSSL extends ObjectFactory { 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.", diff --git a/pgjdbc/src/main/java/org/postgresql/util/ObjectFactory.java b/pgjdbc/src/main/java/org/postgresql/util/ObjectFactory.java index 273ac6d..dd37812 100644 --- a/pgjdbc/src/main/java/org/postgresql/util/ObjectFactory.java +++ b/pgjdbc/src/main/java/org/postgresql/util/ObjectFactory.java @@ -34,13 +34,14 @@ public class ObjectFactory { * @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 instantiate(Class expectedClass, String classname, Properties info, + boolean tryString, String stringarg) throws ClassNotFoundException, SecurityException, NoSuchMethodException, IllegalArgumentException, InstantiationException, IllegalAccessException, InvocationTargetException { Object[] args = {info}; - Constructor ctor = null; - Class cls = Class.forName(classname); + Constructor ctor = null; + Class cls = Class.forName(classname).asSubclass(expectedClass); try { ctor = cls.getConstructor(Properties.class); } catch (NoSuchMethodException nsme) { 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 0000000..e0a9d1f --- /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); + }); + } +}