ext/bz2: Check int params of bzcompress() are correct (#16108)

Also add a TODO to check the length of the source strings

Author: Girgias

UPGRADING

@@ -19,6 +19,12 @@ PHP 8.5 UPGRADE NOTES
 1. Backward Incompatible Changes
 ========================================
 
+- BZ2:
+  . bzcompress() now throws a ValueError when $block_size is not between
+    1 and 9.
+  . bzcompress() now throws a ValueError when work_factor is not between
+    0 and 250.
+
 - LDAP:
   . ldap_get_option() and ldap_set_option() now throw a ValueError when
     passing an invalid option.

ext/bz2/bz2.c

@@ -439,40 +439,40 @@ PHP_FUNCTION(bzerror)
 /* {{{ Compresses a string into BZip2 encoded data */
 PHP_FUNCTION(bzcompress)
 {
-	char             *source;          /* Source data to compress */
-	zend_long              zblock_size = 0; /* Optional block size to use */
-	zend_long              zwork_factor = 0;/* Optional work factor to use */
-	zend_string      *dest = NULL;     /* Destination to place the compressed data into */
-	int               error,           /* Error Container */
-					  block_size  = 4, /* Block size for compression algorithm */
-					  work_factor = 0, /* Work factor for compression algorithm */
-					  argc = ZEND_NUM_ARGS(); /* Argument count */
-	size_t               source_len;      /* Length of the source data */
-	unsigned int      dest_len;        /* Length of the destination buffer */
-
-	if (zend_parse_parameters(argc, "s|ll", &source, &source_len, &zblock_size, &zwork_factor) == FAILURE) {
+	char        *source;           /* Source data to compress */
+	zend_long    zblock_size = 4;  /* Block size for compression algorithm */
+	zend_long    zwork_factor = 0; /* Work factor for compression algorithm */
+	zend_string *dest = NULL;      /* Destination to place the compressed data into */
+	size_t       source_len;       /* Length of the source data */
+	unsigned int dest_len;         /* Length of the destination buffer */
+
+	if (zend_parse_parameters(ZEND_NUM_ARGS(), "s|ll", &source, &source_len, &zblock_size, &zwork_factor) == FAILURE) {
 		RETURN_THROWS();
 	}
 
+	if (zblock_size < 1 || zblock_size > 9) {
+		zend_argument_value_error(2, "must be between 1 and 9");
+		RETURN_THROWS();
+	}
+	int block_size = (int) zblock_size;
+
+	if (zwork_factor < 0 || zwork_factor > 250) {
+		zend_argument_value_error(3, "must be between 0 and 250");
+		RETURN_THROWS();
+	}
+	int work_factor = (int) zwork_factor;
+
 	/* Assign them to easy to use variables, dest_len is initially the length of the data
 	   + .01 x length of data + 600 which is the largest size the results of the compression
 	   could possibly be, at least that's what the libbz2 docs say (thanks to jeremy@nirvani.net
 	   for pointing this out).  */
+	// TODO Check source string length fits in unsigned int
 	dest_len = (unsigned int) (source_len + (0.01 * source_len) + 600);
 
 	/* Allocate the destination buffer */
 	dest = zend_string_alloc(dest_len, 0);
 
-	/* Handle the optional arguments */
-	if (argc > 1) {
-		block_size = zblock_size;
-	}
-
-	if (argc > 2) {
-		work_factor = zwork_factor;
-	}
-
-	error = BZ2_bzBuffToBuffCompress(ZSTR_VAL(dest), &dest_len, source, source_len, block_size, 0, work_factor);
+	int error = BZ2_bzBuffToBuffCompress(ZSTR_VAL(dest), &dest_len, source, source_len, block_size, 0, work_factor);
 	if (error != BZ_OK) {
 		zend_string_efree(dest);
 		RETURN_LONG(error);
@@ -512,6 +512,7 @@ PHP_FUNCTION(bzdecompress)
 		RETURN_FALSE;
 	}
 
+	// TODO Check source string length fits in unsigned int
 	bzs.next_in = source;
 	bzs.avail_in = source_len;
 

ext/bz2/tests/005.phpt

@@ -11,10 +11,6 @@ Getting lost within myself
 Nothing matters no one else";
 
 var_dump(bzcompress(1,1,1));
-var_dump(bzcompress($string, 100));
-var_dump(bzcompress($string, 100, -1));
-var_dump(bzcompress($string, 100, 1000));
-var_dump(bzcompress($string, -1, 1));
 
 $data = bzcompress($string);
 $data2 = bzcompress($string, 1, 10);
@@ -35,10 +31,6 @@ echo "Done\n";
 ?>
 --EXPECTF--
 string(%d) "BZ%a"
-int(-2)
-int(-2)
-int(-2)
-int(-2)
 int(-5)
 int(-5)
 int(-5)

ext/bz2/tests/bzcompress_programming_errors.phpt

@@ -0,0 +1,39 @@
+--TEST--
+bzcompress(): providing invalid options
+--EXTENSIONS--
+bz2
+--FILE--
+<?php
+
+$string = "Life it seems, will fade away
+Drifting further everyday
+Getting lost within myself
+Nothing matters no one else";
+
+try {
+    var_dump(bzcompress($string, 0));
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+try {
+    var_dump(bzcompress($string, 100));
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+try {
+    var_dump(bzcompress($string, work_factor: -1));
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+try {
+    var_dump(bzcompress($string, work_factor: 255));
+} catch (Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+?>
+--EXPECT--
+ValueError: bzcompress(): Argument #2 ($block_size) must be between 1 and 9
+ValueError: bzcompress(): Argument #2 ($block_size) must be between 1 and 9
+ValueError: bzcompress(): Argument #3 ($work_factor) must be between 0 and 250
+ValueError: bzcompress(): Argument #3 ($work_factor) must be between 0 and 250