diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index 4041271f4d041..4c8ec23b158a4 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: @@ -421,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 }} @@ -494,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 @@ -512,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 @@ -525,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 @@ -543,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 @@ -554,7 +560,7 @@ jobs: exit 1 fi - name: Test Symfony - if: always() + if: ${{ !cancelled() && !inputs.skip_symfony }} run: | git clone https://github.com/symfony/symfony.git --depth=1 cd symfony @@ -575,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 @@ -586,7 +592,7 @@ jobs: exit 1 fi - name: 'Symfony Preloading' - if: always() + 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 @@ -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: ${{ !cancelled() && !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 diff --git a/ext/session/session.c b/ext/session/session.c index bb34567dd5dae..ae6576856e48e 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)) @@ -705,7 +706,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) { @@ -716,7 +722,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; } @@ -1430,11 +1436,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))); @@ -1554,7 +1556,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 ef68b80b3def9..0bd4dcf21a8a5 100644 --- a/ext/session/tests/session_name_variation1.phpt +++ b/ext/session/tests/session_name_variation1.phpt @@ -32,20 +32,18 @@ ob_end_flush(); ?> --EXPECTF-- *** Testing session_name() : variation *** -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