diff --git a/CHANGELOG.md b/CHANGELOG.md index 7444e896b3..137f444b3c 100644 --- 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 index 8263f5a1fc..46dbfd180d 100644 --- 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 new file mode 100644 index 0000000000..90424d1b8b --- /dev/null +++ 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 + + + +**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 index 4ae093a8f7..c1eb81ffb0 100644 --- 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 index d7851d4271..b683e68c37 100644 --- 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."), - /** - *
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
- */ - 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 index 6ef64db941..0e10796863 100644 --- 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 index 4a75fdc736..8fc68cd926 100644 --- 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 index db466c864a..a4105461c4 100644 --- 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 index 30541a8eae..e4cea2af1d 100644 --- 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 index 3588f464c9..590670a9fe 100644 --- 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'"; diff --git a/pgjdbc/src/main/java/org/postgresql/jdbc/PgPreparedStatement.java b/pgjdbc/src/main/java/org/postgresql/jdbc/PgPreparedStatement.java index 92e261d815..d6cb55aef1 100644 --- a/pgjdbc/src/main/java/org/postgresql/jdbc/PgPreparedStatement.java +++ b/pgjdbc/src/main/java/org/postgresql/jdbc/PgPreparedStatement.java @@ -130,13 +130,11 @@ public ResultSet executeQuery(String sql) throws SQLException { */ @Override public ResultSet executeQuery() throws SQLException { - synchronized (this) { - if (!executeWithFlags(0)) { - throw new PSQLException(GT.tr("No results were returned by the query."), PSQLState.NO_DATA); - } - - return getSingleResultSet(); + if (!executeWithFlags(0)) { + throw new PSQLException(GT.tr("No results were returned by the query."), PSQLState.NO_DATA); } + + return getSingleResultSet(); } @Override @@ -148,20 +146,16 @@ public int executeUpdate(String sql) throws SQLException { @Override public int executeUpdate() throws SQLException { - synchronized (this) { - executeWithFlags(QueryExecutor.QUERY_NO_RESULTS); - checkNoResultUpdate(); - return getUpdateCount(); - } + executeWithFlags(QueryExecutor.QUERY_NO_RESULTS); + checkNoResultUpdate(); + return getUpdateCount(); } @Override public long executeLargeUpdate() throws SQLException { - synchronized (this) { - executeWithFlags(QueryExecutor.QUERY_NO_RESULTS); - checkNoResultUpdate(); - return getLargeUpdateCount(); - } + executeWithFlags(QueryExecutor.QUERY_NO_RESULTS); + checkNoResultUpdate(); + return getLargeUpdateCount(); } @Override @@ -173,22 +167,20 @@ public boolean execute(String sql) throws SQLException { @Override public boolean execute() throws SQLException { - synchronized (this) { - return executeWithFlags(0); - } + return executeWithFlags(0); } public boolean executeWithFlags(int flags) throws SQLException { try { - synchronized (this) { - checkClosed(); + checkClosed(); - if (connection.getPreferQueryMode() == PreferQueryMode.SIMPLE) { - flags |= QueryExecutor.QUERY_EXECUTE_AS_SIMPLE; - } + if (connection.getPreferQueryMode() == PreferQueryMode.SIMPLE) { + flags |= QueryExecutor.QUERY_EXECUTE_AS_SIMPLE; + } - execute(preparedQuery, preparedParameters, flags); + execute(preparedQuery, preparedParameters, flags); + synchronized (this) { checkClosed(); return (result != null && result.getResultSet() != null); } diff --git a/pgjdbc/src/main/java/org/postgresql/jdbc/PgStatement.java b/pgjdbc/src/main/java/org/postgresql/jdbc/PgStatement.java index 19f071f77e..438ab66c89 100644 --- a/pgjdbc/src/main/java/org/postgresql/jdbc/PgStatement.java +++ b/pgjdbc/src/main/java/org/postgresql/jdbc/PgStatement.java @@ -240,13 +240,11 @@ public void handleWarning(SQLWarning warning) { @Override public ResultSet executeQuery(String sql) throws SQLException { - synchronized (this) { - if (!executeWithFlags(sql, 0)) { - throw new PSQLException(GT.tr("No results were returned by the query."), PSQLState.NO_DATA); - } - - return getSingleResultSet(); + if (!executeWithFlags(sql, 0)) { + throw new PSQLException(GT.tr("No results were returned by the query."), PSQLState.NO_DATA); } + + return getSingleResultSet(); } protected ResultSet getSingleResultSet() throws SQLException { @@ -264,11 +262,9 @@ protected ResultSet getSingleResultSet() throws SQLException { @Override public int executeUpdate(String sql) throws SQLException { - synchronized (this) { - executeWithFlags(sql, QueryExecutor.QUERY_NO_RESULTS); - checkNoResultUpdate(); - return getUpdateCount(); - } + executeWithFlags(sql, QueryExecutor.QUERY_NO_RESULTS); + checkNoResultUpdate(); + return getUpdateCount(); } protected final void checkNoResultUpdate() throws SQLException { @@ -408,19 +404,17 @@ protected boolean isOneShotQuery(@Nullable CachedQuery cachedQuery) { protected final void execute(CachedQuery cachedQuery, @Nullable ParameterList queryParameters, int flags) throws SQLException { - synchronized (this) { - try { - executeInternal(cachedQuery, queryParameters, flags); - } catch (SQLException e) { - // Don't retry composite queries as it might get partially executed - if (cachedQuery.query.getSubqueries() != null - || !connection.getQueryExecutor().willHealOnRetry(e)) { - throw e; - } - cachedQuery.query.close(); - // Execute the query one more time - executeInternal(cachedQuery, queryParameters, flags); + try { + executeInternal(cachedQuery, queryParameters, flags); + } catch (SQLException e) { + // Don't retry composite queries as it might get partially executed + if (cachedQuery.query.getSubqueries() != null + || !connection.getQueryExecutor().willHealOnRetry(e)) { + throw e; } + cachedQuery.query.close(); + // Execute the query one more time + executeInternal(cachedQuery, queryParameters, flags); } } @@ -520,10 +514,7 @@ public void setCursorName(String name) throws SQLException { // No-op. } - private volatile int isClosed = 0; - private static final AtomicIntegerFieldUpdater