Commit 545d1536d8b for php.net
commit 545d1536d8b2c16ff9c844791d8d9d39530e0313
Author: Jakub Zelenka <bukka@php.net>
Date: Tue Mar 4 17:23:01 2025 +0100
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.
diff --git a/ext/pdo_pgsql/pgsql_driver.c b/ext/pdo_pgsql/pgsql_driver.c
index 46b3f25f408..1cccfd2ab07 100644
--- a/ext/pdo_pgsql/pgsql_driver.c
+++ b/ext/pdo_pgsql/pgsql_driver.c
@@ -353,11 +353,15 @@ static zend_string* pgsql_handle_quoter(pdo_dbh_t *dbh, const zend_string *unquo
zend_string *quoted_str;
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 *)ZSTR_VAL(unquoted), ZSTR_LEN(unquoted), &tmp_len);
+ if (escaped == NULL) {
+ return NULL;
+ }
quotedlen = tmp_len + 1;
quoted = emalloc(quotedlen + 1);
memcpy(quoted+1, escaped, quotedlen-2);
@@ -369,7 +373,11 @@ static zend_string* pgsql_handle_quoter(pdo_dbh_t *dbh, const zend_string *unquo
default:
quoted = safe_emalloc(2, ZSTR_LEN(unquoted), 3);
quoted[0] = '\'';
- quotedlen = PQescapeStringConn(H->server, quoted + 1, ZSTR_VAL(unquoted), ZSTR_LEN(unquoted), NULL);
+ quotedlen = PQescapeStringConn(H->server, quoted + 1, ZSTR_VAL(unquoted), ZSTR_LEN(unquoted), &err);
+ if (err) {
+ efree(quoted);
+ return NULL;
+ }
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..8566a26753b
--- /dev/null
+++ b/ext/pdo_pgsql/tests/ghsa-hrwm-9436-5mv3.phpt
@@ -0,0 +1,24 @@
+--TEST--
+#GHSA-hrwm-9436-5mv3: pdo_pgsql extension does not check for errors during escaping
+--EXTENSIONS--
+pdo
+pdo_pgsql
+--SKIPIF--
+<?php
+require_once dirname(__FILE__) . '/../../../ext/pdo/tests/pdo_test.inc';
+require_once dirname(__FILE__) . '/config.inc';
+PDOTest::skip();
+?>
+--FILE--
+<?php
+require_once dirname(__FILE__) . '/../../../ext/pdo/tests/pdo_test.inc';
+require_once dirname(__FILE__) . '/config.inc';
+$db = PDOTest::test_factory(dirname(__FILE__) . '/common.phpt');
+$db->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 63acd26ea01..11ce814cbec 100644
--- a/ext/pgsql/pgsql.c
+++ b/ext/pgsql/pgsql.c
@@ -3269,8 +3269,14 @@ PHP_FUNCTION(pg_escape_string)
to = zend_string_safe_alloc(ZSTR_LEN(from), 2, 0, 0);
if (link) {
+ int err;
pgsql = link->conn;
- 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));
@@ -3313,6 +3319,10 @@ PHP_FUNCTION(pg_escape_bytea)
} else {
to = (char *)PQescapeBytea((unsigned char *)ZSTR_VAL(from), ZSTR_LEN(from), &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);
@@ -4245,7 +4255,7 @@ PHP_PGSQL_API zend_result php_pgsql_meta_data(PGconn *pg_link, const zend_string
char *escaped;
smart_str querystr = {0};
size_t new_len;
- int i, num_rows;
+ int i, num_rows, err;
zval elem;
ZEND_ASSERT(ZSTR_LEN(table_name) != 0);
@@ -4283,7 +4293,14 @@ PHP_PGSQL_API zend_result php_pgsql_meta_data(PGconn *pg_link, const zend_string
"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", ZSTR_VAL(table_name));
+ efree(src);
+ efree(escaped);
+ smart_str_free(&querystr);
+ return FAILURE;
+ }
if (new_len) {
smart_str_appendl(&querystr, escaped, new_len);
}
@@ -4291,7 +4308,14 @@ PHP_PGSQL_API zend_result php_pgsql_meta_data(PGconn *pg_link, const zend_string
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", ZSTR_VAL(table_name));
+ efree(src);
+ efree(escaped);
+ smart_str_free(&querystr);
+ return FAILURE;
+ }
if (new_len) {
smart_str_appendl(&querystr, escaped, new_len);
}
@@ -4552,7 +4576,7 @@ PHP_PGSQL_API zend_result php_pgsql_convert(PGconn *pg_link, const zend_string *
{
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);
@@ -4804,8 +4828,13 @@ PHP_PGSQL_API zend_result php_pgsql_convert(PGconn *pg_link, const zend_string *
/* 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);
- ZVAL_STR(&new_val, php_pgsql_add_quotes(str));
+ 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 {
+ ZVAL_STR(&new_val, php_pgsql_add_quotes(str));
+ }
zend_string_release_ex(str, false);
}
break;
@@ -4828,7 +4857,14 @@ PHP_PGSQL_API zend_result php_pgsql_convert(PGconn *pg_link, const zend_string *
}
PGSQL_CONV_CHECK_IGNORE();
if (err) {
- zend_type_error("%s(): Field \"%s\" must be of type string|null, %s given", get_active_function_name(), ZSTR_VAL(field), Z_STRVAL_P(type));
+ 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 {
+ zend_type_error("%s(): Field \"%s\" must be of type string|null, %s given",
+ get_active_function_name(), ZSTR_VAL(field), Z_STRVAL_P(type));
+ }
}
break;
@@ -5103,6 +5139,11 @@ PHP_PGSQL_API zend_result php_pgsql_convert(PGconn *pg_link, const zend_string *
zend_string *tmp_zstr;
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;
+ }
tmp_zstr = zend_string_init((char *)tmp, to_len - 1, false); /* PQescapeBytea's to_len includes additional '\0' */
PQfreemem(tmp);
@@ -5181,6 +5222,12 @@ PHP_PGSQL_API zend_result php_pgsql_convert(PGconn *pg_link, const zend_string *
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);
}
@@ -5259,7 +5306,7 @@ static bool do_exec(smart_str *querystr, ExecStatusType expect, PGconn *pg_link,
}
/* }}} */
-static inline void build_tablename(smart_str *querystr, PGconn *pg_link, const zend_string *table) /* {{{ */
+static inline zend_result build_tablename(smart_str *querystr, PGconn *pg_link, const zend_string *table) /* {{{ */
{
/* schema.table should be "schema"."table" */
const char *dot = memchr(ZSTR_VAL(table), '.', ZSTR_LEN(table));
@@ -5269,6 +5316,10 @@ static inline void build_tablename(smart_str *querystr, PGconn *pg_link, const z
smart_str_appendl(querystr, ZSTR_VAL(table), len);
} else {
char *escaped = PQescapeIdentifier(pg_link, ZSTR_VAL(table), len);
+ if (escaped == NULL) {
+ php_error_docref(NULL, E_NOTICE, "Failed to escape table name '%s'", ZSTR_VAL(table));
+ return FAILURE;
+ }
smart_str_appends(querystr, escaped);
PQfreemem(escaped);
}
@@ -5281,11 +5332,17 @@ static inline void build_tablename(smart_str *querystr, PGconn *pg_link, const z
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'", ZSTR_VAL(table));
+ return FAILURE;
+ }
smart_str_appendc(querystr, '.');
smart_str_appends(querystr, escaped);
PQfreemem(escaped);
}
}
+
+ return SUCCESS;
}
/* }}} */
@@ -5306,7 +5363,9 @@ PHP_PGSQL_API zend_result php_pgsql_insert(PGconn *pg_link, const zend_string *t
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;
@@ -5322,7 +5381,9 @@ PHP_PGSQL_API zend_result php_pgsql_insert(PGconn *pg_link, const zend_string *t
}
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) {
@@ -5332,6 +5393,10 @@ PHP_PGSQL_API zend_result php_pgsql_insert(PGconn *pg_link, const zend_string *t
}
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 {
@@ -5343,15 +5408,19 @@ PHP_PGSQL_API zend_result php_pgsql_insert(PGconn *pg_link, const zend_string *t
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, '\'');
@@ -5507,6 +5576,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 {
@@ -5522,8 +5595,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, '\'');
@@ -5591,7 +5670,9 @@ PHP_PGSQL_API zend_result php_pgsql_update(PGconn *pg_link, const zend_string *t
}
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))
@@ -5694,7 +5775,9 @@ PHP_PGSQL_API zend_result php_pgsql_delete(PGconn *pg_link, const zend_string *t
}
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))
@@ -5834,7 +5917,9 @@ PHP_PGSQL_API zend_result php_pgsql_select(PGconn *pg_link, const zend_string *t
}
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--
+<?php include("skipif.inc"); ?>
+--FILE--
+<?php
+
+include 'config.inc';
+define('FILE_NAME', __DIR__ . '/php.gif');
+
+$db = pg_connect($conn_str);
+pg_query($db, "DROP TABLE IF EXISTS ghsa_hrmw_9436_5mv3");
+pg_query($db, "CREATE TABLE ghsa_hrmw_9436_5mv3 (bar text);");
+
+// pg_escape_literal/pg_escape_identifier
+
+$invalid = "ABC\xff\x30';";
+$flags = PGSQL_DML_NO_CONV | PGSQL_DML_ESCAPE;
+
+var_dump(pg_insert($db, $invalid, ['bar' => '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)