Skip to content

Commit f47bf64

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 phpGH-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.
1 parent 4aac98f commit f47bf64

File tree

4 files changed

+29
-4
lines changed

4 files changed

+29
-4
lines changed

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)