From dcf65219398e78b619c41989dc023f1330f889b1 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Sun, 14 Jun 2026 17:58:55 +0100 Subject: [PATCH] bz2: refactor filter creation to do parameter validation --- ext/bz2/bz2_filter.c | 200 +++++++++++------- ext/bz2/tests/bug72447.phpt | 4 +- ext/bz2/tests/bz2_filter_compress_errors.phpt | 67 ++++++ .../tests/bz2_filter_decompress_errors.phpt | 19 ++ ext/bz2/tests/bz2_filter_unknown_errors.phpt | 16 ++ 5 files changed, 223 insertions(+), 83 deletions(-) create mode 100644 ext/bz2/tests/bz2_filter_compress_errors.phpt create mode 100644 ext/bz2/tests/bz2_filter_decompress_errors.phpt create mode 100644 ext/bz2/tests/bz2_filter_unknown_errors.phpt diff --git a/ext/bz2/bz2_filter.c b/ext/bz2/bz2_filter.c index 09c49fa76687..4c65a5cb75ae 100644 --- a/ext/bz2/bz2_filter.c +++ b/ext/bz2/bz2_filter.c @@ -21,7 +21,7 @@ /* {{{ data structure */ -enum strm_status { +C23_ENUM(strm_status, uint8_t) { PHP_BZ2_UNINITIALIZED, PHP_BZ2_RUNNING, PHP_BZ2_FINISHED @@ -34,12 +34,11 @@ typedef struct _php_bz2_filter_data { size_t inbuf_len; size_t outbuf_len; - enum strm_status status; /* Decompress option */ - unsigned int small_footprint : 1; /* Decompress option */ - unsigned int expect_concatenated : 1; /* Decompress option */ - unsigned int is_flushed : 1; /* only for compression */ - - int persistent; + bool persistent; + bool expect_concatenated : 1; /* Decompress option */ + bool small_footprint : 1; /* Decompress option */ + bool is_flushed : 1; /* only for compression */ + strm_status status; /* Decompress option */ /* Configuration for reset - immutable */ int blockSize100k; /* compress only */ @@ -376,22 +375,8 @@ static const php_stream_filter_ops php_bz2_compress_ops = { /* }}} */ -/* {{{ bzip2.* common factory */ - -static php_stream_filter *php_bz2_filter_create(const char *filtername, zval *filterparams, bool persistent) -{ - const php_stream_filter_ops *fops = NULL; - php_stream_filter_seekable_t write_seekable; - php_bz2_filter_data *data; - int status = BZ_OK; - - if (php_stream_filter_parse_write_seek_mode(filterparams, &write_seekable) == FAILURE) { - return NULL; - } - - /* Create this filter */ - data = pecalloc(1, sizeof(php_bz2_filter_data), persistent); - +static php_bz2_filter_data *php_bz2_filter_data_new(bool persistent) { + php_bz2_filter_data *data = pecalloc(1, sizeof(php_bz2_filter_data), persistent); /* Circular reference */ data->strm.opaque = (void *) data; @@ -402,86 +387,137 @@ static php_stream_filter *php_bz2_filter_create(const char *filtername, zval *fi data->strm.next_in = data->inbuf = (char *) pemalloc(data->inbuf_len, persistent); data->strm.avail_in = 0; data->strm.next_out = data->outbuf = (char *) pemalloc(data->outbuf_len, persistent); + return data; +} - if (strcasecmp(filtername, "bzip2.decompress") == 0) { - data->small_footprint = 0; - data->expect_concatenated = 0; +static php_stream_filter *php_bz2_decompress_filter_create(zval *filter_params, bool persistent) { + php_stream_filter_seekable_t write_seekable = PSFS_SEEKABLE_ALWAYS; + bool small_footprint = false; + bool expect_concatenated = false; - if (filterparams) { - zval *tmpzval = NULL; + if (filter_params) { + if (Z_TYPE_P(filter_params) == IS_TRUE || Z_TYPE_P(filter_params) == IS_FALSE) { + small_footprint = Z_TYPE_P(filter_params) == IS_TRUE; + goto allocate_decompress_filter; + } + if (UNEXPECTED(Z_TYPE_P(filter_params) != IS_ARRAY && Z_TYPE_P(filter_params) != IS_OBJECT)) { + php_error_docref(NULL, E_WARNING, + "Filter parameters for bzip2.decompress filter must be of type array|bool, %s given", + zend_zval_type_name(filter_params) + ); + return NULL; + } - if (Z_TYPE_P(filterparams) == IS_ARRAY || Z_TYPE_P(filterparams) == IS_OBJECT) { - HashTable *ht = HASH_OF(filterparams); + const HashTable *filter_params_ht = HASH_OF(filter_params); + /* TODO: convert php_stream_filter_parse_write_seek_mode() to take HashTable */ + if (php_stream_filter_parse_write_seek_mode(filter_params, &write_seekable) == FAILURE) { + return NULL; + } - if ((tmpzval = zend_hash_str_find_ind(ht, "concatenated", sizeof("concatenated")-1))) { - data->expect_concatenated = zend_is_true(tmpzval); - tmpzval = NULL; - } + const zval *concatenated = zend_hash_str_find_ind(filter_params_ht, ZEND_STRL("concatenated")); + if (concatenated) { + expect_concatenated = zend_is_true(concatenated); + } - tmpzval = zend_hash_str_find_ind(ht, "small", sizeof("small")-1); - } else { - tmpzval = filterparams; - } + const zval *small = zend_hash_str_find_ind(filter_params_ht, ZEND_STRL("small")); + if (small) { + small_footprint = zend_is_true(small); + } + } - if (tmpzval) { - data->small_footprint = zend_is_true(tmpzval); - } +allocate_decompress_filter:; + php_bz2_filter_data *data = php_bz2_filter_data_new(persistent); + /* Save configuration for reset */ + data->small_footprint = small_footprint; + data->expect_concatenated = expect_concatenated; + data->status = PHP_BZ2_UNINITIALIZED; + + return php_stream_filter_alloc(&php_bz2_decompress_ops, data, persistent, PSFS_SEEKABLE_START, write_seekable); +} + +static php_stream_filter *php_bz2_compress_filter_create(zval *filter_params, bool persistent) { + php_stream_filter_seekable_t write_seekable = PSFS_SEEKABLE_ALWAYS; + int blockSize100k = PHP_BZ2_FILTER_DEFAULT_BLOCKSIZE; + int workFactor = PHP_BZ2_FILTER_DEFAULT_WORKFACTOR; + + if (filter_params) { + if (UNEXPECTED(Z_TYPE_P(filter_params) != IS_ARRAY && Z_TYPE_P(filter_params) != IS_OBJECT)) { + php_error_docref(NULL, E_WARNING, + "Filter parameters for bzip2.compress filter must be of type array, %s given", + zend_zval_type_name(filter_params) + ); + return NULL; } - data->status = PHP_BZ2_UNINITIALIZED; - fops = &php_bz2_decompress_ops; - } else if (strcasecmp(filtername, "bzip2.compress") == 0) { - int blockSize100k = PHP_BZ2_FILTER_DEFAULT_BLOCKSIZE; - int workFactor = PHP_BZ2_FILTER_DEFAULT_WORKFACTOR; - - if (filterparams) { - zval *tmpzval; - - if (Z_TYPE_P(filterparams) == IS_ARRAY || Z_TYPE_P(filterparams) == IS_OBJECT) { - HashTable *ht = HASH_OF(filterparams); - - if ((tmpzval = zend_hash_str_find_ind(ht, "blocks", sizeof("blocks")-1))) { - /* How much memory to allocate (1 - 9) x 100kb */ - zend_long blocks = zval_get_long(tmpzval); - if (blocks < 1 || blocks > 9) { - php_error_docref(NULL, E_WARNING, "Invalid parameter given for number of blocks to allocate (" ZEND_LONG_FMT ")", blocks); - } else { - blockSize100k = (int) blocks; - } - } + const HashTable *filter_params_ht = HASH_OF(filter_params); + /* TODO: convert php_stream_filter_parse_write_seek_mode() to take HashTable */ + if (php_stream_filter_parse_write_seek_mode(filter_params, &write_seekable) == FAILURE) { + return NULL; + } - if ((tmpzval = zend_hash_str_find_ind(ht, "work", sizeof("work")-1))) { - /* Work Factor (0 - 250) */ - zend_long work = zval_get_long(tmpzval); - if (work < 0 || work > 250) { - php_error_docref(NULL, E_WARNING, "Invalid parameter given for work factor (" ZEND_LONG_FMT ")", work); - } else { - workFactor = (int) work; - } - } + const zval *blocks_zv = zend_hash_str_find_ind(filter_params_ht, ZEND_STRL("blocks")); + if (blocks_zv) { + ZEND_ASSERT(Z_TYPE_P(blocks_zv) != IS_INDIRECT); + bool failed = false; + /* How much memory to allocate (1 - 9) x 100kb */ + zend_long blocks = zval_try_get_long(blocks_zv, &failed); + if (UNEXPECTED(failed)) { + php_error_docref(NULL, E_WARNING, "Number of blocks parameter must be of type int, %s given", zend_zval_type_name(blocks_zv)); + return NULL; + } else if (blocks < 1 || blocks > 9) { + php_error_docref(NULL, E_WARNING, "Number of blocks to allocate must be between 1 and 9, " ZEND_LONG_FMT " given", blocks); + return NULL; + } else { + blockSize100k = (int) blocks; } } - /* Save configuration for reset */ - data->blockSize100k = blockSize100k; - data->workFactor = workFactor; - - status = BZ2_bzCompressInit(&(data->strm), blockSize100k, 0, workFactor); - data->is_flushed = 1; - fops = &php_bz2_compress_ops; - } else { - status = BZ_DATA_ERROR; + const zval *work_zv = zend_hash_str_find_ind(filter_params_ht, ZEND_STRL("work")); + if (work_zv) { + ZEND_ASSERT(Z_TYPE_P(work_zv) != IS_INDIRECT); + bool failed = false; + /* Work Factor (0 - 250) */ + zend_long work = zval_try_get_long(work_zv, &failed); + if (UNEXPECTED(failed)) { + php_error_docref(NULL, E_WARNING, "Work factor parameter must be of type int, %s given", zend_zval_type_name(work_zv)); + return NULL; + } else if (work < 0 || work > 250) { + php_error_docref(NULL, E_WARNING, "Work factor must be between 0 and 250, " ZEND_LONG_FMT " given", work); + return NULL; + } else { + workFactor = (int) work; + } + } } - if (status != BZ_OK) { + php_bz2_filter_data *data = php_bz2_filter_data_new(persistent); + /* Save configuration for reset */ + data->blockSize100k = blockSize100k; + data->workFactor = workFactor; + + int status = BZ2_bzCompressInit(&(data->strm), blockSize100k, 0, workFactor); + if (UNEXPECTED(status != BZ_OK)) { /* Unspecified (probably strm) error, let stream-filter error do its own whining */ pefree(data->strm.next_in, persistent); pefree(data->strm.next_out, persistent); pefree(data, persistent); return NULL; } + data->is_flushed = true; + + return php_stream_filter_alloc(&php_bz2_compress_ops, data, persistent, PSFS_SEEKABLE_START, write_seekable); +} - return php_stream_filter_alloc(fops, data, persistent, PSFS_SEEKABLE_START, write_seekable); +/* {{{ bzip2.* common factory */ +static php_stream_filter *php_bz2_filter_create(const char *filtername, zval *filterparams, bool persistent) +{ + if (strcasecmp(filtername, "bzip2.decompress") == 0) { + return php_bz2_decompress_filter_create(filterparams, persistent); + } else if (strcasecmp(filtername, "bzip2.compress") == 0) { + return php_bz2_compress_filter_create(filterparams, persistent); + } else { + return NULL; + } } const php_stream_filter_factory php_bz2_filter_factory = { diff --git a/ext/bz2/tests/bug72447.phpt b/ext/bz2/tests/bug72447.phpt index 11f3bd9136b5..8e2fc2b79802 100644 --- a/ext/bz2/tests/bug72447.phpt +++ b/ext/bz2/tests/bug72447.phpt @@ -16,4 +16,6 @@ fclose($fp); unlink('testfile'); ?> --EXPECTF-- -Warning: stream_filter_append(): Invalid parameter given for number of blocks to allocate (0) in %s%ebug72447.php on line %d +Warning: stream_filter_append(): Number of blocks parameter must be of type int, string given in %s on line %d + +Warning: stream_filter_append(): Unable to create or locate filter "bzip2.compress" in %s on line %d diff --git a/ext/bz2/tests/bz2_filter_compress_errors.phpt b/ext/bz2/tests/bz2_filter_compress_errors.phpt new file mode 100644 index 000000000000..fb7059ca5034 --- /dev/null +++ b/ext/bz2/tests/bz2_filter_compress_errors.phpt @@ -0,0 +1,67 @@ +--TEST-- +bzip2.compress filter param errors +--EXTENSIONS-- +bz2 +--FILE-- + 'not an int']; +var_dump(stream_filter_append($fp, 'bzip2.compress', STREAM_FILTER_WRITE, $param)); + +$param = ['blocks' => '15']; +var_dump(stream_filter_append($fp, 'bzip2.compress', STREAM_FILTER_WRITE, $param)); + +$param = ['blocks' => '0']; +var_dump(stream_filter_append($fp, 'bzip2.compress', STREAM_FILTER_WRITE, $param)); + +$param = ['work' => 'not an int']; +var_dump(stream_filter_append($fp, 'bzip2.compress', STREAM_FILTER_WRITE, $param)); + +$param = ['work' => '251']; +var_dump(stream_filter_append($fp, 'bzip2.compress', STREAM_FILTER_WRITE, $param)); + +$param = ['work' => '-1']; +var_dump(stream_filter_append($fp, 'bzip2.compress', STREAM_FILTER_WRITE, $param)); + +fclose($fp); + +?> +--EXPECTF-- +Warning: stream_filter_append(): Filter parameters for bzip2.compress filter must be of type array, string given in %s on line %d + +Warning: stream_filter_append(): Unable to create or locate filter "bzip2.compress" in %s on line %d +bool(false) + +Warning: stream_filter_append(): Number of blocks parameter must be of type int, string given in %s on line %d + +Warning: stream_filter_append(): Unable to create or locate filter "bzip2.compress" in %s on line %d +bool(false) + +Warning: stream_filter_append(): Number of blocks to allocate must be between 1 and 9, 15 given in %s on line %d + +Warning: stream_filter_append(): Unable to create or locate filter "bzip2.compress" in %s on line %d +bool(false) + +Warning: stream_filter_append(): Number of blocks to allocate must be between 1 and 9, 0 given in %s on line %d + +Warning: stream_filter_append(): Unable to create or locate filter "bzip2.compress" in %s on line %d +bool(false) + +Warning: stream_filter_append(): Work factor parameter must be of type int, string given in %s on line %d + +Warning: stream_filter_append(): Unable to create or locate filter "bzip2.compress" in %s on line %d +bool(false) + +Warning: stream_filter_append(): Work factor must be between 0 and 250, 251 given in %s on line %d + +Warning: stream_filter_append(): Unable to create or locate filter "bzip2.compress" in %s on line %d +bool(false) + +Warning: stream_filter_append(): Work factor must be between 0 and 250, -1 given in %s on line %d + +Warning: stream_filter_append(): Unable to create or locate filter "bzip2.compress" in %s on line %d +bool(false) diff --git a/ext/bz2/tests/bz2_filter_decompress_errors.phpt b/ext/bz2/tests/bz2_filter_decompress_errors.phpt new file mode 100644 index 000000000000..f27cea6253f9 --- /dev/null +++ b/ext/bz2/tests/bz2_filter_decompress_errors.phpt @@ -0,0 +1,19 @@ +--TEST-- +bzip2.decompress filter param errors +--EXTENSIONS-- +bz2 +--FILE-- + +--EXPECTF-- +Warning: stream_filter_append(): Filter parameters for bzip2.decompress filter must be of type array|bool, string given in %s on line %d + +Warning: stream_filter_append(): Unable to create or locate filter "bzip2.decompress" in %s on line %d +bool(false) diff --git a/ext/bz2/tests/bz2_filter_unknown_errors.phpt b/ext/bz2/tests/bz2_filter_unknown_errors.phpt new file mode 100644 index 000000000000..356e7237c459 --- /dev/null +++ b/ext/bz2/tests/bz2_filter_unknown_errors.phpt @@ -0,0 +1,16 @@ +--TEST-- +bzip2.decompress filter param errors +--EXTENSIONS-- +bz2 +--FILE-- + +--EXPECTF-- +Warning: stream_filter_append(): Unable to create or locate filter "bzip2.i_dont_exist" in %s on line %d +bool(false)