diff --git a/ext/pgsql/pgsql.c b/ext/pgsql/pgsql.c index e90adea43d0a5..4854fdbfd1a2b 100644 --- a/ext/pgsql/pgsql.c +++ b/ext/pgsql/pgsql.c @@ -1136,7 +1136,7 @@ PHP_FUNCTION(pg_query) link = FETCH_DEFAULT_LINK(); CHECK_DEFAULT_LINK(link); - } else { + } else if (ZEND_NUM_ARGS() == 2) { ZEND_PARSE_PARAMETERS_START(2, 2) Z_PARAM_OBJECT_OF_CLASS(pgsql_link, pgsql_link_ce) Z_PARAM_STRING(query, query_len) @@ -1144,6 +1144,9 @@ PHP_FUNCTION(pg_query) link = Z_PGSQL_LINK_P(pgsql_link); CHECK_PGSQL_LINK(link); + } else { + zend_wrong_parameters_count_error(1, 2); + RETURN_THROWS(); } pgsql = link->conn; @@ -1234,7 +1237,7 @@ PHP_FUNCTION(pg_query_params) link = FETCH_DEFAULT_LINK(); CHECK_DEFAULT_LINK(link); - } else { + } else if (ZEND_NUM_ARGS() == 3) { ZEND_PARSE_PARAMETERS_START(3, 3) Z_PARAM_OBJECT_OF_CLASS(pgsql_link, pgsql_link_ce) Z_PARAM_STRING(query, query_len) @@ -1243,6 +1246,9 @@ PHP_FUNCTION(pg_query_params) link = Z_PGSQL_LINK_P(pgsql_link); CHECK_PGSQL_LINK(link); + } else { + zend_wrong_parameters_count_error(2, 3); + RETURN_THROWS(); } pgsql = link->conn; @@ -1344,7 +1350,7 @@ PHP_FUNCTION(pg_prepare) link = FETCH_DEFAULT_LINK(); CHECK_DEFAULT_LINK(link); - } else { + } else if (ZEND_NUM_ARGS() == 3) { ZEND_PARSE_PARAMETERS_START(3, 3) Z_PARAM_OBJECT_OF_CLASS(pgsql_link, pgsql_link_ce) Z_PARAM_STRING(stmtname, stmtname_len) @@ -1353,6 +1359,9 @@ PHP_FUNCTION(pg_prepare) link = Z_PGSQL_LINK_P(pgsql_link); CHECK_PGSQL_LINK(link); + } else { + zend_wrong_parameters_count_error(2, 3); + RETURN_THROWS(); } pgsql = link->conn; @@ -1430,7 +1439,7 @@ PHP_FUNCTION(pg_execute) link = FETCH_DEFAULT_LINK(); CHECK_DEFAULT_LINK(link); - } else { + } else if (ZEND_NUM_ARGS() == 3) { ZEND_PARSE_PARAMETERS_START(3, 3) Z_PARAM_OBJECT_OF_CLASS(pgsql_link, pgsql_link_ce) Z_PARAM_STRING(stmtname, stmtname_len) @@ -1439,6 +1448,9 @@ PHP_FUNCTION(pg_execute) link = Z_PGSQL_LINK_P(pgsql_link); CHECK_PGSQL_LINK(link); + } else { + zend_wrong_parameters_count_error(2, 3); + RETURN_THROWS(); } pgsql = link->conn; @@ -2230,7 +2242,7 @@ static void php_pgsql_data_info(INTERNAL_FUNCTION_PARAMETERS, int entry_type, bo Z_PARAM_OBJECT_OF_CLASS(result, pgsql_result_ce) Z_PARAM_STR_OR_LONG(field_name, field_offset) ZEND_PARSE_PARAMETERS_END(); - } else { + } else if (ZEND_NUM_ARGS() == 3) { ZEND_PARSE_PARAMETERS_START(3, 3) Z_PARAM_OBJECT_OF_CLASS(result, pgsql_result_ce) if (nullable_row) { @@ -2240,6 +2252,9 @@ static void php_pgsql_data_info(INTERNAL_FUNCTION_PARAMETERS, int entry_type, bo } Z_PARAM_STR_OR_LONG(field_name, field_offset) ZEND_PARSE_PARAMETERS_END(); + } else { + zend_wrong_parameters_count_error(2, 3); + RETURN_THROWS(); } pg_result = Z_PGSQL_RESULT_P(result); @@ -3070,7 +3085,7 @@ PHP_FUNCTION(pg_set_error_verbosity) link = FETCH_DEFAULT_LINK(); CHECK_DEFAULT_LINK(link); - } else { + } else if (ZEND_NUM_ARGS() == 2) { ZEND_PARSE_PARAMETERS_START(2, 2) Z_PARAM_OBJECT_OF_CLASS(pgsql_link, pgsql_link_ce) Z_PARAM_LONG(verbosity) @@ -3078,6 +3093,9 @@ PHP_FUNCTION(pg_set_error_verbosity) link = Z_PGSQL_LINK_P(pgsql_link); CHECK_PGSQL_LINK(link); + } else { + zend_wrong_parameters_count_error(1, 2); + RETURN_THROWS(); } pgsql = link->conn; @@ -3148,7 +3166,7 @@ PHP_FUNCTION(pg_set_client_encoding) link = FETCH_DEFAULT_LINK(); CHECK_DEFAULT_LINK(link); - } else { + } else if (ZEND_NUM_ARGS() == 2) { ZEND_PARSE_PARAMETERS_START(2, 2) Z_PARAM_OBJECT_OF_CLASS(pgsql_link, pgsql_link_ce) Z_PARAM_STRING(encoding, encoding_len) @@ -3156,6 +3174,9 @@ PHP_FUNCTION(pg_set_client_encoding) link = Z_PGSQL_LINK_P(pgsql_link); CHECK_PGSQL_LINK(link); + } else { + zend_wrong_parameters_count_error(1, 2); + RETURN_THROWS(); } pgsql = link->conn; @@ -3242,7 +3263,7 @@ PHP_FUNCTION(pg_put_line) link = FETCH_DEFAULT_LINK(); CHECK_DEFAULT_LINK(link); - } else { + } else if (ZEND_NUM_ARGS() == 2) { ZEND_PARSE_PARAMETERS_START(2, 2) Z_PARAM_OBJECT_OF_CLASS(pgsql_link, pgsql_link_ce) Z_PARAM_STRING(query, query_len) @@ -3250,6 +3271,9 @@ PHP_FUNCTION(pg_put_line) link = Z_PGSQL_LINK_P(pgsql_link); CHECK_PGSQL_LINK(link); + } else { + zend_wrong_parameters_count_error(1, 2); + RETURN_THROWS(); } pgsql = link->conn; diff --git a/ext/pgsql/tests/gh17165.phpt b/ext/pgsql/tests/gh17165.phpt new file mode 100644 index 0000000000000..b29f822f21c47 --- /dev/null +++ b/ext/pgsql/tests/gh17165.phpt @@ -0,0 +1,65 @@ +--TEST-- +Fix pg_query()/pg_query_params()/pg_prepare()/pg_execute()/pg_set_error_verbosity()/pg_set_client_encoding()/pg_put_line() pg field infos calls ArgumentCountError message. +--EXTENSIONS-- +pgsql +--SKIPIF-- + +--FILE-- +getMessage(), PHP_EOL; +} + +try { + pg_query_params($db, "b", array(), "d"); +} catch (ArgumentCountError $e) { + echo $e->getMessage(), PHP_EOL; +} + +try { + pg_prepare($db, "a", "b", "c"); +} catch (ArgumentCountError $e) { + echo $e->getMessage(), PHP_EOL; +} + +try { + pg_set_error_verbosity($db, 0, PHP_INT_MAX); +} catch (ArgumentCountError $e) { + echo $e->getMessage(), PHP_EOL; +} + +try { + pg_set_client_encoding($db, "foo", "bar"); +} catch (ArgumentCountError $e) { + echo $e->getMessage(), PHP_EOL; +} + +try { + pg_put_line($db, "my", "data"); +} catch (ArgumentCountError $e) { + echo $e->getMessage(), PHP_EOL; +} + +try { + pg_field_is_null($db, false, "myfield", new stdClass()); +} catch (ArgumentCountError $e) { + echo $e->getMessage(), PHP_EOL; +} +?> +--EXPECT-- +pg_query() expects at most 2 arguments, 3 given +pg_query_params() expects at most 3 arguments, 4 given +pg_prepare() expects at most 3 arguments, 4 given +pg_set_error_verbosity() expects at most 2 arguments, 3 given +pg_set_client_encoding() expects at most 2 arguments, 3 given +pg_put_line() expects at most 2 arguments, 3 given +pg_field_is_null() expects at most 3 arguments, 4 given diff --git a/ext/phar/phar.c b/ext/phar/phar.c index b50142914ec2d..a4a9b7d2139d2 100644 --- a/ext/phar/phar.c +++ b/ext/phar/phar.c @@ -1506,6 +1506,7 @@ zend_result phar_create_or_parse_filename(char *fname, size_t fname_len, char *a } } + ZEND_ASSERT(!mydata->is_persistent); mydata->alias = alias ? estrndup(alias, alias_len) : estrndup(mydata->fname, fname_len); mydata->alias_len = alias ? alias_len : fname_len; } diff --git a/ext/phar/phar_object.c b/ext/phar/phar_object.c index 25f0d7d9fa6b0..dbc3f91b983d1 100644 --- a/ext/phar/phar_object.c +++ b/ext/phar/phar_object.c @@ -2120,9 +2120,8 @@ static zend_object *phar_rename_archive(phar_archive_data **sphar, char *ext) /* efree(newname); if (PHAR_G(manifest_cached) && NULL != (pphar = zend_hash_str_find_ptr(&cached_phars, newpath, phar->fname_len))) { - efree(oldpath); zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Unable to add newly converted phar \"%s\" to the list of phars, new phar name is in phar.cache_list", phar->fname); - return NULL; + goto err_oldpath; } if (NULL != (pphar = zend_hash_str_find_ptr(&(PHAR_G(phar_fname_map)), newpath, phar->fname_len))) { @@ -2134,41 +2133,42 @@ static zend_object *phar_rename_archive(phar_archive_data **sphar, char *ext) /* pphar->flags = phar->flags; pphar->fp = phar->fp; phar->fp = NULL; - /* FIX: GH-10755 Double-free issue caught by ASAN check */ - pphar->alias = phar->alias; /* Transfer alias to pphar to */ - phar->alias = NULL; /* avoid being free'd twice */ + /* The alias is not owned by the phar, so set it to NULL to avoid freeing it. */ + phar->alias = NULL; phar_destroy_phar_data(phar); *sphar = NULL; phar = pphar; + /* NOTE: this phar is now reused, so the refcount must be increased. */ + phar->refcount++; newpath = oldpath; goto its_ok; } } - efree(oldpath); zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Unable to add newly converted phar \"%s\" to the list of phars, a phar with that name already exists", phar->fname); - return NULL; + goto err_oldpath; } its_ok: if (SUCCESS == php_stream_stat_path(newpath, &ssb)) { zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "phar \"%s\" exists and must be unlinked prior to conversion", newpath); - efree(oldpath); - return NULL; + goto err_reused_oldpath; } if (!phar->is_data) { if (SUCCESS != phar_detect_phar_fname_ext(newpath, phar->fname_len, (const char **) &(phar->ext), &ext_len, 1, 1, 1)) { - efree(oldpath); zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "phar \"%s\" has invalid extension %s", phar->fname, ext); - return NULL; + goto err_reused_oldpath; } phar->ext_len = ext_len; - if (phar->alias) { + /* If we are reusing a phar, then the aliases should be already set up correctly, + * and so we should not clear out the alias information. + * This would also leak memory because, unlike the non-reuse path, we actually own the alias memory. */ + if (phar->alias && phar != pphar) { if (phar->is_temporary_alias) { phar->alias = NULL; phar->alias_len = 0; } else { - phar->alias = estrndup(newpath, strlen(newpath)); + phar->alias = pestrndup(newpath, strlen(newpath), phar->is_persistent); phar->alias_len = strlen(newpath); phar->is_temporary_alias = 1; zend_hash_str_update_ptr(&(PHAR_G(phar_alias_map)), newpath, phar->fname_len, phar); @@ -2178,20 +2178,21 @@ static zend_object *phar_rename_archive(phar_archive_data **sphar, char *ext) /* } else { if (SUCCESS != phar_detect_phar_fname_ext(newpath, phar->fname_len, (const char **) &(phar->ext), &ext_len, 0, 1, 1)) { - efree(oldpath); zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "data phar \"%s\" has invalid extension %s", phar->fname, ext); - return NULL; + goto err_reused_oldpath; } phar->ext_len = ext_len; - phar->alias = NULL; - phar->alias_len = 0; + /* See comment in other branch. */ + if (phar != pphar) { + phar->alias = NULL; + phar->alias_len = 0; + } } if ((!pphar || phar == pphar) && NULL == zend_hash_str_update_ptr(&(PHAR_G(phar_fname_map)), newpath, phar->fname_len, phar)) { - efree(oldpath); zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "Unable to add newly converted phar \"%s\" to the list of phars", phar->fname); - return NULL; + goto err_oldpath; } phar_flush_ex(phar, NULL, 1, &error); @@ -2201,8 +2202,7 @@ static zend_object *phar_rename_archive(phar_archive_data **sphar, char *ext) /* *sphar = NULL; zend_throw_exception_ex(spl_ce_BadMethodCallException, 0, "%s", error); efree(error); - efree(oldpath); - return NULL; + goto err_oldpath; } efree(oldpath); @@ -2225,6 +2225,16 @@ static zend_object *phar_rename_archive(phar_archive_data **sphar, char *ext) /* zend_call_known_instance_method_with_1_params(ce->constructor, Z_OBJ(ret), NULL, &arg1); zval_ptr_dtor(&arg1); return Z_OBJ(ret); + +err_reused_oldpath: + if (pphar == phar) { + /* NOTE: we know it's not the last reference because the phar is reused. */ + phar->refcount--; + } + /* fallthrough */ +err_oldpath: + efree(oldpath); + return NULL; } /* }}} */ @@ -2754,8 +2764,13 @@ PHP_METHOD(Phar, setAlias) oldalias_len = phar_obj->archive->alias_len; old_temp = phar_obj->archive->is_temporary_alias; - phar_obj->archive->alias = estrndup(ZSTR_VAL(new_alias), ZSTR_LEN(new_alias)); phar_obj->archive->alias_len = ZSTR_LEN(new_alias); + if (phar_obj->archive->alias_len) { + phar_obj->archive->alias = pestrndup(ZSTR_VAL(new_alias), ZSTR_LEN(new_alias), phar_obj->archive->is_persistent); + } else { + phar_obj->archive->alias = NULL; + } + phar_obj->archive->is_temporary_alias = 0; phar_flush(phar_obj->archive, &error); diff --git a/ext/phar/tests/gh17137.phpt b/ext/phar/tests/gh17137.phpt new file mode 100644 index 0000000000000..b96036420e931 --- /dev/null +++ b/ext/phar/tests/gh17137.phpt @@ -0,0 +1,31 @@ +--TEST-- +GH-17137 (Segmentation fault ext/phar/phar.c) +--EXTENSIONS-- +phar +--INI-- +phar.readonly=0 +--FILE-- +decompress()); +echo "OK\n"; +?> +--EXPECTF-- +object(Phar)#%d (3) { + ["pathName":"SplFileInfo":private]=> + string(0) "" + ["glob":"DirectoryIterator":private]=> + bool(false) + ["subPathName":"RecursiveDirectoryIterator":private]=> + string(0) "" +} +object(Phar)#%d (3) { + ["pathName":"SplFileInfo":private]=> + string(0) "" + ["glob":"DirectoryIterator":private]=> + bool(false) + ["subPathName":"RecursiveDirectoryIterator":private]=> + string(0) "" +} +OK diff --git a/ext/standard/tests/general_functions/proc_open_cmd.phpt b/ext/standard/tests/general_functions/proc_open_cmd.phpt index 6c80871d47df8..2dcc207bfe6ec 100644 --- a/ext/standard/tests/general_functions/proc_open_cmd.phpt +++ b/ext/standard/tests/general_functions/proc_open_cmd.phpt @@ -1,5 +1,7 @@ --TEST-- Harden against cmd.exe hijacking +--CONFLICTS-- +all --SKIPIF-- --EXPECTF-- resource(%d) of type (process)