Skip to content

Commit 405be1c

Browse files
committed
Fix phar crash and file corruption with SplFileObject
There are two bugfixes here. The first was a crash that I discovered while working on GH-19035. The check for when a file pointer was still occupied was wrong, leading to a UAF. Strangely, zip got this right. The second issue was that even after fixing the first one, the file contents were garbage. This is because the file write offset for the phar stream was wrong. Closes GH-19038.
1 parent 32344c4 commit 405be1c

File tree

5 files changed

+30
-4
lines changed

5 files changed

+30
-4
lines changed

NEWS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ PHP NEWS
3939

4040
- Phar:
4141
. Fix stream double free in phar. (nielsdos, dixyes)
42+
. Fix phar crash and file corruption with SplFileObject. (nielsdos)
4243

4344
- SOAP:
4445
. Fixed bug GH-18990, bug #81029, bug #47314 (SOAP HTTP socket not closing

ext/phar/phar.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2737,7 +2737,7 @@ int phar_flush(phar_archive_data *phar, char *user_stub, zend_long len, int conv
27372737
/* remove this from the new phar */
27382738
continue;
27392739
}
2740-
if (!entry->is_modified && entry->fp_refcount) {
2740+
if (entry->fp_refcount) {
27412741
/* open file pointers refer to this fp, do not free the stream */
27422742
switch (entry->fp_type) {
27432743
case PHAR_FP:

ext/phar/stream.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -450,12 +450,12 @@ static ssize_t phar_stream_write(php_stream *stream, const char *buf, size_t cou
450450
{
451451
phar_entry_data *data = (phar_entry_data *) stream->abstract;
452452

453-
php_stream_seek(data->fp, data->position, SEEK_SET);
453+
php_stream_seek(data->fp, data->position + data->zero, SEEK_SET);
454454
if (count != php_stream_write(data->fp, buf, count)) {
455455
php_stream_wrapper_log_error(stream->wrapper, stream->flags, "phar error: Could not write %d characters to \"%s\" in phar \"%s\"", (int) count, data->internal_file->filename, data->phar->fname);
456456
return -1;
457457
}
458-
data->position = php_stream_tell(data->fp);
458+
data->position = php_stream_tell(data->fp) - data->zero;
459459
if (data->position > (zend_off_t)data->internal_file->uncompressed_filesize) {
460460
data->internal_file->uncompressed_filesize = data->position;
461461
}

ext/phar/tar.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -832,7 +832,7 @@ static int phar_tar_writeheaders_int(phar_entry_info *entry, void *argument) /*
832832
php_stream_write(fp->new, padding, ((entry->uncompressed_filesize +511)&~511) - entry->uncompressed_filesize);
833833
}
834834

835-
if (!entry->is_modified && entry->fp_refcount) {
835+
if (entry->fp_refcount) {
836836
/* open file pointers refer to this fp, do not free the stream */
837837
switch (entry->fp_type) {
838838
case PHAR_FP:

ext/phar/tests/gh19038.phpt

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
--TEST--
2+
GH-19038 (Phar crash and data corruption with SplFileObject)
3+
--EXTENSIONS--
4+
phar
5+
--INI--
6+
phar.readonly=0
7+
--FILE--
8+
<?php
9+
10+
$phar = new Phar(__DIR__ . "/gh19038.phar");
11+
$phar->addFromString("file", "123");
12+
$file = $phar["file"]->openFile();
13+
$file->fseek(3);
14+
var_dump($file->fwrite("456", 3));
15+
$file->fseek(0);
16+
echo $file->fread(100);
17+
18+
?>
19+
--CLEAN--
20+
<?php
21+
@unlink(__DIR__ . "/gh19038.phar");
22+
?>
23+
--EXPECT--
24+
int(3)
25+
123456

0 commit comments

Comments
 (0)