Opened 3 days ago
Closed 2 days ago
#66408 closed defect (bug) (fixed)
PclZip: fatal TypeError in privExtractFileUsingTempFile() when the temporary file cannot be created (PHP 8)
| Reported by: | ExtremsX | Owned by: | SergeyBiryukov |
|---|---|---|---|
| Priority: | normal | Milestone: | 7.2 |
| Component: | Filesystem API | Version: | trunk |
| Severity: | normal | Keywords: | has-patch |
| Cc: | Focuses: |
Description
In wp-admin/includes/class-pclzip.php, PclZip::privExtractFileUsingTempFile() closes a variable that was never opened when it fails to create its temporary file:
<?php $v_gzip_temp_name = PCLZIP_TEMPORARY_DIR.uniqid('pclzip-').'.gz'; if (($v_dest_file = @fopen($v_gzip_temp_name, "wb")) == 0) { fclose($v_file); // $v_file is not defined in this function PclZip::privErrorLog(PCLZIP_ERR_WRITE_OPEN_FAIL, 'Unable to open temporary file ...'); return PclZip::errorCode(); }
On PHP 7 this was a notice plus a warning. On PHP 8, fclose(null) throws a TypeError, so instead of returning PCLZIP_ERR_WRITE_OPEN_FAIL the request dies with a fatal error.
Steps to reproduce (PHP 8.2, trunk, standalone):
<?php define( 'PCLZIP_TEMPORARY_DIR', '/path/that/does/not/exist/' ); require 'wp-admin/includes/class-pclzip.php'; file_put_contents( '/tmp/data.txt', str_repeat( "test line\n", 2000 ) ); $zip = new PclZip( '/tmp/test.zip' ); $zip->create( '/tmp/data.txt', PCLZIP_OPT_REMOVE_ALL_PATH ); $zip->extract( PCLZIP_OPT_PATH, '/tmp/out', PCLZIP_OPT_TEMP_FILE_ON );
Result:
Warning: Undefined variable $v_file in .../class-pclzip.php on line 3984 Fatal error: Uncaught TypeError: fclose(): Argument #1 ($stream) must be of type resource, null given
Expected: extract() returns 0 and errorInfo() reports PCLZIP_ERR_WRITE_OPEN_FAIL.
Impact is low: core's own _unzip_file_pclzip() uses PCLZIP_OPT_EXTRACT_AS_STRING, which does not go through this function. It is reachable by plugins that use the bundled PclZip to extract to disk, either with PCLZIP_OPT_TEMP_FILE_ON or with an entry above the temp-file threshold, when the temporary directory is not writable. Note that class-pclzip.php is excluded from PHPStan (tests/phpstan/base.neon), which is probably why it was not caught there.
Suggested fix: nothing has been opened at that point, so the fclose() can simply be removed:
if (($v_dest_file = @fopen($v_gzip_temp_name, "wb")) == 0) { - fclose($v_file); PclZip::privErrorLog(PCLZIP_ERR_WRITE_OPEN_FAIL, 'Unable to open temporary file \''.$v_gzip_temp_name.'\' in binary write mode'); return PclZip::errorCode(); }
Found with the help of a static analysis tool I'm building (code-intruder) and AI review; reproduced manually on PHP 8.2.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
PclZip::privExtractFileUsingTempFile()callsfclose( $v_file )when itfails to open its temporary file, but
$v_fileis not defined in thatfunction. On PHP 8,
fclose( null )throws aTypeError, so the requestdies with a fatal error instead of
extract()returning 0 withPCLZIP_ERR_WRITE_OPEN_FAIL. Nothing has been opened at that point, so thecall is removed.
The similar
fclose( $v_file )inprivAddFileUsingTempFile()is correct(the file is opened a few lines above) and is left unchanged.
Tested on PHP 8.2 with the reproduction script from the ticket: before, a
fatal
TypeErrorat line 3984; after,extract()returns 0 anderrorInfo()reportsPCLZIP_ERR_WRITE_OPEN_FAIL.Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code, with code-intruder (my own static analysis tool)
Model(s): Claude Opus
Used for: code-intruder flagged
$v_fileas read without being assignedin that function. AI helped reproduce it on PHP 8.2, check that core's own
_unzip_file_pclzip()does not reach this path, and verify the fix.I reviewed and tested the change myself.