Skip to content

Commit

Permalink
ext/bz2: Check int params of bzcompress() are correct (#16108)
Browse files Browse the repository at this point in the history
Also add a TODO to check the length of the source strings
  • Loading branch information
Girgias authored Sep 28, 2024
1 parent 4a8cd31 commit d4c88a2
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 30 deletions.
6 changes: 6 additions & 0 deletions UPGRADING
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
45 changes: 23 additions & 22 deletions ext/bz2/bz2.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 [email protected]
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);
Expand Down Expand Up @@ -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;

Expand Down
8 changes: 0 additions & 8 deletions ext/bz2/tests/005.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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)
Expand Down
39 changes: 39 additions & 0 deletions ext/bz2/tests/bzcompress_programming_errors.phpt
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit d4c88a2

Please sign in to comment.