Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 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);
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