Skip to content

Commit 49d94cc

Browse files
committed
Merge branch 'PHP-8.4'
* PHP-8.4: Fix GHSA-453j-q27h-5p8x Fix GHSA-hrwm-9436-5mv3: pgsql escaping no error checks Fix GHSA-3cr5-j632-f35r: Null byte in hostnames
2 parents 4a98b36 + a179e39 commit 49d94cc

File tree

10 files changed

+332
-28
lines changed

10 files changed

+332
-28
lines changed

ext/pdo_pgsql/pgsql_driver.c

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -378,11 +378,15 @@ static zend_string* pgsql_handle_quoter(pdo_dbh_t *dbh, const zend_string *unquo
378378
zend_string *quoted_str;
379379
pdo_pgsql_db_handle *H = (pdo_pgsql_db_handle *)dbh->driver_data;
380380
size_t tmp_len;
381+
int err;
381382

382383
switch (paramtype) {
383384
case PDO_PARAM_LOB:
384385
/* escapedlen returned by PQescapeBytea() accounts for trailing 0 */
385386
escaped = PQescapeByteaConn(H->server, (unsigned char *)ZSTR_VAL(unquoted), ZSTR_LEN(unquoted), &tmp_len);
387+
if (escaped == NULL) {
388+
return NULL;
389+
}
386390
quotedlen = tmp_len + 1;
387391
quoted = emalloc(quotedlen + 1);
388392
memcpy(quoted+1, escaped, quotedlen-2);
@@ -394,7 +398,11 @@ static zend_string* pgsql_handle_quoter(pdo_dbh_t *dbh, const zend_string *unquo
394398
default:
395399
quoted = safe_emalloc(2, ZSTR_LEN(unquoted), 3);
396400
quoted[0] = '\'';
397-
quotedlen = PQescapeStringConn(H->server, quoted + 1, ZSTR_VAL(unquoted), ZSTR_LEN(unquoted), NULL);
401+
quotedlen = PQescapeStringConn(H->server, quoted + 1, ZSTR_VAL(unquoted), ZSTR_LEN(unquoted), &err);
402+
if (err) {
403+
efree(quoted);
404+
return NULL;
405+
}
398406
quoted[quotedlen + 1] = '\'';
399407
quoted[quotedlen + 2] = '\0';
400408
quotedlen += 2;
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
--TEST--
2+
#GHSA-hrwm-9436-5mv3: pdo_pgsql extension does not check for errors during escaping
3+
--EXTENSIONS--
4+
pdo
5+
pdo_pgsql
6+
--SKIPIF--
7+
<?php
8+
require_once dirname(__FILE__) . '/../../../ext/pdo/tests/pdo_test.inc';
9+
require_once dirname(__FILE__) . '/config.inc';
10+
PDOTest::skip();
11+
?>
12+
--FILE--
13+
<?php
14+
require_once dirname(__FILE__) . '/../../../ext/pdo/tests/pdo_test.inc';
15+
require_once dirname(__FILE__) . '/config.inc';
16+
$db = PDOTest::test_factory(dirname(__FILE__) . '/common.phpt');
17+
$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
18+
19+
$invalid = "ABC\xff\x30';";
20+
var_dump($db->quote($invalid));
21+
22+
?>
23+
--EXPECT--
24+
bool(false)

ext/pgsql/pgsql.c

Lines changed: 105 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3572,8 +3572,14 @@ PHP_FUNCTION(pg_escape_string)
35723572

35733573
to = zend_string_safe_alloc(ZSTR_LEN(from), 2, 0, 0);
35743574
if (link) {
3575+
int err;
35753576
pgsql = link->conn;
3576-
ZSTR_LEN(to) = PQescapeStringConn(pgsql, ZSTR_VAL(to), ZSTR_VAL(from), ZSTR_LEN(from), NULL);
3577+
ZSTR_LEN(to) = PQescapeStringConn(pgsql, ZSTR_VAL(to), ZSTR_VAL(from), ZSTR_LEN(from), &err);
3578+
if (err) {
3579+
zend_argument_value_error(ZEND_NUM_ARGS(), "Escaping string failed");
3580+
zend_string_efree(to);
3581+
RETURN_THROWS();
3582+
}
35773583
} else
35783584
{
35793585
ZSTR_LEN(to) = PQescapeString(ZSTR_VAL(to), ZSTR_VAL(from), ZSTR_LEN(from));
@@ -3619,6 +3625,10 @@ PHP_FUNCTION(pg_escape_bytea)
36193625
} else {
36203626
to = (char *)PQescapeBytea((unsigned char *)ZSTR_VAL(from), ZSTR_LEN(from), &to_len);
36213627
}
3628+
if (to == NULL) {
3629+
zend_argument_value_error(ZEND_NUM_ARGS(), "Escape failure");
3630+
RETURN_THROWS();
3631+
}
36223632

36233633
RETVAL_STRINGL(to, to_len-1); /* to_len includes additional '\0' */
36243634
PQfreemem(to);
@@ -4572,7 +4582,7 @@ PHP_PGSQL_API zend_result php_pgsql_meta_data(PGconn *pg_link, const zend_string
45724582
char *escaped;
45734583
smart_str querystr = {0};
45744584
size_t new_len, len;
4575-
int i, num_rows;
4585+
int i, num_rows, err;
45764586
zval elem;
45774587

45784588
ZEND_ASSERT(ZSTR_LEN(table_name) != 0);
@@ -4611,7 +4621,14 @@ PHP_PGSQL_API zend_result php_pgsql_meta_data(PGconn *pg_link, const zend_string
46114621
}
46124622
len = strlen(tmp_name2);
46134623
escaped = (char *)safe_emalloc(len, 2, 1);
4614-
new_len = PQescapeStringConn(pg_link, escaped, tmp_name2, len, NULL);
4624+
new_len = PQescapeStringConn(pg_link, escaped, tmp_name2, len, &err);
4625+
if (err) {
4626+
php_error_docref(NULL, E_WARNING, "Escaping table name '%s' failed", ZSTR_VAL(table_name));
4627+
efree(src);
4628+
efree(escaped);
4629+
smart_str_free(&querystr);
4630+
return FAILURE;
4631+
}
46154632
if (new_len) {
46164633
smart_str_appendl(&querystr, escaped, new_len);
46174634
}
@@ -4620,7 +4637,14 @@ PHP_PGSQL_API zend_result php_pgsql_meta_data(PGconn *pg_link, const zend_string
46204637
smart_str_appends(&querystr, "' AND n.nspname = '");
46214638
len = strlen(tmp_name);
46224639
escaped = (char *)safe_emalloc(len, 2, 1);
4623-
new_len = PQescapeStringConn(pg_link, escaped, tmp_name, len, NULL);
4640+
new_len = PQescapeStringConn(pg_link, escaped, tmp_name, len, &err);
4641+
if (err) {
4642+
php_error_docref(NULL, E_WARNING, "Escaping table namespace '%s' failed", ZSTR_VAL(table_name));
4643+
efree(src);
4644+
efree(escaped);
4645+
smart_str_free(&querystr);
4646+
return FAILURE;
4647+
}
46244648
if (new_len) {
46254649
smart_str_appendl(&querystr, escaped, new_len);
46264650
}
@@ -4875,7 +4899,7 @@ PHP_PGSQL_API zend_result php_pgsql_convert(PGconn *pg_link, const zend_string *
48754899
{
48764900
zend_string *field = NULL;
48774901
zval meta, *def, *type, *not_null, *has_default, *is_enum, *val, new_val;
4878-
int err = 0;
4902+
int err = 0, escape_err = 0;
48794903
bool skip_field;
48804904
php_pgsql_data_type data_type;
48814905

@@ -5121,8 +5145,13 @@ PHP_PGSQL_API zend_result php_pgsql_convert(PGconn *pg_link, const zend_string *
51215145
/* PostgreSQL ignores \0 */
51225146
str = zend_string_alloc(Z_STRLEN_P(val) * 2, 0);
51235147
/* better to use PGSQLescapeLiteral since PGescapeStringConn does not handle special \ */
5124-
ZSTR_LEN(str) = PQescapeStringConn(pg_link, ZSTR_VAL(str), Z_STRVAL_P(val), Z_STRLEN_P(val), NULL);
5125-
ZVAL_STR(&new_val, php_pgsql_add_quotes(str));
5148+
ZSTR_LEN(str) = PQescapeStringConn(pg_link, ZSTR_VAL(str),
5149+
Z_STRVAL_P(val), Z_STRLEN_P(val), &escape_err);
5150+
if (escape_err) {
5151+
err = 1;
5152+
} else {
5153+
ZVAL_STR(&new_val, php_pgsql_add_quotes(str));
5154+
}
51265155
zend_string_release_ex(str, false);
51275156
}
51285157
break;
@@ -5145,7 +5174,14 @@ PHP_PGSQL_API zend_result php_pgsql_convert(PGconn *pg_link, const zend_string *
51455174
}
51465175
PGSQL_CONV_CHECK_IGNORE();
51475176
if (err) {
5148-
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));
5177+
if (escape_err) {
5178+
php_error_docref(NULL, E_NOTICE,
5179+
"String value escaping failed for PostgreSQL '%s' (%s)",
5180+
Z_STRVAL_P(type), ZSTR_VAL(field));
5181+
} else {
5182+
zend_type_error("%s(): Field \"%s\" must be of type string|null, %s given",
5183+
get_active_function_name(), ZSTR_VAL(field), Z_STRVAL_P(type));
5184+
}
51495185
}
51505186
break;
51515187

@@ -5379,6 +5415,11 @@ PHP_PGSQL_API zend_result php_pgsql_convert(PGconn *pg_link, const zend_string *
53795415
zend_string *tmp_zstr;
53805416

53815417
tmp = PQescapeByteaConn(pg_link, (unsigned char *)Z_STRVAL_P(val), Z_STRLEN_P(val), &to_len);
5418+
if (tmp == NULL) {
5419+
php_error_docref(NULL, E_NOTICE, "Escaping value failed for %s field (%s)", Z_STRVAL_P(type), ZSTR_VAL(field));
5420+
err = 1;
5421+
break;
5422+
}
53825423
tmp_zstr = zend_string_init((char *)tmp, to_len - 1, false); /* PQescapeBytea's to_len includes additional '\0' */
53835424
PQfreemem(tmp);
53845425

@@ -5455,6 +5496,12 @@ PHP_PGSQL_API zend_result php_pgsql_convert(PGconn *pg_link, const zend_string *
54555496
zend_hash_update(Z_ARRVAL_P(result), field, &new_val);
54565497
} else {
54575498
char *escaped = PQescapeIdentifier(pg_link, ZSTR_VAL(field), ZSTR_LEN(field));
5499+
if (escaped == NULL) {
5500+
/* This cannot fail because of invalid string but only due to failed memory allocation */
5501+
php_error_docref(NULL, E_NOTICE, "Escaping field '%s' failed", ZSTR_VAL(field));
5502+
err = 1;
5503+
break;
5504+
}
54585505
add_assoc_zval(result, escaped, &new_val);
54595506
PQfreemem(escaped);
54605507
}
@@ -5537,7 +5584,7 @@ static bool do_exec(smart_str *querystr, ExecStatusType expect, PGconn *pg_link,
55375584
}
55385585
/* }}} */
55395586

5540-
static inline void build_tablename(smart_str *querystr, PGconn *pg_link, const zend_string *table) /* {{{ */
5587+
static inline zend_result build_tablename(smart_str *querystr, PGconn *pg_link, const zend_string *table) /* {{{ */
55415588
{
55425589
/* schema.table should be "schema"."table" */
55435590
const char *dot = memchr(ZSTR_VAL(table), '.', ZSTR_LEN(table));
@@ -5547,6 +5594,10 @@ static inline void build_tablename(smart_str *querystr, PGconn *pg_link, const z
55475594
smart_str_appendl(querystr, ZSTR_VAL(table), len);
55485595
} else {
55495596
char *escaped = PQescapeIdentifier(pg_link, ZSTR_VAL(table), len);
5597+
if (escaped == NULL) {
5598+
php_error_docref(NULL, E_NOTICE, "Failed to escape table name '%s'", ZSTR_VAL(table));
5599+
return FAILURE;
5600+
}
55505601
smart_str_appends(querystr, escaped);
55515602
PQfreemem(escaped);
55525603
}
@@ -5559,11 +5610,17 @@ static inline void build_tablename(smart_str *querystr, PGconn *pg_link, const z
55595610
smart_str_appendl(querystr, after_dot, len);
55605611
} else {
55615612
char *escaped = PQescapeIdentifier(pg_link, after_dot, len);
5613+
if (escaped == NULL) {
5614+
php_error_docref(NULL, E_NOTICE, "Failed to escape table name '%s'", ZSTR_VAL(table));
5615+
return FAILURE;
5616+
}
55625617
smart_str_appendc(querystr, '.');
55635618
smart_str_appends(querystr, escaped);
55645619
PQfreemem(escaped);
55655620
}
55665621
}
5622+
5623+
return SUCCESS;
55675624
}
55685625
/* }}} */
55695626

@@ -5584,7 +5641,9 @@ PHP_PGSQL_API zend_result php_pgsql_insert(PGconn *pg_link, const zend_string *t
55845641
ZVAL_UNDEF(&converted);
55855642
if (zend_hash_num_elements(Z_ARRVAL_P(var_array)) == 0) {
55865643
smart_str_appends(&querystr, "INSERT INTO ");
5587-
build_tablename(&querystr, pg_link, table);
5644+
if (build_tablename(&querystr, pg_link, table) == FAILURE) {
5645+
goto cleanup;
5646+
}
55885647
smart_str_appends(&querystr, " DEFAULT VALUES");
55895648

55905649
goto no_values;
@@ -5600,7 +5659,9 @@ PHP_PGSQL_API zend_result php_pgsql_insert(PGconn *pg_link, const zend_string *t
56005659
}
56015660

56025661
smart_str_appends(&querystr, "INSERT INTO ");
5603-
build_tablename(&querystr, pg_link, table);
5662+
if (build_tablename(&querystr, pg_link, table) == FAILURE) {
5663+
goto cleanup;
5664+
}
56045665
smart_str_appends(&querystr, " (");
56055666

56065667
ZEND_HASH_FOREACH_STR_KEY(Z_ARRVAL_P(var_array), fld) {
@@ -5610,6 +5671,10 @@ PHP_PGSQL_API zend_result php_pgsql_insert(PGconn *pg_link, const zend_string *t
56105671
}
56115672
if (opt & PGSQL_DML_ESCAPE) {
56125673
tmp = PQescapeIdentifier(pg_link, ZSTR_VAL(fld), ZSTR_LEN(fld) + 1);
5674+
if (tmp == NULL) {
5675+
php_error_docref(NULL, E_NOTICE, "Failed to escape field '%s'", ZSTR_VAL(fld));
5676+
goto cleanup;
5677+
}
56135678
smart_str_appends(&querystr, tmp);
56145679
PQfreemem(tmp);
56155680
} else {
@@ -5621,15 +5686,19 @@ PHP_PGSQL_API zend_result php_pgsql_insert(PGconn *pg_link, const zend_string *t
56215686
smart_str_appends(&querystr, ") VALUES (");
56225687

56235688
/* make values string */
5624-
ZEND_HASH_FOREACH_VAL(Z_ARRVAL_P(var_array), val) {
5689+
ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(var_array), fld, val) {
56255690
/* we can avoid the key_type check here, because we tested it in the other loop */
56265691
switch (Z_TYPE_P(val)) {
56275692
case IS_STRING:
56285693
if (opt & PGSQL_DML_ESCAPE) {
5629-
size_t new_len;
5630-
char *tmp;
5631-
tmp = (char *)safe_emalloc(Z_STRLEN_P(val), 2, 1);
5632-
new_len = PQescapeStringConn(pg_link, tmp, Z_STRVAL_P(val), Z_STRLEN_P(val), NULL);
5694+
int error;
5695+
char *tmp = safe_emalloc(Z_STRLEN_P(val), 2, 1);
5696+
size_t new_len = PQescapeStringConn(pg_link, tmp, Z_STRVAL_P(val), Z_STRLEN_P(val), &error);
5697+
if (error) {
5698+
php_error_docref(NULL, E_NOTICE, "Failed to escape field '%s' value", ZSTR_VAL(fld));
5699+
efree(tmp);
5700+
goto cleanup;
5701+
}
56335702
smart_str_appendc(&querystr, '\'');
56345703
smart_str_appendl(&querystr, tmp, new_len);
56355704
smart_str_appendc(&querystr, '\'');
@@ -5787,6 +5856,10 @@ static inline int build_assignment_string(PGconn *pg_link, smart_str *querystr,
57875856
}
57885857
if (opt & PGSQL_DML_ESCAPE) {
57895858
char *tmp = PQescapeIdentifier(pg_link, ZSTR_VAL(fld), ZSTR_LEN(fld) + 1);
5859+
if (tmp == NULL) {
5860+
php_error_docref(NULL, E_NOTICE, "Failed to escape field '%s'", ZSTR_VAL(fld));
5861+
return -1;
5862+
}
57905863
smart_str_appends(querystr, tmp);
57915864
PQfreemem(tmp);
57925865
} else {
@@ -5802,8 +5875,14 @@ static inline int build_assignment_string(PGconn *pg_link, smart_str *querystr,
58025875
switch (Z_TYPE_P(val)) {
58035876
case IS_STRING:
58045877
if (opt & PGSQL_DML_ESCAPE) {
5878+
int error;
58055879
char *tmp = (char *)safe_emalloc(Z_STRLEN_P(val), 2, 1);
5806-
size_t new_len = PQescapeStringConn(pg_link, tmp, Z_STRVAL_P(val), Z_STRLEN_P(val), NULL);
5880+
size_t new_len = PQescapeStringConn(pg_link, tmp, Z_STRVAL_P(val), Z_STRLEN_P(val), &error);
5881+
if (error) {
5882+
php_error_docref(NULL, E_NOTICE, "Failed to escape field '%s' value", ZSTR_VAL(fld));
5883+
efree(tmp);
5884+
return -1;
5885+
}
58075886
smart_str_appendc(querystr, '\'');
58085887
smart_str_appendl(querystr, tmp, new_len);
58095888
smart_str_appendc(querystr, '\'');
@@ -5871,7 +5950,9 @@ PHP_PGSQL_API zend_result php_pgsql_update(PGconn *pg_link, const zend_string *t
58715950
}
58725951

58735952
smart_str_appends(&querystr, "UPDATE ");
5874-
build_tablename(&querystr, pg_link, table);
5953+
if (build_tablename(&querystr, pg_link, table) == FAILURE) {
5954+
goto cleanup;
5955+
}
58755956
smart_str_appends(&querystr, " SET ");
58765957

58775958
if (build_assignment_string(pg_link, &querystr, Z_ARRVAL_P(var_array), 0, ",", 1, opt))
@@ -5977,7 +6058,9 @@ PHP_PGSQL_API zend_result php_pgsql_delete(PGconn *pg_link, const zend_string *t
59776058
}
59786059

59796060
smart_str_appends(&querystr, "DELETE FROM ");
5980-
build_tablename(&querystr, pg_link, table);
6061+
if (build_tablename(&querystr, pg_link, table) == FAILURE) {
6062+
goto cleanup;
6063+
}
59816064
smart_str_appends(&querystr, " WHERE ");
59826065

59836066
if (build_assignment_string(pg_link, &querystr, Z_ARRVAL_P(ids_array), 1, " AND ", sizeof(" AND ")-1, opt))
@@ -6121,7 +6204,9 @@ PHP_PGSQL_API zend_result php_pgsql_select(PGconn *pg_link, const zend_string *t
61216204
}
61226205

61236206
smart_str_appends(&querystr, "SELECT * FROM ");
6124-
build_tablename(&querystr, pg_link, table);
6207+
if (build_tablename(&querystr, pg_link, table) == FAILURE) {
6208+
goto cleanup;
6209+
}
61256210

61266211
if (is_valid_ids_array) {
61276212
smart_str_appends(&querystr, " WHERE ");

0 commit comments

Comments
 (0)