[php-src] PR #24677: Fix GH-24493: preserve session data when upload-progress decoding fails
| From: | programsurf | Date: | Sun, 25 Oct 2026 11:07:51 +0000 |
| Subject: | [php-src] PR #24677: Fix GH-24493: preserve session data when upload-progress decoding fails | ||
| Groups: | php.git-pulls | ||
| Request: | Send a blank email to git-pulls+get-39855@lists.php.net to get a copy of this message | ||
Pull Request: https://github.com/php/php-src/pull/24677
Author: programsurf
Fix GH-24493: preserve session data when upload-progress decoding fails
================================================================
A valid session written with
session_start() / session_write_close() may
contain an ordinary object property named __PHP_Incomplete_Class_Name,
__PHP_Incomplete_Class_Serialized, or __PHP_Incomplete_Enum_Case. During a
multipart upload, PHP 8.5.12's restricted pre-RINIT decoder rejects those names. The shared
decode-failure path then destroys the entire stored session, including unrelated application data.
This change aborts only the upload-progress session update when decoding fails in
in_rfc1867_update. It closes the session handler without writing or deleting the stored
record. The normal application request can subsequently decode its session with the regular decoder.
The reserved-member checks and incomplete_classes_only restriction remain in place.
Ordinary session_start() decode failures keep their existing behavior. For a record the
restricted decoder cannot handle, progress updates are skipped; the upload itself and the stored
application data are preserved.
Changes
-------
- Add the upload-progress-specific abort path in php_session_cancel_decode().
- Update the existing reserved-property regression test to require stored-record preservation.
- Add gh24493_upload_progress_session_roundtrip.phpt: sessions are produced through the
public PHP API, then uploaded with the same cookie and read back. The test covers all three
serializers, three reserved names plus a normal-name control, and both cleanup settings (24
combinations).
- Check upload success, no premature $_SESSION exposure, independent stored literals,
object alias identity, and byte-identical stored records immediately after rejected progress
updates.
Validation
----------
PR base: PHP-8.4, commit 055e11e0c7e6fe5a9649afc2b79391608933ee79. On that
branch, the same frozen normal suites pass 40/40 twice after repair (official build: Blue 18/20, Red
10/20). The full session suite has 267 passes, one existing expected failure, one skip requiring
unavailable posix/pcntl extensions, and no unexpected failures. The reserved-property and
no-user-code regression tests pass.
Additional validation against PHP 8.5.12, commit
136e049ac10b032c83018e65b63b6df858943750, using Clang/AddressSanitizer and real
PHP-FPM/CGI execution:
- Frozen Daybreak Blue normal suite: official build 18/20, repaired build 20/20.
- Frozen Daybreak Red normal suite: official build 10/20, repaired build 20/20.
- Both suites repeated twice with unchanged inputs and expected results.
- Original stale-fatal-backtrace PoC: vulnerable control reproduces UAF twice; official and repaired
builds do not reproduce it in the same replay.
- Retained session-loss witness: official build loses the stored values twice; repaired build
retains them twice.
- Full ext/session/tests: 269 passed, one pre-existing expected failure
(bug71162.phpt), zero unexpected failures, zero skips.
- New and updated regression tests both fail on the unmodified official build (negative control).
- Existing rfc1867_no_user_code.phpt passes, including raw C:/E: payload preservation
and callback timing checks.
Prior work and attribution
--------------------------
This is the known regression in [php/php-src#24493](https://github.com/php/php-src/issues/24493),
not a new vulnerability discovery. The abort-update approach also appears in [Lazizbek Ergashev
(lazerg)'s commit
a45ffe71](https://github.com/lazerg/php-src/commit/a45ffe71a56741579f648d969193bd027a783e36). This
local proposal acknowledges that implementation and adds a public-API round-trip matrix and
independent validation evidence. The additional public-API regression matrix and execution results
can also be incorporated into that existing fix if preferred.
The attached results concern this source version and these executions. The skipped progress update
is an intentional fallback for session records unsupported by the restricted startup decoder.