From 74eff98c84b26a8088fb56b5be748a3e0e1da419 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Tue, 6 Aug 2024 19:13:09 +0200 Subject: [PATCH] Deprecate passing incorrect data types for options to ext/hash functions (#15236) RFC: https://wiki.php.net/rfc/deprecations_php_8_4#deprecate_passing_incorrect_data_types_for_options_to_exthash_functions --- NEWS | 4 ++ UPGRADING | 4 ++ ext/hash/hash_murmur.c | 39 +++++++++++++------ ext/hash/hash_xxhash.c | 29 +++++++++----- ext/hash/tests/murmur_seed_deprecation.phpt | 16 ++++++++ .../tests/xxh3_convert_secret_to_string.phpt | 3 +- ext/hash/tests/xxhash_secret.phpt | 6 ++- ext/hash/tests/xxhash_seed_deprecation.phpt | 28 +++++++++++++ 8 files changed, 106 insertions(+), 23 deletions(-) create mode 100644 ext/hash/tests/murmur_seed_deprecation.phpt create mode 100644 ext/hash/tests/xxhash_seed_deprecation.phpt diff --git a/NEWS b/NEWS index 2a0c854e3b0..393aab037a2 100644 --- a/NEWS +++ b/NEWS @@ -20,6 +20,10 @@ PHP NEWS . Deprecated DOM_PHP_ERR constant. (nielsdos) . Removed DOMImplementation::getFeature(). (nielsdos) +- Hash: + . Deprecated passing incorrect data types for options to ext/hash functions. + (nielsdos) + - PHPDBG: . array out of bounds, stack overflow handled for segfault handler on windows. (David Carlier) diff --git a/UPGRADING b/UPGRADING index 6f48afe6b76..51705677354 100644 --- a/UPGRADING +++ b/UPGRADING @@ -413,6 +413,10 @@ PHP 8.4 UPGRADE NOTES . Deprecated DOM_PHP_ERR constant. RFC: https://wiki.php.net/rfc/deprecations_php_8_4#deprecate_dom_php_err_constant +- Hash: + . Deprecated passing incorrect data types for options to ext/hash functions. + RFC: https://wiki.php.net/rfc/deprecations_php_8_4 + - Intl: . Calling intlcal_set() as well as calling IntlCalendar::set() with more than 2 arguments is deprecated. Use either IntlCalendar::setDate() diff --git a/ext/hash/hash_murmur.c b/ext/hash/hash_murmur.c index 2d4d2a66843..384133b84bd 100644 --- a/ext/hash/hash_murmur.c +++ b/ext/hash/hash_murmur.c @@ -42,8 +42,13 @@ PHP_HASH_API void PHP_MURMUR3AInit(PHP_MURMUR3A_CTX *ctx, HashTable *args) zval *seed = zend_hash_str_find_deref(args, "seed", sizeof("seed") - 1); /* This might be a bit too restrictive, but thinking that a seed might be set once and for all, it should be done a clean way. */ - if (seed && IS_LONG == Z_TYPE_P(seed)) { - ctx->h = (uint32_t)Z_LVAL_P(seed); + if (seed) { + if (IS_LONG == Z_TYPE_P(seed)) { + ctx->h = (uint32_t) Z_LVAL_P(seed); + } else { + php_error_docref(NULL, E_DEPRECATED, "Passing a seed of a type other than int is deprecated because it is the same as setting the seed to 0"); + ctx->h = 0; + } } else { ctx->h = 0; } @@ -99,12 +104,17 @@ PHP_HASH_API void PHP_MURMUR3CInit(PHP_MURMUR3C_CTX *ctx, HashTable *args) zval *seed = zend_hash_str_find_deref(args, "seed", sizeof("seed") - 1); /* This might be a bit too restrictive, but thinking that a seed might be set once and for all, it should be done a clean way. */ - if (seed && IS_LONG == Z_TYPE_P(seed)) { - uint32_t _seed = (uint32_t)Z_LVAL_P(seed); - ctx->h[0] = _seed; - ctx->h[1] = _seed; - ctx->h[2] = _seed; - ctx->h[3] = _seed; + if (seed) { + if (IS_LONG == Z_TYPE_P(seed)) { + uint32_t _seed = (uint32_t)Z_LVAL_P(seed); + ctx->h[0] = _seed; + ctx->h[1] = _seed; + ctx->h[2] = _seed; + ctx->h[3] = _seed; + } else { + php_error_docref(NULL, E_DEPRECATED, "Passing a seed of a type other than int is deprecated because it is the same as setting the seed to 0"); + memset(&ctx->h, 0, sizeof ctx->h); + } } else { memset(&ctx->h, 0, sizeof ctx->h); } @@ -173,10 +183,15 @@ PHP_HASH_API void PHP_MURMUR3FInit(PHP_MURMUR3F_CTX *ctx, HashTable *args) zval *seed = zend_hash_str_find_deref(args, "seed", sizeof("seed") - 1); /* This might be a bit too restrictive, but thinking that a seed might be set once and for all, it should be done a clean way. */ - if (seed && IS_LONG == Z_TYPE_P(seed)) { - uint64_t _seed = (uint64_t)Z_LVAL_P(seed); - ctx->h[0] = _seed; - ctx->h[1] = _seed; + if (seed) { + if (IS_LONG == Z_TYPE_P(seed)) { + uint64_t _seed = (uint64_t) Z_LVAL_P(seed); + ctx->h[0] = _seed; + ctx->h[1] = _seed; + } else { + php_error_docref(NULL, E_DEPRECATED, "Passing a seed of a type other than int is deprecated because it is the same as setting the seed to 0"); + memset(&ctx->h, 0, sizeof ctx->h); + } } else { memset(&ctx->h, 0, sizeof ctx->h); } diff --git a/ext/hash/hash_xxhash.c b/ext/hash/hash_xxhash.c index 24da754d883..07ac801d99e 100644 --- a/ext/hash/hash_xxhash.c +++ b/ext/hash/hash_xxhash.c @@ -46,14 +46,17 @@ PHP_HASH_API void PHP_XXH32Init(PHP_XXH32_CTX *ctx, HashTable *args) zval *seed = zend_hash_str_find_deref(args, "seed", sizeof("seed") - 1); /* This might be a bit too restrictive, but thinking that a seed might be set once and for all, it should be done a clean way. */ - if (seed && IS_LONG == Z_TYPE_P(seed)) { - XXH32_reset(&ctx->s, (XXH32_hash_t)Z_LVAL_P(seed)); - } else { - XXH32_reset(&ctx->s, 0); + if (seed) { + if (IS_LONG == Z_TYPE_P(seed)) { + XXH32_reset(&ctx->s, (XXH32_hash_t)Z_LVAL_P(seed)); + return; + } else { + php_error_docref(NULL, E_DEPRECATED, "Passing a seed of a type other than int is deprecated because it is the same as setting the seed to 0"); + } } - } else { - XXH32_reset(&ctx->s, 0); } + + XXH32_reset(&ctx->s, 0); } PHP_HASH_API void PHP_XXH32Update(PHP_XXH32_CTX *ctx, const unsigned char *in, size_t len) @@ -112,12 +115,13 @@ PHP_HASH_API void PHP_XXH64Init(PHP_XXH64_CTX *ctx, HashTable *args) once and for all, it should be done a clean way. */ if (seed && IS_LONG == Z_TYPE_P(seed)) { XXH64_reset(&ctx->s, (XXH64_hash_t)Z_LVAL_P(seed)); + return; } else { - XXH64_reset(&ctx->s, 0); + php_error_docref(NULL, E_DEPRECATED, "Passing a seed of a type other than int is deprecated because it is the same as setting the seed to 0"); } - } else { - XXH64_reset(&ctx->s, 0); } + + XXH64_reset(&ctx->s, 0); } PHP_HASH_API void PHP_XXH64Update(PHP_XXH64_CTX *ctx, const unsigned char *in, size_t len) @@ -168,12 +172,19 @@ zend_always_inline static void _PHP_XXH3_Init(PHP_XXH3_64_CTX *ctx, HashTable *a return; } + if (_seed && IS_LONG != Z_TYPE_P(_seed)) { + php_error_docref(NULL, E_DEPRECATED, "Passing a seed of a type other than int is deprecated because it is ignored"); + } + if (_seed && IS_LONG == Z_TYPE_P(_seed)) { /* This might be a bit too restrictive, but thinking that a seed might be set once and for all, it should be done a clean way. */ func_init_seed(&ctx->s, (XXH64_hash_t)Z_LVAL_P(_seed)); return; } else if (_secret) { + if (IS_STRING != Z_TYPE_P(_secret)) { + php_error_docref(NULL, E_DEPRECATED, "Passing a seed of a type other than string is deprecated because it implicitly converts to a string, potentially hiding bugs"); + } zend_string *secret_string = zval_try_get_string(_secret); if (UNEXPECTED(!secret_string)) { ZEND_ASSERT(EG(exception)); diff --git a/ext/hash/tests/murmur_seed_deprecation.phpt b/ext/hash/tests/murmur_seed_deprecation.phpt new file mode 100644 index 00000000000..0baefd8e7a3 --- /dev/null +++ b/ext/hash/tests/murmur_seed_deprecation.phpt @@ -0,0 +1,16 @@ +--TEST-- +Hash: murmur3 seed deprecation of edge cases +--FILE-- + "42"]); +} + +?> +--EXPECTF-- +Deprecated: hash_init(): Passing a seed of a type other than int is deprecated because it is the same as setting the seed to 0 in %s on line %d + +Deprecated: hash_init(): Passing a seed of a type other than int is deprecated because it is the same as setting the seed to 0 in %s on line %d + +Deprecated: hash_init(): Passing a seed of a type other than int is deprecated because it is the same as setting the seed to 0 in %s on line %d diff --git a/ext/hash/tests/xxh3_convert_secret_to_string.phpt b/ext/hash/tests/xxh3_convert_secret_to_string.phpt index dfc161b084c..a68c4a14223 100644 --- a/ext/hash/tests/xxh3_convert_secret_to_string.phpt +++ b/ext/hash/tests/xxh3_convert_secret_to_string.phpt @@ -7,7 +7,8 @@ try { } catch (Throwable) {} var_dump($x); ?> ---EXPECT-- +--EXPECTF-- +Deprecated: hash_init(): Passing a seed of a type other than string is deprecated because it implicitly converts to a string, potentially hiding bugs in %s on line %d array(1) { ["secret"]=> int(4) diff --git a/ext/hash/tests/xxhash_secret.phpt b/ext/hash/tests/xxhash_secret.phpt index 6f1efbe6c90..d450d057867 100644 --- a/ext/hash/tests/xxhash_secret.phpt +++ b/ext/hash/tests/xxhash_secret.phpt @@ -46,12 +46,16 @@ foreach (["xxh3", "xxh128"] as $a) { } ?> ---EXPECT-- +--EXPECTF-- string(67) "xxh3: Only one of seed or secret is to be passed for initialization" + +Deprecated: hash_init(): Passing a seed of a type other than string is deprecated because it implicitly converts to a string, potentially hiding bugs in %s on line %d string(23) "exception in __toString" string(57) "xxh3: Secret length must be >= 136 bytes, 17 bytes passed" 8028aa834c03557a == 8028aa834c03557a == true string(69) "xxh128: Only one of seed or secret is to be passed for initialization" + +Deprecated: hash_init(): Passing a seed of a type other than string is deprecated because it implicitly converts to a string, potentially hiding bugs in %s on line %d string(23) "exception in __toString" string(59) "xxh128: Secret length must be >= 136 bytes, 17 bytes passed" 54279097795e7218093a05d4d781cbb9 == 54279097795e7218093a05d4d781cbb9 == true diff --git a/ext/hash/tests/xxhash_seed_deprecation.phpt b/ext/hash/tests/xxhash_seed_deprecation.phpt new file mode 100644 index 00000000000..6c9634cade2 --- /dev/null +++ b/ext/hash/tests/xxhash_seed_deprecation.phpt @@ -0,0 +1,28 @@ +--TEST-- +Hash: xxHash seed deprecation of edge cases +--FILE-- + "42"]); +} + +foreach (["xxh3", "xxh128"] as $a) { + try { + hash_init($a, options: ["secret" => 42]); + } catch (Throwable) {} +} + +?> +--EXPECTF-- +Deprecated: hash_init(): Passing a seed of a type other than int is deprecated because it is the same as setting the seed to 0 in %s on line %d + +Deprecated: hash_init(): Passing a seed of a type other than int is deprecated because it is the same as setting the seed to 0 in %s on line %d + +Deprecated: hash_init(): Passing a seed of a type other than int is deprecated because it is ignored in %s on line %d + +Deprecated: hash_init(): Passing a seed of a type other than int is deprecated because it is ignored in %s on line %d + +Deprecated: hash_init(): Passing a seed of a type other than string is deprecated because it implicitly converts to a string, potentially hiding bugs in %s on line %d + +Deprecated: hash_init(): Passing a seed of a type other than string is deprecated because it implicitly converts to a string, potentially hiding bugs in %s on line %d