From 7633d987cc11ee2601223e73cfdb8b31fed5980f Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Tue, 4 Mar 2025 17:23:01 +0100 Subject: [PATCH 3/4] Fix GHSA-hrwm-9436-5mv3: pgsql escaping no error checks This adds error checks for escape function is pgsql and pdo_pgsql extensions. It prevents possibility of storing not properly escaped data which could potentially lead to some security issues. (cherry picked from commit 9376aeef9f8ff81f2705b8016237ec3e30bdee44) --- ext/pdo_pgsql/pgsql_driver.c | 10 +- ext/pdo_pgsql/tests/ghsa-hrwm-9436-5mv3.phpt | 22 ++++ ext/pgsql/pgsql.c | 129 +++++++++++++++---- ext/pgsql/tests/ghsa-hrwm-9436-5mv3.phpt | 64 +++++++++ 4 files changed, 202 insertions(+), 23 deletions(-) create mode 100644 ext/pdo_pgsql/tests/ghsa-hrwm-9436-5mv3.phpt create mode 100644 ext/pgsql/tests/ghsa-hrwm-9436-5mv3.phpt diff --git a/ext/pdo_pgsql/pgsql_driver.c b/ext/pdo_pgsql/pgsql_driver.c index c90ef468907..218a306fa3c 100644 --- a/ext/pdo_pgsql/pgsql_driver.c +++ b/ext/pdo_pgsql/pgsql_driver.c @@ -354,11 +354,15 @@ static int pgsql_handle_quoter(pdo_dbh_t *dbh, const char *unquoted, size_t unqu unsigned char *escaped; pdo_pgsql_db_handle *H = (pdo_pgsql_db_handle *)dbh->driver_data; size_t tmp_len; + int err; switch (paramtype) { case PDO_PARAM_LOB: /* escapedlen returned by PQescapeBytea() accounts for trailing 0 */ escaped = PQescapeByteaConn(H->server, (unsigned char *)unquoted, unquotedlen, &tmp_len); + if (escaped == NULL) { + return 0; + } *quotedlen = tmp_len + 1; *quoted = emalloc(*quotedlen + 1); memcpy((*quoted)+1, escaped, *quotedlen-2); @@ -370,7 +374,11 @@ static int pgsql_handle_quoter(pdo_dbh_t *dbh, const char *unquoted, size_t unqu default: *quoted = safe_emalloc(2, unquotedlen, 3); (*quoted)[0] = '\''; - *quotedlen = PQescapeStringConn(H->server, *quoted + 1, unquoted, unquotedlen, NULL); + *quotedlen = PQescapeStringConn(H->server, *quoted + 1, unquoted, unquotedlen, &err); + if (err) { + efree(*quoted); + return 0; + } (*quoted)[*quotedlen + 1] = '\''; (*quoted)[*quotedlen + 2] = '\0'; *quotedlen += 2; diff --git a/ext/pdo_pgsql/tests/ghsa-hrwm-9436-5mv3.phpt b/ext/pdo_pgsql/tests/ghsa-hrwm-9436-5mv3.phpt new file mode 100644 index 00000000000..60e13613d04 --- /dev/null +++ b/ext/pdo_pgsql/tests/ghsa-hrwm-9436-5mv3.phpt @@ -0,0 +1,22 @@ +--TEST-- +#GHSA-hrwm-9436-5mv3: pdo_pgsql extension does not check for errors during escaping +--SKIPIF-- + +--FILE-- +setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION); + +$invalid = "ABC\xff\x30';"; +var_dump($db->quote($invalid)); + +?> +--EXPECT-- +bool(false) diff --git a/ext/pgsql/pgsql.c b/ext/pgsql/pgsql.c index 588f481a498..e9a68a8555f 100644 --- a/ext/pgsql/pgsql.c +++ b/ext/pgsql/pgsql.c @@ -3298,10 +3298,16 @@ PHP_FUNCTION(pg_escape_string) to = zend_string_safe_alloc(ZSTR_LEN(from), 2, 0, 0); if (link) { + int err; if ((pgsql = (PGconn *)zend_fetch_resource2(link, "PostgreSQL link", le_link, le_plink)) == NULL) { RETURN_THROWS(); } - ZSTR_LEN(to) = PQescapeStringConn(pgsql, ZSTR_VAL(to), ZSTR_VAL(from), ZSTR_LEN(from), NULL); + ZSTR_LEN(to) = PQescapeStringConn(pgsql, ZSTR_VAL(to), ZSTR_VAL(from), ZSTR_LEN(from), &err); + if (err) { + zend_argument_value_error(ZEND_NUM_ARGS(), "Escaping string failed"); + zend_string_efree(to); + RETURN_THROWS(); + } } else { ZSTR_LEN(to) = PQescapeString(ZSTR_VAL(to), ZSTR_VAL(from), ZSTR_LEN(from)); @@ -3344,6 +3350,10 @@ PHP_FUNCTION(pg_escape_bytea) to = (char *)PQescapeByteaConn(pgsql, (unsigned char *)from, (size_t)from_len, &to_len); } else to = (char *)PQescapeBytea((unsigned char*)from, from_len, &to_len); + if (to == NULL) { + zend_argument_value_error(ZEND_NUM_ARGS(), "Escape failure"); + RETURN_THROWS(); + } RETVAL_STRINGL(to, to_len-1); /* to_len includes additional '\0' */ PQfreemem(to); @@ -4251,7 +4261,7 @@ PHP_PGSQL_API int php_pgsql_meta_data(PGconn *pg_link, const char *table_name, z char *escaped; smart_str querystr = {0}; size_t new_len; - int i, num_rows; + int i, num_rows, err; zval elem; ZEND_ASSERT(*table_name); @@ -4290,7 +4300,14 @@ PHP_PGSQL_API int php_pgsql_meta_data(PGconn *pg_link, const char *table_name, z "WHERE a.attnum > 0 AND c.relname = '"); } escaped = (char *)safe_emalloc(strlen(tmp_name2), 2, 1); - new_len = PQescapeStringConn(pg_link, escaped, tmp_name2, strlen(tmp_name2), NULL); + new_len = PQescapeStringConn(pg_link, escaped, tmp_name2, strlen(tmp_name2), &err); + if (err) { + php_error_docref(NULL, E_WARNING, "Escaping table name '%s' failed", table_name); + efree(src); + efree(escaped); + smart_str_free(&querystr); + return FAILURE; + } if (new_len) { smart_str_appendl(&querystr, escaped, new_len); } @@ -4298,7 +4315,14 @@ PHP_PGSQL_API int php_pgsql_meta_data(PGconn *pg_link, const char *table_name, z smart_str_appends(&querystr, "' AND n.nspname = '"); escaped = (char *)safe_emalloc(strlen(tmp_name), 2, 1); - new_len = PQescapeStringConn(pg_link, escaped, tmp_name, strlen(tmp_name), NULL); + new_len = PQescapeStringConn(pg_link, escaped, tmp_name, strlen(tmp_name), &err); + if (err) { + php_error_docref(NULL, E_WARNING, "Escaping table namespace '%s' failed", table_name); + efree(src); + efree(escaped); + smart_str_free(&querystr); + return FAILURE; + } if (new_len) { smart_str_appendl(&querystr, escaped, new_len); } @@ -4575,7 +4599,7 @@ PHP_PGSQL_API int php_pgsql_convert(PGconn *pg_link, const char *table_name, con { zend_string *field = NULL; zval meta, *def, *type, *not_null, *has_default, *is_enum, *val, new_val; - int err = 0, skip_field; + int err = 0, escape_err = 0, skip_field; php_pgsql_data_type data_type; ZEND_ASSERT(pg_link != NULL); @@ -4829,10 +4853,14 @@ PHP_PGSQL_API int php_pgsql_convert(PGconn *pg_link, const char *table_name, con /* PostgreSQL ignores \0 */ str = zend_string_alloc(Z_STRLEN_P(val) * 2, 0); /* better to use PGSQLescapeLiteral since PGescapeStringConn does not handle special \ */ - ZSTR_LEN(str) = PQescapeStringConn(pg_link, ZSTR_VAL(str), Z_STRVAL_P(val), Z_STRLEN_P(val), NULL); - str = zend_string_truncate(str, ZSTR_LEN(str), 0); - ZVAL_NEW_STR(&new_val, str); - php_pgsql_add_quotes(&new_val, 1); + ZSTR_LEN(str) = PQescapeStringConn(pg_link, ZSTR_VAL(str), Z_STRVAL_P(val), Z_STRLEN_P(val), &escape_err); + if (escape_err) { + err = 1; + } else { + str = zend_string_truncate(str, ZSTR_LEN(str), 0); + ZVAL_NEW_STR(&new_val, str); + php_pgsql_add_quotes(&new_val, 1); + } } break; @@ -4854,7 +4882,15 @@ PHP_PGSQL_API int php_pgsql_convert(PGconn *pg_link, const char *table_name, con } PGSQL_CONV_CHECK_IGNORE(); if (err) { - php_error_docref(NULL, E_NOTICE, "Expects NULL, string, long or double value for PostgreSQL '%s' (%s)", Z_STRVAL_P(type), ZSTR_VAL(field)); + if (escape_err) { + php_error_docref(NULL, E_NOTICE, + "String value escaping failed for PostgreSQL '%s' (%s)", + Z_STRVAL_P(type), ZSTR_VAL(field)); + } else { + php_error_docref(NULL, E_NOTICE, + "Expects NULL, string, long or double value for PostgreSQL '%s' (%s)", + Z_STRVAL_P(type), ZSTR_VAL(field)); + } } break; @@ -5129,6 +5165,11 @@ PHP_PGSQL_API int php_pgsql_convert(PGconn *pg_link, const char *table_name, con size_t to_len; smart_str s = {0}; tmp = PQescapeByteaConn(pg_link, (unsigned char *)Z_STRVAL_P(val), Z_STRLEN_P(val), &to_len); + if (tmp == NULL) { + php_error_docref(NULL, E_NOTICE, "Escaping value failed for %s field (%s)", Z_STRVAL_P(type), ZSTR_VAL(field)); + err = 1; + break; + } ZVAL_STRINGL(&new_val, (char *)tmp, to_len - 1); /* PQescapeBytea's to_len includes additional '\0' */ PQfreemem(tmp); php_pgsql_add_quotes(&new_val, 1); @@ -5210,6 +5251,12 @@ PHP_PGSQL_API int php_pgsql_convert(PGconn *pg_link, const char *table_name, con zend_hash_update(Z_ARRVAL_P(result), field, &new_val); } else { char *escaped = PQescapeIdentifier(pg_link, ZSTR_VAL(field), ZSTR_LEN(field)); + if (escaped == NULL) { + /* This cannot fail because of invalid string but only due to failed memory allocation */ + php_error_docref(NULL, E_NOTICE, "Escaping field '%s' failed", ZSTR_VAL(field)); + err = 1; + break; + } add_assoc_zval(result, escaped, &new_val); PQfreemem(escaped); } @@ -5290,7 +5337,7 @@ static int do_exec(smart_str *querystr, ExecStatusType expect, PGconn *pg_link, } /* }}} */ -static inline void build_tablename(smart_str *querystr, PGconn *pg_link, const char *table) /* {{{ */ +static inline zend_result build_tablename(smart_str *querystr, PGconn *pg_link, const char *table) /* {{{ */ { size_t table_len = strlen(table); @@ -5301,6 +5348,10 @@ static inline void build_tablename(smart_str *querystr, PGconn *pg_link, const c smart_str_appendl(querystr, table, len); } else { char *escaped = PQescapeIdentifier(pg_link, table, len); + if (escaped == NULL) { + php_error_docref(NULL, E_NOTICE, "Failed to escape table name '%s'", table); + return FAILURE; + } smart_str_appends(querystr, escaped); PQfreemem(escaped); } @@ -5313,11 +5364,17 @@ static inline void build_tablename(smart_str *querystr, PGconn *pg_link, const c smart_str_appendl(querystr, after_dot, len); } else { char *escaped = PQescapeIdentifier(pg_link, after_dot, len); + if (escaped == NULL) { + php_error_docref(NULL, E_NOTICE, "Failed to escape table name '%s'", table); + return FAILURE; + } smart_str_appendc(querystr, '.'); smart_str_appends(querystr, escaped); PQfreemem(escaped); } } + + return SUCCESS; } /* }}} */ @@ -5338,7 +5395,9 @@ PHP_PGSQL_API int php_pgsql_insert(PGconn *pg_link, const char *table, zval *var ZVAL_UNDEF(&converted); if (zend_hash_num_elements(Z_ARRVAL_P(var_array)) == 0) { smart_str_appends(&querystr, "INSERT INTO "); - build_tablename(&querystr, pg_link, table); + if (build_tablename(&querystr, pg_link, table) == FAILURE) { + goto cleanup; + } smart_str_appends(&querystr, " DEFAULT VALUES"); goto no_values; @@ -5354,7 +5413,9 @@ PHP_PGSQL_API int php_pgsql_insert(PGconn *pg_link, const char *table, zval *var } smart_str_appends(&querystr, "INSERT INTO "); - build_tablename(&querystr, pg_link, table); + if (build_tablename(&querystr, pg_link, table) == FAILURE) { + goto cleanup; + } smart_str_appends(&querystr, " ("); ZEND_HASH_FOREACH_STR_KEY(Z_ARRVAL_P(var_array), fld) { @@ -5364,6 +5425,10 @@ PHP_PGSQL_API int php_pgsql_insert(PGconn *pg_link, const char *table, zval *var } if (opt & PGSQL_DML_ESCAPE) { tmp = PQescapeIdentifier(pg_link, ZSTR_VAL(fld), ZSTR_LEN(fld) + 1); + if (tmp == NULL) { + php_error_docref(NULL, E_NOTICE, "Failed to escape field '%s'", ZSTR_VAL(fld)); + goto cleanup; + } smart_str_appends(&querystr, tmp); PQfreemem(tmp); } else { @@ -5375,15 +5440,19 @@ PHP_PGSQL_API int php_pgsql_insert(PGconn *pg_link, const char *table, zval *var smart_str_appends(&querystr, ") VALUES ("); /* make values string */ - ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(var_array), val) { + ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(var_array), fld, val) { /* we can avoid the key_type check here, because we tested it in the other loop */ switch (Z_TYPE_P(val)) { case IS_STRING: if (opt & PGSQL_DML_ESCAPE) { - size_t new_len; - char *tmp; - tmp = (char *)safe_emalloc(Z_STRLEN_P(val), 2, 1); - new_len = PQescapeStringConn(pg_link, tmp, Z_STRVAL_P(val), Z_STRLEN_P(val), NULL); + int error; + char *tmp = safe_emalloc(Z_STRLEN_P(val), 2, 1); + size_t new_len = PQescapeStringConn(pg_link, tmp, Z_STRVAL_P(val), Z_STRLEN_P(val), &error); + if (error) { + php_error_docref(NULL, E_NOTICE, "Failed to escape field '%s' value", ZSTR_VAL(fld)); + efree(tmp); + goto cleanup; + } smart_str_appendc(&querystr, '\''); smart_str_appendl(&querystr, tmp, new_len); smart_str_appendc(&querystr, '\''); @@ -5537,6 +5606,10 @@ static inline int build_assignment_string(PGconn *pg_link, smart_str *querystr, } if (opt & PGSQL_DML_ESCAPE) { char *tmp = PQescapeIdentifier(pg_link, ZSTR_VAL(fld), ZSTR_LEN(fld) + 1); + if (tmp == NULL) { + php_error_docref(NULL, E_NOTICE, "Failed to escape field '%s'", ZSTR_VAL(fld)); + return -1; + } smart_str_appends(querystr, tmp); PQfreemem(tmp); } else { @@ -5551,8 +5624,14 @@ static inline int build_assignment_string(PGconn *pg_link, smart_str *querystr, switch (Z_TYPE_P(val)) { case IS_STRING: if (opt & PGSQL_DML_ESCAPE) { + int error; char *tmp = (char *)safe_emalloc(Z_STRLEN_P(val), 2, 1); - size_t new_len = PQescapeStringConn(pg_link, tmp, Z_STRVAL_P(val), Z_STRLEN_P(val), NULL); + size_t new_len = PQescapeStringConn(pg_link, tmp, Z_STRVAL_P(val), Z_STRLEN_P(val), &error); + if (error) { + php_error_docref(NULL, E_NOTICE, "Failed to escape field '%s' value", ZSTR_VAL(fld)); + efree(tmp); + return -1; + } smart_str_appendc(querystr, '\''); smart_str_appendl(querystr, tmp, new_len); smart_str_appendc(querystr, '\''); @@ -5620,7 +5699,9 @@ PHP_PGSQL_API int php_pgsql_update(PGconn *pg_link, const char *table, zval *var } smart_str_appends(&querystr, "UPDATE "); - build_tablename(&querystr, pg_link, table); + if (build_tablename(&querystr, pg_link, table) == FAILURE) { + goto cleanup; + } smart_str_appends(&querystr, " SET "); if (build_assignment_string(pg_link, &querystr, Z_ARRVAL_P(var_array), 0, ",", 1, opt)) @@ -5722,7 +5803,9 @@ PHP_PGSQL_API int php_pgsql_delete(PGconn *pg_link, const char *table, zval *ids } smart_str_appends(&querystr, "DELETE FROM "); - build_tablename(&querystr, pg_link, table); + if (build_tablename(&querystr, pg_link, table) == FAILURE) { + goto cleanup; + } smart_str_appends(&querystr, " WHERE "); if (build_assignment_string(pg_link, &querystr, Z_ARRVAL_P(ids_array), 1, " AND ", sizeof(" AND ")-1, opt)) @@ -5860,7 +5943,9 @@ PHP_PGSQL_API void php_pgsql_result2array(PGresult *pg_result, zval *ret_array, } smart_str_appends(&querystr, "SELECT * FROM "); - build_tablename(&querystr, pg_link, table); + if (build_tablename(&querystr, pg_link, table) == FAILURE) { + goto cleanup; + } smart_str_appends(&querystr, " WHERE "); if (build_assignment_string(pg_link, &querystr, Z_ARRVAL_P(ids_array), 1, " AND ", sizeof(" AND ")-1, opt)) diff --git a/ext/pgsql/tests/ghsa-hrwm-9436-5mv3.phpt b/ext/pgsql/tests/ghsa-hrwm-9436-5mv3.phpt new file mode 100644 index 00000000000..c1c5e05dce6 --- /dev/null +++ b/ext/pgsql/tests/ghsa-hrwm-9436-5mv3.phpt @@ -0,0 +1,64 @@ +--TEST-- +#GHSA-hrwm-9436-5mv3: pgsql extension does not check for errors during escaping +--EXTENSIONS-- +pgsql +--SKIPIF-- + +--FILE-- + 'test'])); // table name str escape in php_pgsql_meta_data +var_dump(pg_insert($db, "$invalid.tbl", ['bar' => 'test'])); // schema name str escape in php_pgsql_meta_data +var_dump(pg_insert($db, 'ghsa_hrmw_9436_5mv3', ['bar' => $invalid])); // converted value str escape in php_pgsql_convert +var_dump(pg_insert($db, $invalid, [])); // ident escape in build_tablename +var_dump(pg_insert($db, 'ghsa_hrmw_9436_5mv3', [$invalid => 'foo'], $flags)); // ident escape for field php_pgsql_insert +var_dump(pg_insert($db, 'ghsa_hrmw_9436_5mv3', ['bar' => $invalid], $flags)); // str escape for field value in php_pgsql_insert +var_dump(pg_update($db, 'ghsa_hrmw_9436_5mv3', ['bar' => 'val'], [$invalid => 'test'], $flags)); // ident escape in build_assignment_string +var_dump(pg_update($db, 'ghsa_hrmw_9436_5mv3', ['bar' => 'val'], ['bar' => $invalid], $flags)); // invalid str escape in build_assignment_string +var_dump(pg_escape_literal($db, $invalid)); // pg_escape_literal escape +var_dump(pg_escape_identifier($db, $invalid)); // pg_escape_identifier escape + +?> +--EXPECTF-- + +Warning: pg_insert(): Escaping table name 'ABC%s';' failed in %s on line %d +bool(false) + +Warning: pg_insert(): Escaping table namespace 'ABC%s';.tbl' failed in %s on line %d +bool(false) + +Notice: pg_insert(): String value escaping failed for PostgreSQL 'text' (bar) in %s on line %d +bool(false) + +Notice: pg_insert(): Failed to escape table name 'ABC%s';' in %s on line %d +bool(false) + +Notice: pg_insert(): Failed to escape field 'ABC%s';' in %s on line %d +bool(false) + +Notice: pg_insert(): Failed to escape field 'bar' value in %s on line %d +bool(false) + +Notice: pg_update(): Failed to escape field 'ABC%s';' in %s on line %d +bool(false) + +Notice: pg_update(): Failed to escape field 'bar' value in %s on line %d +bool(false) + +Warning: pg_escape_literal(): Failed to escape in %s on line %d +bool(false) + +Warning: pg_escape_identifier(): Failed to escape in %s on line %d +bool(false) -- 2.50.0 From 970548b94b7f23be32154d05a9545b10c98bfd62 Mon Sep 17 00:00:00 2001 From: Remi Collet Date: Thu, 3 Jul 2025 09:32:25 +0200 Subject: [PATCH 4/4] NEWS --- NEWS | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/NEWS b/NEWS index 7db6f2660d2..c813f4f357a 100644 --- a/NEWS +++ b/NEWS @@ -1,6 +1,20 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| +Backported from 8.1.33 + +- PGSQL: + . Fixed GHSA-hrwm-9436-5mv3 (pgsql extension does not check for errors during + escaping). (CVE-2025-1735) (Jakub Zelenka) + +- SOAP: + . Fixed GHSA-453j-q27h-5p8x (NULL Pointer Dereference in PHP SOAP Extension + via Large XML Namespace Prefix). (CVE-2025-6491) (Lekssays, nielsdos) + +- Standard: + . Fixed GHSA-3cr5-j632-f35r (Null byte termination in hostnames). + (CVE-2025-1220) (Jakub Zelenka) + Backported from 8.1.32 - LibXML: -- 2.50.0