From f26250c7c78aba959e57d5ac5b1aa388d5685f64 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Fri, 24 Jan 2025 12:24:18 +0100 Subject: [PATCH 1/7] Backport nightly.yml This file should stay up-to-date for consistent behavior across workflow triggers. --- .github/workflows/nightly.yml | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 90e9a1d7b760c..4041271f4d041 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -227,6 +227,8 @@ jobs: runs-on: ubuntu-latest container: image: ubuntu:${{ inputs.ubuntu_version }} + env: + PDO_FIREBIRD_TEST_DSN: firebird:dbname=firebird:test.fdb services: mysql: image: mysql:8.3 @@ -235,6 +237,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 @@ -952,10 +963,18 @@ jobs: - x64: true zts: true opcache: true + asan: false - x64: false zts: false opcache: false - name: "WINDOWS_${{ matrix.x64 && 'X64' || 'X86' }}_${{ matrix.zts && 'ZTS' || 'NTS' }}" + asan: false + - x64: true + zts: true + opcache: true + asan: true + branch: 'master' + timeout: 120 + name: "WINDOWS_${{ matrix.x64 && 'X64' || 'X86' }}_${{ matrix.zts && 'ZTS' || 'NTS' }}${{ matrix.asan && '_ASAN' || ''}}" runs-on: windows-${{ inputs.windows_version }} env: PHP_BUILD_CACHE_BASE_DIR: C:\build-cache @@ -968,6 +987,7 @@ jobs: INTRINSICS: "${{ matrix.zts && 'AVX2' || '' }}" PARALLEL: -j2 OPCACHE: "${{ matrix.opcache && '1' || '0' }}" + ASAN: "${{ matrix.asan && '1' || '0' }}" steps: - name: git config run: git config --global core.autocrlf false && git config --global core.eol lf From 5b8c960c9feb0db240da8818d901a11bdaceba0e Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Fri, 24 Jan 2025 12:40:36 +0100 Subject: [PATCH 2/7] Skip Symfony/Wordpress in 8.1 build There are two issues: The latest Symfony branches don't support 8.1 anymore. This could ber mitigated by switching to LTS for security builds. However, there are also some JIT bugs that are hard to backport. We'll skip these builds on 8.1 instead. --- .github/workflows/nightly.yml | 12 +++++++++--- .github/workflows/root.yml | 2 ++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 4041271f4d041..fba5b095f83d1 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -26,6 +26,12 @@ on: windows_version: required: true type: string + skip_symfony: + required: true + type: boolean + skip_wordpress: + required: true + type: boolean permissions: contents: read jobs: @@ -554,7 +560,7 @@ jobs: exit 1 fi - name: Test Symfony - if: always() + if: always() && !inputs.skip_symfony run: | git clone https://github.com/symfony/symfony.git --depth=1 cd symfony @@ -586,7 +592,7 @@ jobs: exit 1 fi - name: 'Symfony Preloading' - if: always() + if: always() && !inputs.skip_symfony run: | php /usr/bin/composer create-project symfony/symfony-demo symfony_demo --no-progress --ignore-platform-reqs cd symfony_demo @@ -594,7 +600,7 @@ jobs: sed -i 's/PHP_SAPI/"cli-server"/g' var/cache/dev/App_KernelDevDebugContainer.preload.php php -d opcache.preload=var/cache/dev/App_KernelDevDebugContainer.preload.php public/index.php - name: Test Wordpress - if: always() + if: always() && !inputs.skip_wordpress run: | git clone https://github.com/WordPress/wordpress-develop.git wordpress --depth=1 cd wordpress diff --git a/.github/workflows/root.yml b/.github/workflows/root.yml index cefabd0394a46..5cdd70489343f 100644 --- a/.github/workflows/root.yml +++ b/.github/workflows/root.yml @@ -59,4 +59,6 @@ jobs: || ((matrix.branch.version[0] == 8 && matrix.branch.version[1] >= 3) && '22.04') || '20.04' }} windows_version: ${{ ((matrix.branch.version[0] == 8 && matrix.branch.version[1] >= 4) || matrix.branch.version[0] >= 9) && '2022' || '2019' }} + skip_symfony: ${{ matrix.branch.version[0] == 8 && matrix.branch.version[1] == 1 }} + skip_wordpress: ${{ matrix.branch.version[0] == 8 && matrix.branch.version[1] == 1 }} secrets: inherit From 5b32011fb5651bbad42572c7ceacdd85e65f4726 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Fri, 24 Jan 2025 14:25:24 +0100 Subject: [PATCH 3/7] [skip ci] Use !cancelled() over always() in GHA config According to the documentation, !cancelled() should be used over always() when the step should be executed regardless of success of failure, but canceled when the workflow is canceled. See https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/evaluate-expressions-in-workflows-and-actions#always --- .github/workflows/nightly.yml | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index fba5b095f83d1..c2816d72cf18e 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -427,7 +427,7 @@ jobs: -d zend_extension=opcache.so -d opcache.enable_cli=1 - uses: codecov/codecov-action@v4 - if: always() + if: !cancelled() with: fail_ci_if_error: true token: ${{ secrets.CODECOV_TOKEN }} @@ -500,7 +500,7 @@ jobs: echo opcache.jit_hot_side_exit=1 >> /etc/php.d/opcache.ini php -v - name: Test AMPHP - if: always() + if: !cancelled() run: | repositories="amp cache dns file http parallel parser pipeline process serialization socket sync websocket-client websocket-server" X=0 @@ -518,7 +518,7 @@ jobs: done exit $X - name: Test Laravel - if: always() + if: !cancelled() run: | git clone https://github.com/laravel/framework.git --branch=master --depth=1 cd framework @@ -531,7 +531,7 @@ jobs: exit 1 fi - name: Test ReactPHP - if: always() + if: !cancelled() run: | repositories="async cache child-process datagram dns event-loop promise promise-stream promise-timer stream" X=0 @@ -549,7 +549,7 @@ jobs: done exit $X - name: Test Revolt PHP - if: always() + if: !cancelled() run: | git clone https://github.com/revoltphp/event-loop.git --depth=1 cd event-loop @@ -560,7 +560,7 @@ jobs: exit 1 fi - name: Test Symfony - if: always() && !inputs.skip_symfony + if: !cancelled() && !inputs.skip_symfony run: | git clone https://github.com/symfony/symfony.git --depth=1 cd symfony @@ -581,7 +581,7 @@ jobs: done exit $X - name: Test PHPUnit - if: always() + if: !cancelled() run: | git clone https://github.com/sebastianbergmann/phpunit.git --branch=main --depth=1 cd phpunit @@ -592,7 +592,7 @@ jobs: exit 1 fi - name: 'Symfony Preloading' - if: always() && !inputs.skip_symfony + if: !cancelled() && !inputs.skip_symfony run: | php /usr/bin/composer create-project symfony/symfony-demo symfony_demo --no-progress --ignore-platform-reqs cd symfony_demo @@ -600,7 +600,7 @@ jobs: sed -i 's/PHP_SAPI/"cli-server"/g' var/cache/dev/App_KernelDevDebugContainer.preload.php php -d opcache.preload=var/cache/dev/App_KernelDevDebugContainer.preload.php public/index.php - name: Test Wordpress - if: always() && !inputs.skip_wordpress + if: !cancelled() && !inputs.skip_wordpress run: | git clone https://github.com/WordPress/wordpress-develop.git wordpress --depth=1 cd wordpress From a85666c17b4cfd68a8f782bbab606ee97aeb9b52 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Wed, 22 Jan 2025 17:27:59 +0000 Subject: [PATCH 4/7] ext/session: Fix GH-17541 (ext/session NULL pointer dereferencement during ID reset) Closes GH-17541 Closes GH-17546 --- NEWS | 2 ++ ext/session/session.c | 21 +++++++++------- ext/session/tests/bug66481.phpt | 2 +- ext/session/tests/gh17541.phpt | 24 +++++++++++++++++++ .../tests/session_name_variation1.phpt | 20 ++++++++-------- 5 files changed, 50 insertions(+), 19 deletions(-) create mode 100644 ext/session/tests/gh17541.phpt diff --git a/NEWS b/NEWS index 9c75b031eb19e..1084d620deb9a 100644 --- a/NEWS +++ b/NEWS @@ -50,6 +50,8 @@ PHP NEWS - Session: . Fix type confusion with session SID constant. (nielsdos) + . Fixed bug GH-17541 (ext/session NULL pointer dereferencement during + ID reset). (Girgias) - SimpleXML: . Fixed bug GH-17409 (Assertion failure Zend/zend_hash.c:1730). (nielsdos) diff --git a/ext/session/session.c b/ext/session/session.c index b261f9a35594a..ccf01d093b653 100644 --- a/ext/session/session.c +++ b/ext/session/session.c @@ -94,6 +94,7 @@ zend_class_entry *php_session_update_timestamp_iface_entry; } #define SESSION_FORBIDDEN_CHARS "=,;.[ \t\r\n\013\014" +#define SESSION_FORBIDDEN_CHARS_FOR_ERROR_MSG "=,;.[ \\t\\r\\n\\013\\014" #define APPLY_TRANS_SID (PS(use_trans_sid) && !PS(use_only_cookies)) @@ -683,7 +684,12 @@ static PHP_INI_MH(OnUpdateName) /* {{{ */ SESSION_CHECK_OUTPUT_STATE; /* Numeric session.name won't work at all */ - if ((!ZSTR_LEN(new_value) || is_numeric_string(ZSTR_VAL(new_value), ZSTR_LEN(new_value), NULL, NULL, 0))) { + if ( + ZSTR_LEN(new_value) == 0 + || zend_str_has_nul_byte(new_value) + || is_numeric_str_function(new_value, NULL, NULL) + || strpbrk(ZSTR_VAL(new_value), SESSION_FORBIDDEN_CHARS) != NULL + ) { int err_type; if (stage == ZEND_INI_STAGE_RUNTIME || stage == ZEND_INI_STAGE_ACTIVATE || stage == ZEND_INI_STAGE_STARTUP) { @@ -694,7 +700,7 @@ static PHP_INI_MH(OnUpdateName) /* {{{ */ /* Do not output error when restoring ini options. */ if (stage != ZEND_INI_STAGE_DEACTIVATE) { - php_error_docref(NULL, err_type, "session.name \"%s\" cannot be numeric or empty", ZSTR_VAL(new_value)); + php_error_docref(NULL, err_type, "session.name \"%s\" must not be numeric, empty, contain null bytes or any of the following characters \"" SESSION_FORBIDDEN_CHARS_FOR_ERROR_MSG "\"", ZSTR_VAL(new_value)); } return FAILURE; } @@ -1338,11 +1344,7 @@ static zend_result php_session_send_cookie(void) /* {{{ */ return FAILURE; } - /* Prevent broken Set-Cookie header, because the session_name might be user supplied */ - if (strpbrk(PS(session_name), SESSION_FORBIDDEN_CHARS) != NULL) { /* man isspace for \013 and \014 */ - php_error_docref(NULL, E_WARNING, "session.name cannot contain any of the following '=,;.[ \\t\\r\\n\\013\\014'"); - return FAILURE; - } + ZEND_ASSERT(strpbrk(PS(session_name), SESSION_FORBIDDEN_CHARS) == NULL); /* URL encode id because it might be user supplied */ e_id = php_url_encode(ZSTR_VAL(PS(id)), ZSTR_LEN(PS(id))); @@ -1462,7 +1464,10 @@ PHPAPI zend_result php_session_reset_id(void) /* {{{ */ } if (PS(use_cookies) && PS(send_cookie)) { - php_session_send_cookie(); + zend_result cookies_sent = php_session_send_cookie(); + if (UNEXPECTED(cookies_sent == FAILURE)) { + return FAILURE; + } PS(send_cookie) = 0; } diff --git a/ext/session/tests/bug66481.phpt b/ext/session/tests/bug66481.phpt index 88c2e48ed7999..26a31279fbed2 100644 --- a/ext/session/tests/bug66481.phpt +++ b/ext/session/tests/bug66481.phpt @@ -15,6 +15,6 @@ var_dump(session_name("foo")); var_dump(session_name("bar")); ?> --EXPECT-- -Warning: PHP Startup: session.name "" cannot be numeric or empty in Unknown on line 0 +Warning: PHP Startup: session.name "" must not be numeric, empty, contain null bytes or any of the following characters "=,;.[ \t\r\n\013\014" in Unknown on line 0 string(9) "PHPSESSID" string(3) "foo" diff --git a/ext/session/tests/gh17541.phpt b/ext/session/tests/gh17541.phpt new file mode 100644 index 0000000000000..bbf6a50393b5e --- /dev/null +++ b/ext/session/tests/gh17541.phpt @@ -0,0 +1,24 @@ +--TEST-- +GH-17541 (ext/session NULL pointer dereferencement during ID reset) +--EXTENSIONS-- +session +--SKIPIF-- + +--FILE-- + +--EXPECTF-- +Warning: session_destroy(): Trying to destroy uninitialized session in %s on line %d +string(9) "PHPSESSID" +bool(true) diff --git a/ext/session/tests/session_name_variation1.phpt b/ext/session/tests/session_name_variation1.phpt index 07fb7f10ca89b..a298e643c0490 100644 --- a/ext/session/tests/session_name_variation1.phpt +++ b/ext/session/tests/session_name_variation1.phpt @@ -38,25 +38,25 @@ ob_end_flush(); ?> --EXPECTF-- *** Testing session_name() : variation *** + +Warning: session_name(): session.name "" must not be numeric, empty, contain null bytes or any of the following characters "=,;.[ \t\r\n\013\014" in %s on line %d string(9) "PHPSESSID" bool(true) string(9) "PHPSESSID" bool(true) string(9) "PHPSESSID" -string(9) "PHPSESSID" -Warning: session_start(): session.name cannot contain any of the following '=,;.[ \t\r\n\013\014' in %s on line %d +Warning: session_name(): session.name " " must not be numeric, empty, contain null bytes or any of the following characters "=,;.[ \t\r\n\013\014" in %s on line %d +string(9) "PHPSESSID" bool(true) -string(1) " " +string(9) "PHPSESSID" bool(true) -string(1) " " - -Warning: session_name(): session.name "" cannot be numeric or empty in %s on line %d -string(1) " " +string(9) "PHPSESSID" -Warning: session_start(): session.name cannot contain any of the following '=,;.[ \t\r\n\013\014' in %s on line %d +Warning: session_name(): session.name "" must not be numeric, empty, contain null bytes or any of the following characters "=,;.[ \t\r\n\013\014" in %s on line %d +string(9) "PHPSESSID" bool(true) -string(1) " " +string(9) "PHPSESSID" bool(true) -string(1) " " +string(9) "PHPSESSID" Done From bd021416598f5eb674c4097f1bd8b1ba28162d53 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Fri, 24 Jan 2025 14:26:11 +0000 Subject: [PATCH 5/7] Add into zval to GC buffer first --- ext/pdo/pdo_stmt.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index dc0b952a86f18..18ecbf1ddc63b 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -2027,8 +2027,11 @@ static zend_function *dbstmt_method_get(zend_object **object_pp, zend_string *me static HashTable *dbstmt_get_gc(zend_object *object, zval **gc_data, int *gc_count) { pdo_stmt_t *stmt = php_pdo_stmt_fetch_object(object); - *gc_data = &stmt->fetch.into; - *gc_count = 1; + + zend_get_gc_buffer *gc_buffer = zend_get_gc_buffer_create(); + zend_get_gc_buffer_add_zval(gc_buffer, &stmt->fetch.into); + zend_get_gc_buffer_add_zval(gc_buffer, &stmt->database_object_handle); + zend_get_gc_buffer_use(gc_buffer, gc_data, gc_count); /** * If there are no dynamic properties and the default property is 1 (that is, there is only one property From b2480c3b554cb95114aebc65ecbd96045918c464 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Fri, 24 Jan 2025 14:26:42 +0000 Subject: [PATCH 6/7] Revert "Add into zval to GC buffer first" I thought I was on a branch and not master oopsie This reverts commit bd021416598f5eb674c4097f1bd8b1ba28162d53. --- ext/pdo/pdo_stmt.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index 18ecbf1ddc63b..dc0b952a86f18 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -2027,11 +2027,8 @@ static zend_function *dbstmt_method_get(zend_object **object_pp, zend_string *me static HashTable *dbstmt_get_gc(zend_object *object, zval **gc_data, int *gc_count) { pdo_stmt_t *stmt = php_pdo_stmt_fetch_object(object); - - zend_get_gc_buffer *gc_buffer = zend_get_gc_buffer_create(); - zend_get_gc_buffer_add_zval(gc_buffer, &stmt->fetch.into); - zend_get_gc_buffer_add_zval(gc_buffer, &stmt->database_object_handle); - zend_get_gc_buffer_use(gc_buffer, gc_data, gc_count); + *gc_data = &stmt->fetch.into; + *gc_count = 1; /** * If there are no dynamic properties and the default property is 1 (that is, there is only one property From 3e6f4702ba2a1e58561b62263a63436d59a4a463 Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Fri, 24 Jan 2025 15:43:22 +0100 Subject: [PATCH 7/7] Fix GHA config yml error --- .github/workflows/nightly.yml | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index c2816d72cf18e..4c8ec23b158a4 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -427,7 +427,7 @@ jobs: -d zend_extension=opcache.so -d opcache.enable_cli=1 - uses: codecov/codecov-action@v4 - if: !cancelled() + if: ${{ !cancelled() }} with: fail_ci_if_error: true token: ${{ secrets.CODECOV_TOKEN }} @@ -500,7 +500,7 @@ jobs: echo opcache.jit_hot_side_exit=1 >> /etc/php.d/opcache.ini php -v - name: Test AMPHP - if: !cancelled() + if: ${{ !cancelled() }} run: | repositories="amp cache dns file http parallel parser pipeline process serialization socket sync websocket-client websocket-server" X=0 @@ -518,7 +518,7 @@ jobs: done exit $X - name: Test Laravel - if: !cancelled() + if: ${{ !cancelled() }} run: | git clone https://github.com/laravel/framework.git --branch=master --depth=1 cd framework @@ -531,7 +531,7 @@ jobs: exit 1 fi - name: Test ReactPHP - if: !cancelled() + if: ${{ !cancelled() }} run: | repositories="async cache child-process datagram dns event-loop promise promise-stream promise-timer stream" X=0 @@ -549,7 +549,7 @@ jobs: done exit $X - name: Test Revolt PHP - if: !cancelled() + if: ${{ !cancelled() }} run: | git clone https://github.com/revoltphp/event-loop.git --depth=1 cd event-loop @@ -560,7 +560,7 @@ jobs: exit 1 fi - name: Test Symfony - if: !cancelled() && !inputs.skip_symfony + if: ${{ !cancelled() && !inputs.skip_symfony }} run: | git clone https://github.com/symfony/symfony.git --depth=1 cd symfony @@ -581,7 +581,7 @@ jobs: done exit $X - name: Test PHPUnit - if: !cancelled() + if: ${{ !cancelled() }} run: | git clone https://github.com/sebastianbergmann/phpunit.git --branch=main --depth=1 cd phpunit @@ -592,7 +592,7 @@ jobs: exit 1 fi - name: 'Symfony Preloading' - if: !cancelled() && !inputs.skip_symfony + if: ${{ !cancelled() && !inputs.skip_symfony }} run: | php /usr/bin/composer create-project symfony/symfony-demo symfony_demo --no-progress --ignore-platform-reqs cd symfony_demo @@ -600,7 +600,7 @@ jobs: sed -i 's/PHP_SAPI/"cli-server"/g' var/cache/dev/App_KernelDevDebugContainer.preload.php php -d opcache.preload=var/cache/dev/App_KernelDevDebugContainer.preload.php public/index.php - name: Test Wordpress - if: !cancelled() && !inputs.skip_wordpress + if: ${{ !cancelled() && !inputs.skip_wordpress }} run: | git clone https://github.com/WordPress/wordpress-develop.git wordpress --depth=1 cd wordpress