From 912b13a77953d6bb424ff46986ec7e154abef8a1 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Fri, 6 Dec 2024 17:27:59 +0100 Subject: [PATCH 1/4] Test Firebird in 32-bit Linux CI (#17045) Co-authored-by: =?UTF-8?q?=E6=AD=A6=E7=94=B0=20=E6=86=B2=E5=A4=AA=E9=83=8E?= Co-authored-by: Christoph M. Becker --- .github/actions/apt-x32/action.yml | 1 + .github/actions/configure-x32/action.yml | 1 + .github/actions/test-linux/action.yml | 4 +++- .github/workflows/push.yml | 10 ++++++++++ ext/pdo_firebird/firebird_driver.c | 2 +- ext/pdo_firebird/tests/bug_15604.phpt | 2 +- ext/pdo_firebird/tests/bug_76448.phpt | 2 ++ 7 files changed, 19 insertions(+), 3 deletions(-) diff --git a/.github/actions/apt-x32/action.yml b/.github/actions/apt-x32/action.yml index af1d7918bb28d..39ef1df6c2c8c 100644 --- a/.github/actions/apt-x32/action.yml +++ b/.github/actions/apt-x32/action.yml @@ -38,6 +38,7 @@ runs: libxml2-dev:i386 \ libxpm-dev:i386 \ libxslt1-dev:i386 \ + firebird-dev:i386 \ locales \ make \ pkg-config:i386 \ diff --git a/.github/actions/configure-x32/action.yml b/.github/actions/configure-x32/action.yml index de500e02d4991..a5c5df4f7971d 100644 --- a/.github/actions/configure-x32/action.yml +++ b/.github/actions/configure-x32/action.yml @@ -27,6 +27,7 @@ runs: --with-pgsql \ --with-pdo-pgsql \ --with-pdo-sqlite \ + --with-pdo-firebird \ --without-pear \ --enable-gd \ --with-jpeg \ diff --git a/.github/actions/test-linux/action.yml b/.github/actions/test-linux/action.yml index 164606ecddc12..a5bf49bd00157 100644 --- a/.github/actions/test-linux/action.yml +++ b/.github/actions/test-linux/action.yml @@ -30,7 +30,9 @@ runs: export PDO_PGSQL_TEST_DSN="pgsql:host=localhost port=5432 dbname=test user=postgres password=postgres" fi export PDO_FIREBIRD_TEST_DATABASE=test.fdb - export PDO_FIREBIRD_TEST_DSN=firebird:dbname=localhost:test.fdb + if [[ -z "$PDO_FIREBIRD_TEST_DSN" ]]; then + export PDO_FIREBIRD_TEST_DSN=firebird:dbname=localhost:test.fdb + fi export PDO_FIREBIRD_TEST_PASS=test export PDO_FIREBIRD_TEST_USER=test export ODBC_TEST_USER="odbc_test" diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index f59fcb8d7badc..4705b8874dd05 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -147,6 +147,7 @@ jobs: MYSQL_TEST_HOST: mysql PDO_MYSQL_TEST_DSN: mysql:host=mysql;dbname=test PDO_MYSQL_TEST_HOST: mysql + PDO_FIREBIRD_TEST_DSN: firebird:dbname=firebird:test.fdb services: mysql: image: mysql:8.3 @@ -155,6 +156,15 @@ jobs: env: MYSQL_DATABASE: test MYSQL_ROOT_PASSWORD: root + firebird: + image: jacobalberty/firebird + ports: + - 3050:3050 + env: + ISC_PASSWORD: test + FIREBIRD_DATABASE: test.fdb + FIREBIRD_USER: test + FIREBIRD_PASSWORD: test steps: - name: git checkout uses: actions/checkout@v4 diff --git a/ext/pdo_firebird/firebird_driver.c b/ext/pdo_firebird/firebird_driver.c index 157766f51dd17..9d7cb20d2cdeb 100644 --- a/ext/pdo_firebird/firebird_driver.c +++ b/ext/pdo_firebird/firebird_driver.c @@ -1409,7 +1409,7 @@ static int pdo_firebird_handle_factory(pdo_dbh_t *dbh, zval *driver_options) /* char errmsg[512]; const ISC_STATUS *s = H->isc_status; fb_interpret(errmsg, sizeof(errmsg),&s); - zend_throw_exception_ex(php_pdo_get_exception(), H->isc_status[1], "SQLSTATE[%s] [%ld] %s", + zend_throw_exception_ex(php_pdo_get_exception(), H->isc_status[1], "SQLSTATE[%s] [%" PRIiPTR "] %s", "HY000", H->isc_status[1], errmsg); } diff --git a/ext/pdo_firebird/tests/bug_15604.phpt b/ext/pdo_firebird/tests/bug_15604.phpt index e8aa04c9ddf54..2a31d29e7b8cc 100644 --- a/ext/pdo_firebird/tests/bug_15604.phpt +++ b/ext/pdo_firebird/tests/bug_15604.phpt @@ -15,7 +15,7 @@ $dbh = getDbConnection(); $dbh->exec(' recreate table t_bug_15604 ( - id bigint not null, + id int not null, a int not null, b int, constraint pk_bug_15604 primary key(id) diff --git a/ext/pdo_firebird/tests/bug_76448.phpt b/ext/pdo_firebird/tests/bug_76448.phpt index 0d567dbc6282f..af19736842f69 100644 --- a/ext/pdo_firebird/tests/bug_76448.phpt +++ b/ext/pdo_firebird/tests/bug_76448.phpt @@ -6,6 +6,8 @@ sockets --XLEAK-- A bug in firebird causes a memory leak when calling `isc_attach_database()`. See https://github.com/FirebirdSQL/firebird/issues/7849 +--SKIPIF-- + --FILE-- Date: Thu, 5 Dec 2024 21:13:55 +0100 Subject: [PATCH 2/4] Fix GH-17047: UAF on iconv filter failure The first while loop sets the bucket variable, and this is freed in out_failure. However, when the second "goto out_failure" is triggered then bucket still refers to the bucket from the first while loop, causing a UAF. Fix this by separating the error paths. Closes GH-17058. --- NEWS | 3 +++ ext/iconv/iconv.c | 11 +++-------- ext/iconv/tests/gh17047.phpt | 17 +++++++++++++++++ 3 files changed, 23 insertions(+), 8 deletions(-) create mode 100644 ext/iconv/tests/gh17047.phpt diff --git a/NEWS b/NEWS index a139c14513a57..4a86a80264ce6 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,9 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.3.16 +- Iconv: + . Fixed bug GH-17047 (UAF on iconv filter failure). (nielsdos) + - Streams: . Fixed bug GH-17037 (UAF in user filter when adding existing filter name due to incorrect error handling). (nielsdos) diff --git a/ext/iconv/iconv.c b/ext/iconv/iconv.c index 75f78dd174238..8ac3bc9b3c480 100644 --- a/ext/iconv/iconv.c +++ b/ext/iconv/iconv.c @@ -2536,7 +2536,8 @@ static php_stream_filter_status_t php_iconv_stream_filter_do_filter( if (php_iconv_stream_filter_append_bucket(self, stream, filter, buckets_out, bucket->buf, bucket->buflen, &consumed, php_stream_is_persistent(stream)) != SUCCESS) { - goto out_failure; + php_stream_bucket_delref(bucket); + return PSFS_ERR_FATAL; } php_stream_bucket_delref(bucket); @@ -2546,7 +2547,7 @@ static php_stream_filter_status_t php_iconv_stream_filter_do_filter( if (php_iconv_stream_filter_append_bucket(self, stream, filter, buckets_out, NULL, 0, &consumed, php_stream_is_persistent(stream)) != SUCCESS) { - goto out_failure; + return PSFS_ERR_FATAL; } } @@ -2555,12 +2556,6 @@ static php_stream_filter_status_t php_iconv_stream_filter_do_filter( } return PSFS_PASS_ON; - -out_failure: - if (bucket != NULL) { - php_stream_bucket_delref(bucket); - } - return PSFS_ERR_FATAL; } /* }}} */ diff --git a/ext/iconv/tests/gh17047.phpt b/ext/iconv/tests/gh17047.phpt new file mode 100644 index 0000000000000..a0307ddbe5553 --- /dev/null +++ b/ext/iconv/tests/gh17047.phpt @@ -0,0 +1,17 @@ +--TEST-- +GH-17047 (UAF on iconv filter failure) +--EXTENSIONS-- +iconv +--FILE-- + +--EXPECTF-- +Warning: stream_get_contents(): iconv stream filter ("UTF-16BE"=>"UTF-16BE"): invalid multibyte sequence in %s on line %d +string(0) "" From 063c3c852236ecbe45ab23c0fb271b6292ce82c3 Mon Sep 17 00:00:00 2001 From: Saki Takamachi Date: Sat, 7 Dec 2024 01:52:30 +0900 Subject: [PATCH 3/4] Correctly compare 0 and -0 (#17051) Fixes #17049 Closes #17051 --- NEWS | 5 +- ext/bcmath/libbcmath/src/compare.c | 12 ++++ ext/bcmath/libbcmath/src/str2num.c | 9 +++ ext/bcmath/tests/bccomp_variation002.phpt | 2 + .../number/methods/compare_near_zero.phpt | 71 +++++++++++++++++++ 5 files changed, 98 insertions(+), 1 deletion(-) create mode 100644 ext/bcmath/tests/number/methods/compare_near_zero.phpt diff --git a/NEWS b/NEWS index 7f59f831c9126..19d95ae07cf85 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,9 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.4.3 +- BcMath: + . Fixed bug GH-17049 (Correctly compare 0 and -0). (Saki Takamachi) + - Iconv: . Fixed bug GH-17047 (UAF on iconv filter failure). (nielsdos) @@ -12,7 +15,7 @@ PHP NEWS 05 Dec 2024, PHP 8.4.2 - BcMath: - . Fixed bug GH-16978 (Avoid unnecessary padding with leading zeros) + . Fixed bug GH-16978 (Avoid unnecessary padding with leading zeros). (Saki Takamachi) - COM: diff --git a/ext/bcmath/libbcmath/src/compare.c b/ext/bcmath/libbcmath/src/compare.c index f2df5cc24c57e..2c24dab777059 100644 --- a/ext/bcmath/libbcmath/src/compare.c +++ b/ext/bcmath/libbcmath/src/compare.c @@ -45,6 +45,18 @@ bcmath_compare_result _bc_do_compare(bc_num n1, bc_num n2, size_t scale, bool us /* First, compare signs. */ if (use_sign && n1->n_sign != n2->n_sign) { + /* + * scale and n->n_scale differ only in Number objects. + * This happens when explicitly specify the scale in a Number method. + */ + if ((n1->n_scale > scale || n2->n_scale > scale) && + n1->n_len == 1 && n2->n_len == 1 && + n1->n_value[0] == 0 && n2->n_value[0] == 0 && + bc_is_zero_for_scale(n1, scale) && bc_is_zero_for_scale(n2, scale) + ) { + /* e.g. 0.00 <=> -0.00 */ + return BCMATH_EQUAL; + } if (n1->n_sign == PLUS) { /* Positive N1 > Negative N2 */ return BCMATH_LEFT_GREATER; diff --git a/ext/bcmath/libbcmath/src/str2num.c b/ext/bcmath/libbcmath/src/str2num.c index a56fefa02a4c7..79c1d4216fe7b 100644 --- a/ext/bcmath/libbcmath/src/str2num.c +++ b/ext/bcmath/libbcmath/src/str2num.c @@ -173,6 +173,15 @@ bool bc_str2num(bc_num *num, const char *str, const char *end, size_t scale, siz if (str_scale > scale && !auto_scale) { fractional_end -= str_scale - scale; str_scale = scale; + + /* + * e.g. 123.0001 with scale 2 -> 123.00 + * So, remove the trailing 0 again. + */ + if (str_scale > 0) { + const char *fractional_new_end = bc_skip_zero_reverse(fractional_end, fractional_ptr); + str_scale -= fractional_new_end - fractional_end; + } } } else { if (full_scale) { diff --git a/ext/bcmath/tests/bccomp_variation002.phpt b/ext/bcmath/tests/bccomp_variation002.phpt index 299f454780601..145181083570b 100644 --- a/ext/bcmath/tests/bccomp_variation002.phpt +++ b/ext/bcmath/tests/bccomp_variation002.phpt @@ -13,6 +13,7 @@ echo bccomp("-2.29", "2.3", "2") . "\n"; echo bccomp("2.29", "-2.3", "2") . "\n"; echo bccomp("-2.29", "-2.3", "1") . "\n"; echo bccomp("-2.29", "0", "1") . "\n"; +echo bccomp("0.001", "-0.001", "1") . "\n"; ?> --EXPECT-- 0 @@ -22,3 +23,4 @@ echo bccomp("-2.29", "0", "1") . "\n"; 1 1 -1 +0 diff --git a/ext/bcmath/tests/number/methods/compare_near_zero.phpt b/ext/bcmath/tests/number/methods/compare_near_zero.phpt new file mode 100644 index 0000000000000..342b38294790b --- /dev/null +++ b/ext/bcmath/tests/number/methods/compare_near_zero.phpt @@ -0,0 +1,71 @@ +--TEST-- +BcMath\Number compare() with scale +--EXTENSIONS-- +bcmath +--FILE-- + {$value2}: ", 20, ' ', STR_PAD_LEFT); + $output .= str_pad((string) $value1->compare($value2, $scale), 2, ' ', STR_PAD_LEFT); + echo "{$output}\n"; + } + } +} +?> +--EXPECT-- +========== scale is 0 ========== + 0.001 <=> 0: 0 + 0.001 <=> 0: 0 + 0.001 <=> 0.0011: 0 + 0.001 <=> -0.0011: 0 + -0.001 <=> 0: 0 + -0.001 <=> 0: 0 + -0.001 <=> 0.0011: 0 +-0.001 <=> -0.0011: 0 +========== scale is 2 ========== + 0.001 <=> 0: 0 + 0.001 <=> 0: 0 + 0.001 <=> 0.0011: 0 + 0.001 <=> -0.0011: 0 + -0.001 <=> 0: 0 + -0.001 <=> 0: 0 + -0.001 <=> 0.0011: 0 +-0.001 <=> -0.0011: 0 +========== scale is 3 ========== + 0.001 <=> 0: 1 + 0.001 <=> 0: 1 + 0.001 <=> 0.0011: 0 + 0.001 <=> -0.0011: 1 + -0.001 <=> 0: -1 + -0.001 <=> 0: -1 + -0.001 <=> 0.0011: -1 +-0.001 <=> -0.0011: 0 +========== scale is 4 ========== + 0.001 <=> 0: 1 + 0.001 <=> 0: 1 + 0.001 <=> 0.0011: -1 + 0.001 <=> -0.0011: 1 + -0.001 <=> 0: -1 + -0.001 <=> 0: -1 + -0.001 <=> 0.0011: -1 +-0.001 <=> -0.0011: 1 From b88dcc9f3edfabe168e812b632c2b223a0217418 Mon Sep 17 00:00:00 2001 From: Saki Takamachi Date: Sat, 7 Dec 2024 02:00:04 +0900 Subject: [PATCH 4/4] Now `Number::round()` does not remove trailing zeros (#17063) Fixes #17061 Closes #17063 --- NEWS | 2 + ext/bcmath/bcmath.c | 2 - .../methods/round_has_trailing_zeros.phpt | 72 +++++++++++++++++++ 3 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 ext/bcmath/tests/number/methods/round_has_trailing_zeros.phpt diff --git a/NEWS b/NEWS index 19d95ae07cf85..d9568bbae24e2 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,8 @@ PHP NEWS - BcMath: . Fixed bug GH-17049 (Correctly compare 0 and -0). (Saki Takamachi) + . Fixed bug GH-17061 (Now Number::round() does not remove trailing zeros). + (Saki Takamachi) - Iconv: . Fixed bug GH-17047 (UAF on iconv filter failure). (nielsdos) diff --git a/ext/bcmath/bcmath.c b/ext/bcmath/bcmath.c index 1b4dd331c9b95..832d2758fe968 100644 --- a/ext/bcmath/bcmath.c +++ b/ext/bcmath/bcmath.c @@ -1801,8 +1801,6 @@ PHP_METHOD(BcMath_Number, round) bc_num ret = NULL; bc_round(intern->num, precision, rounding_mode, &ret); - bc_rm_trailing_zeros(ret); - bcmath_number_obj_t *new_intern = bcmath_number_new_obj(ret, ret->n_scale); RETURN_OBJ(&new_intern->std); } diff --git a/ext/bcmath/tests/number/methods/round_has_trailing_zeros.phpt b/ext/bcmath/tests/number/methods/round_has_trailing_zeros.phpt new file mode 100644 index 0000000000000..f526f3b2f91cd --- /dev/null +++ b/ext/bcmath/tests/number/methods/round_has_trailing_zeros.phpt @@ -0,0 +1,72 @@ +--TEST-- +BcMath\Number round() - has trailing zeros +--EXTENSIONS-- +bcmath +--FILE-- +name} ==========\n"; + foreach ([ + ['0.00', 1], + ['0.1', 5], + ['-0.300', 5], + ['1.995', 2], + ] as [$value, $precision]) { + $number = new BcMath\Number($value); + $output = 'value is '; + $output .= str_pad($value, 6, ' ', STR_PAD_RIGHT) . ', '; + $output .= "precision is {$precision}: "; + $output .= $number->round($precision, $mode) . "\n"; + echo $output; + } + echo "\n"; +} +?> +--EXPECT-- +========== HalfAwayFromZero ========== +value is 0.00 , precision is 1: 0.0 +value is 0.1 , precision is 5: 0.10000 +value is -0.300, precision is 5: -0.30000 +value is 1.995 , precision is 2: 2.00 + +========== HalfTowardsZero ========== +value is 0.00 , precision is 1: 0.0 +value is 0.1 , precision is 5: 0.10000 +value is -0.300, precision is 5: -0.30000 +value is 1.995 , precision is 2: 1.99 + +========== HalfEven ========== +value is 0.00 , precision is 1: 0.0 +value is 0.1 , precision is 5: 0.10000 +value is -0.300, precision is 5: -0.30000 +value is 1.995 , precision is 2: 2.00 + +========== HalfOdd ========== +value is 0.00 , precision is 1: 0.0 +value is 0.1 , precision is 5: 0.10000 +value is -0.300, precision is 5: -0.30000 +value is 1.995 , precision is 2: 1.99 + +========== TowardsZero ========== +value is 0.00 , precision is 1: 0.0 +value is 0.1 , precision is 5: 0.10000 +value is -0.300, precision is 5: -0.30000 +value is 1.995 , precision is 2: 1.99 + +========== AwayFromZero ========== +value is 0.00 , precision is 1: 0.0 +value is 0.1 , precision is 5: 0.10000 +value is -0.300, precision is 5: -0.30000 +value is 1.995 , precision is 2: 2.00 + +========== NegativeInfinity ========== +value is 0.00 , precision is 1: 0.0 +value is 0.1 , precision is 5: 0.10000 +value is -0.300, precision is 5: -0.30000 +value is 1.995 , precision is 2: 1.99 + +========== PositiveInfinity ========== +value is 0.00 , precision is 1: 0.0 +value is 0.1 , precision is 5: 0.10000 +value is -0.300, precision is 5: -0.30000 +value is 1.995 , precision is 2: 2.00