Prepare for release (#2580)

* Revert "fix: replace syncronization in Connection.close with compareAndSet" * Revert "feat: synchronize statement executions (e.g. avoid deadlock when Connection.isValid is executed from concurrent threads)" * Revert "set a timeout to get the return from requesting SSL upgrade. (#2572)" * Revert "add alias to the generated query for clarity (#2553)"
pgjdbc · Aug 3, 2022 · bd91c4c · bd91c4c
1 parent 739e599
commit bd91c4c
Show file tree

Hide file tree

Showing 13 changed files with 197 additions and 260 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,15 +5,29 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 
 ## [Unreleased]
 ### Changed
-chore: skip publishing pgjdbc-osgi-test to Central
-chore: bump Gradle to 7.5
-test: update JUnit to 5.8.2
 
 ### Added
-chore: added Gradle Wrapper Validation for verifying gradle-wrapper.jar
-chore: added "permissions: contents: read" for GitHub Actions to avoid unintentional modifications by the CI
-chore: support building pgjdbc with Java 17
-feat: synchronize statement executions (e.g. avoid deadlock when Connection.isValid is executed from concurrent threads)
+
+### Fixed
+
+[42.4.1] (2022-08-01 16:24:20 -0400)
+### Security
+- fix: CVE-2022-31197 Fixes SQL generated in PgResultSet.refresh() to escape column identifiers so as to prevent SQL injection.
+  - Previously, the column names for both key and data columns in the table were copied as-is into the generated
+  SQL. This allowed a malicious table with column names that include statement terminator to be parsed and
+  executed as multiple separate commands.
+  - Also adds a new test class ResultSetRefreshTest to verify this change.
+  - Reported by [Sho Kato](https://github.com/kato-sho)
+
+### Changed
+- chore: skip publishing pgjdbc-osgi-test to Central
+- chore: bump Gradle to 7.5
+- test: update JUnit to 5.8.2
+
+### Added
+- chore: added Gradle Wrapper Validation for verifying gradle-wrapper.jar
+- chore: added "permissions: contents: read" for GitHub Actions to avoid unintentional modifications by the CI
+- chore: support building pgjdbc with Java 17
 
 ### Fixed
 
@@ -709,4 +723,5 @@ thrown to caller to be dealt with so no need to log at this verbosity by pgjdbc
 [42.3.4]: https://github.com/pgjdbc/pgjdbc/compare/REL42.3.4...REL42.3.5
 [42.3.5]: https://github.com/pgjdbc/pgjdbc/compare/REL42.3.5...REL42.3.6
 [42.3.6]: https://github.com/pgjdbc/pgjdbc/compare/REL42.3.6...REL42.4.0
-[Unreleased]: https://github.com/pgjdbc/pgjdbc/compare/REL42.4.0...HEAD
+[42.4.0]: https://github.com/pgjdbc/pgjdbc/compare/REL42.4.0...REL42.4.1
+[Unreleased]: https://github.com/pgjdbc/pgjdbc/compare/REL42.4.1...HEAD
diff --git a/README.md b/README.md
@@ -118,7 +118,6 @@ In addition to the standard connection parameters the driver supports a number o
 | loginTimeout                  | Integer | 0       | Specify how long to wait for establishment of a database connection.|
 | connectTimeout                | Integer | 10      | The timeout value used for socket connect operations. |
 | socketTimeout                 | Integer | 0       | The timeout value used for socket read operations. |
-| sslResponseTimeout            | Integer | 5000    | Socket timeout waiting for a response from a request for SSL upgrade from the server. |                                                                                                                                                                        
 | tcpKeepAlive                  | Boolean | false   | Enable or disable TCP keep-alive. |
 | tcpNoDelay                    | Boolean | true    | Enable or disable TCP no delay. |
 | ApplicationName               | String  | PostgreSQL JDBC Driver    | The application name (require server version >= 9.0). If assumeMinServerVersion is set to >= 9.0 this will be sent in the startup packets, otherwise after the connection is made |

diff --git a/docs/_posts/2022-08-03-42.4.1-release.md b/docs/_posts/2022-08-03-42.4.1-release.md
@@ -0,0 +1,44 @@
+---
+title:  PostgreSQL JDBC Driver 42.4.1 Released
+date:   2022-08-03 08:09:07 -0400
+categories:
+    - new_release
+version: 42.4.1
+---
+**Notable changes**
+
+### Security
+- fix: CVE-2022-31197 Fixes SQL generated in PgResultSet.refresh() to escape column identifiers so as to prevent SQL injection.
+  - Previously, the column names for both key and data columns in the table were copied as-is into the generated
+  SQL. This allowed a malicious table with column names that include statement terminator to be parsed and
+  executed as multiple separate commands.
+  - Also adds a new test class ResultSetRefreshTest to verify this change.
+  - Reported by [Sho Kato](https://github.com/kato-sho)
+
+### Changed
+- chore: skip publishing pgjdbc-osgi-test to Central
+- chore: bump Gradle to 7.5
+- test: update JUnit to 5.8.2
+
+### Added
+- chore: added Gradle Wrapper Validation for verifying gradle-wrapper.jar
+- chore: added "permissions: contents: read" for GitHub Actions to avoid unintentional modifications by the CI
+- chore: support building pgjdbc with Java 17
+
+<!--more-->
+
+**Commits by author**
+
+Dave Cramer (9):
+      bump gradle to version 3 to fix compile errors with jdk17 [PR 2550](https://github.com/pgjdbc/pgjdbc/pull/2550)
+      update the website content [PR 2578](https://github.com/pgjdbc/pgjdbc/pull/2578)
+
+Sehrope Sarkuni (1):
+    Fix SQL generated in PgResultSet.refresh() to escape column identifiers so as to prevent SQL injection.
+
+Vladimir Sitnikov (34):
+    bump system-stubs-jupiter to 2.0.1 to support Java 16+
+    update JUnit to 5.8.2
+    migrate DriverTest to JUnit5
+    bump Gradle to 7.5
+
diff --git a/docs/documentation/head/connect.md b/docs/documentation/head/connect.md
@@ -327,10 +327,6 @@ Connection conn = DriverManager.getConnection(url);
 	detecting network problems. The timeout is specified in seconds and a
 	value of zero means that it is disabled.
 
-* **sslResponseTimeout** = int
-    The timeout value in milliseconds that we wait for a response from the server
-    after requesting the connection be upgraded to SSL.
-
 * **cancelSignalTimeout** = int
 
 	Cancel command is sent out of band over its own connection, so cancel message can itself get

diff --git a/pgjdbc/src/main/java/org/postgresql/PGProperty.java b/pgjdbc/src/main/java/org/postgresql/PGProperty.java
@@ -672,15 +672,6 @@ public enum PGProperty {
       null,
       "A class, implementing javax.security.auth.callback.CallbackHandler that can handle PassworCallback for the ssl password."),
 
-  /**
-   * <p>After requesting an upgrade to SSL from the server there are reports of the server not responding due to a failover
-   * without a timeout here, the client can wait forever. This timeout will be set before the request and reset after </p>
-   */
-  SSL_RESPONSE_TIMEOUT(
-      "sslResponseTimeout",
-      "5000",
-      "Time in milliseconds we wait for a response from the server after requesting SSL upgrade"),
-
   /**
    * File containing the root certificate when validating server ({@code sslmode} = {@code
    * verify-ca} or {@code verify-full}). Default will be the file {@code root.crt} in {@code

diff --git a/pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java b/pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java
@@ -532,17 +532,6 @@ private PGStream enableSSL(PGStream pgStream, SslMode sslMode, Properties info,
 
     LOGGER.log(Level.FINEST, " FE=> SSLRequest");
 
-    int sslTimeout = PGProperty.SSL_RESPONSE_TIMEOUT.getInt(info);
-    int currentTimeout = pgStream.getSocket().getSoTimeout();
-
-    // if the current timeout is less than sslTimeout then
-    // use the smaller timeout. We could do something tricky
-    // here to not set it in that case but this is pretty readable
-    if (currentTimeout > 0 && currentTimeout < sslTimeout) {
-      sslTimeout = currentTimeout;
-    }
-
-    pgStream.getSocket().setSoTimeout(sslTimeout);
     // Send SSL request packet
     pgStream.sendInteger4(8);
     pgStream.sendInteger2(1234);
@@ -551,8 +540,6 @@ private PGStream enableSSL(PGStream pgStream, SslMode sslMode, Properties info,
 
     // Now get the response from the backend, one of N, E, S.
     int beresp = pgStream.receiveChar();
-    pgStream.getSocket().setSoTimeout(currentTimeout);
-
     switch (beresp) {
       case 'E':
         LOGGER.log(Level.FINEST, " <=BE SSLError");

diff --git a/pgjdbc/src/main/java/org/postgresql/ds/common/BaseDataSource.java b/pgjdbc/src/main/java/org/postgresql/ds/common/BaseDataSource.java
@@ -354,24 +354,6 @@ public void setConnectTimeout(int connectTimeout) {
     PGProperty.CONNECT_TIMEOUT.set(properties, connectTimeout);
   }
 
-  /**
-   *
-   * @return SSL ResponseTimeout
-   * @see PGProperty#SSL_RESPONSE_TIMEOUT
-   */
-  public int getSslResponseTimeout() {
-    return PGProperty.SSL_RESPONSE_TIMEOUT.getIntNoCheck(properties);
-  }
-
-  /**
-   *
-   * @param sslResponseTimeout ssl response timeout
-   * @see PGProperty#SSL_RESPONSE_TIMEOUT
-   */
-  public void setSslResponseTimeout(int sslResponseTimeout) {
-    PGProperty.SSL_RESPONSE_TIMEOUT.set(properties,sslResponseTimeout);
-  }
-
   /**
    * @return protocol version
    * @see PGProperty#PROTOCOL_VERSION

diff --git a/pgjdbc/src/main/java/org/postgresql/jdbc/PgCallableStatement.java b/pgjdbc/src/main/java/org/postgresql/jdbc/PgCallableStatement.java
@@ -80,79 +80,79 @@ public int executeUpdate() throws SQLException {
 
   @Override
   public boolean executeWithFlags(int flags) throws SQLException {
-    synchronized (this) {
-      boolean hasResultSet = super.executeWithFlags(flags);
-      int[] functionReturnType = this.functionReturnType;
-      if (!isFunction || !returnTypeSet || functionReturnType == null) {
-        return hasResultSet;
-      }
+    boolean hasResultSet = super.executeWithFlags(flags);
+    int[] functionReturnType = this.functionReturnType;
+    if (!isFunction || !returnTypeSet || functionReturnType == null) {
+      return hasResultSet;
+    }
 
-      // If we are executing and there are out parameters
-      // callable statement function set the return data
-      if (!hasResultSet) {
-        throw new PSQLException(GT.tr("A CallableStatement was executed with nothing returned."),
-            PSQLState.NO_DATA);
-      }
+    // If we are executing and there are out parameters
+    // callable statement function set the return data
+    if (!hasResultSet) {
+      throw new PSQLException(GT.tr("A CallableStatement was executed with nothing returned."),
+          PSQLState.NO_DATA);
+    }
 
-      ResultSet rs = castNonNull(getResultSet());
-      if (!rs.next()) {
-        throw new PSQLException(GT.tr("A CallableStatement was executed with nothing returned."),
-            PSQLState.NO_DATA);
-      }
+    ResultSet rs = castNonNull(getResultSet());
+    if (!rs.next()) {
+      throw new PSQLException(GT.tr("A CallableStatement was executed with nothing returned."),
+          PSQLState.NO_DATA);
+    }
 
-      // figure out how many columns
-      int cols = rs.getMetaData().getColumnCount();
+    // figure out how many columns
+    int cols = rs.getMetaData().getColumnCount();
 
-      int outParameterCount = preparedParameters.getOutParameterCount();
+    int outParameterCount = preparedParameters.getOutParameterCount();
 
-      if (cols != outParameterCount) {
-        throw new PSQLException(
-            GT.tr("A CallableStatement was executed with an invalid number of parameters"),
-            PSQLState.SYNTAX_ERROR);
+    if (cols != outParameterCount) {
+      throw new PSQLException(
+          GT.tr("A CallableStatement was executed with an invalid number of parameters"),
+          PSQLState.SYNTAX_ERROR);
+    }
+
+    // reset last result fetched (for wasNull)
+    lastIndex = 0;
+
+    // allocate enough space for all possible parameters without regard to in/out
+    @Nullable Object[] callResult = new Object[preparedParameters.getParameterCount() + 1];
+    this.callResult = callResult;
+
+    // move them into the result set
+    for (int i = 0, j = 0; i < cols; i++, j++) {
+      // find the next out parameter, the assumption is that the functionReturnType
+      // array will be initialized with 0 and only out parameters will have values
+      // other than 0. 0 is the value for java.sql.Types.NULL, which should not
+      // conflict
+      while (j < functionReturnType.length && functionReturnType[j] == 0) {
+        j++;
       }
 
-      // reset last result fetched (for wasNull)
-      lastIndex = 0;
-
-      // allocate enough space for all possible parameters without regard to in/out
-      @Nullable Object[] callResult = new Object[preparedParameters.getParameterCount() + 1];
-      this.callResult = callResult;
-
-      // move them into the result set
-      for (int i = 0, j = 0; i < cols; i++, j++) {
-        // find the next out parameter, the assumption is that the functionReturnType
-        // array will be initialized with 0 and only out parameters will have values
-        // other than 0. 0 is the value for java.sql.Types.NULL, which should not
-        // conflict
-        while (j < functionReturnType.length && functionReturnType[j] == 0) {
-          j++;
-        }
+      callResult[j] = rs.getObject(i + 1);
+      int columnType = rs.getMetaData().getColumnType(i + 1);
 
-        callResult[j] = rs.getObject(i + 1);
-        int columnType = rs.getMetaData().getColumnType(i + 1);
-
-        if (columnType != functionReturnType[j]) {
-          // this is here for the sole purpose of passing the cts
-          if (columnType == Types.DOUBLE && functionReturnType[j] == Types.REAL) {
-            // return it as a float
-            Object result = callResult[j];
-            if (result != null) {
-              callResult[j] = ((Double) result).floatValue();
-            }
-          } else if (columnType == Types.REF_CURSOR && functionReturnType[j] == Types.OTHER) {
-            // For backwards compatibility reasons we support that ref cursors can be
-            // registered with both Types.OTHER and Types.REF_CURSOR so we allow
-            // this specific mismatch
-          } else {
-            throw new PSQLException(GT.tr(
-                "A CallableStatement function was executed and the out parameter {0} was of type {1} however type {2} was registered.",
-                i + 1, "java.sql.Types=" + columnType, "java.sql.Types=" + functionReturnType[j]),
-                PSQLState.DATA_TYPE_MISMATCH);
+      if (columnType != functionReturnType[j]) {
+        // this is here for the sole purpose of passing the cts
+        if (columnType == Types.DOUBLE && functionReturnType[j] == Types.REAL) {
+          // return it as a float
+          Object result = callResult[j];
+          if (result != null) {
+            callResult[j] = ((Double) result).floatValue();
           }
+        } else if (columnType == Types.REF_CURSOR && functionReturnType[j] == Types.OTHER) {
+          // For backwards compatibility reasons we support that ref cursors can be
+          // registered with both Types.OTHER and Types.REF_CURSOR so we allow
+          // this specific mismatch
+        } else {
+          throw new PSQLException(GT.tr(
+              "A CallableStatement function was executed and the out parameter {0} was of type {1} however type {2} was registered.",
+              i + 1, "java.sql.Types=" + columnType, "java.sql.Types=" + functionReturnType[j]),
+              PSQLState.DATA_TYPE_MISMATCH);
         }
-
       }
-      rs.close();
+
+    }
+    rs.close();
+    synchronized (this) {
       result = null;
     }
     return false;

diff --git a/pgjdbc/src/main/java/org/postgresql/jdbc/PgConnection.java b/pgjdbc/src/main/java/org/postgresql/jdbc/PgConnection.java
@@ -1454,13 +1454,8 @@ public boolean isValid(int timeout) throws SQLException {
           statement.execute("IDENTIFY_SYSTEM");
           statement.close();
         } else {
-          PreparedStatement checkConnectionQuery;
-          synchronized (this) {
-            checkConnectionQuery = this.checkConnectionQuery;
-            if (checkConnectionQuery == null) {
-              checkConnectionQuery = prepareStatement("");
-              this.checkConnectionQuery = checkConnectionQuery;
-            }
+          if (checkConnectionQuery == null) {
+            checkConnectionQuery = prepareStatement("");
           }
           checkConnectionQuery.executeUpdate();
         }

diff --git a/pgjdbc/src/main/java/org/postgresql/jdbc/PgDatabaseMetaData.java b/pgjdbc/src/main/java/org/postgresql/jdbc/PgDatabaseMetaData.java
@@ -2696,11 +2696,11 @@ public ResultSet getUDTs(@Nullable String catalog, @Nullable String schemaPatter
       long longTypOid = typeInfo.intOidToLong(typOid);
       int sqlType = typeInfo.getSQLType(typOid);
 
-      sqlwhen.append(" when base_type.oid = ").append(longTypOid).append(" then ").append(sqlType);
+      sqlwhen.append(" when oid = ").append(longTypOid).append(" then ").append(sqlType);
     }
     sql += sqlwhen.toString();
 
-    sql += " else " + java.sql.Types.OTHER + " end from pg_type base_type where base_type.oid=t.typbasetype) "
+    sql += " else " + java.sql.Types.OTHER + " end from pg_type where oid=t.typbasetype) "
         + "else null end as base_type "
         + "from pg_catalog.pg_type t, pg_catalog.pg_namespace n where t.typnamespace = n.oid and n.nspname != 'pg_catalog' and n.nspname != 'pg_toast'";