Skip to content

Commit 61884c3

Browse files
committed
Fix GH-18642: Signed integer overflow in ext/phar fseek
The overflow checking code already existed, but didn't work because the math was done on signed numbers instead of unsigned numbers. In the process I also discovered a pre-existing issue that needs to be fixed (and seems that other stream wrappers can have this issue too). Closes GH-18644.
1 parent fab0a6d commit 61884c3

File tree

3 files changed

+41
-9
lines changed

3 files changed

+41
-9
lines changed

NEWS

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

1111
- Phar:
1212
. Add missing filter cleanups on phar failure. (nielsdos)
13+
. Fixed bug GH-18642 (Signed integer overflow in ext/phar fseek). (nielsdos)
1314

1415
- Readline:
1516
. Fix memory leak when calloc() fails in php_readline_completion_cb().

ext/phar/stream.c

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,7 @@ static int phar_stream_seek(php_stream *stream, zend_off_t offset, int whence, z
405405
phar_entry_data *data = (phar_entry_data *)stream->abstract;
406406
phar_entry_info *entry;
407407
int res;
408-
zend_off_t temp;
408+
zend_ulong temp;
409409

410410
if (data->internal_file->link) {
411411
entry = phar_get_link_source(data->internal_file);
@@ -415,26 +415,28 @@ static int phar_stream_seek(php_stream *stream, zend_off_t offset, int whence, z
415415

416416
switch (whence) {
417417
case SEEK_END :
418-
temp = data->zero + entry->uncompressed_filesize + offset;
418+
temp = (zend_ulong) data->zero + (zend_ulong) entry->uncompressed_filesize + (zend_ulong) offset;
419419
break;
420420
case SEEK_CUR :
421-
temp = data->zero + data->position + offset;
421+
temp = (zend_ulong) data->zero + (zend_ulong) data->position + (zend_ulong) offset;
422422
break;
423423
case SEEK_SET :
424-
temp = data->zero + offset;
424+
temp = (zend_ulong) data->zero + (zend_ulong) offset;
425425
break;
426426
default:
427427
temp = 0;
428428
}
429-
if (temp > data->zero + (zend_off_t) entry->uncompressed_filesize) {
430-
*newoffset = -1;
429+
430+
zend_off_t temp_signed = (zend_off_t) temp;
431+
if (temp_signed > data->zero + (zend_off_t) entry->uncompressed_filesize) {
432+
*newoffset = -1; /* FIXME: this will invalidate the ZEND_ASSERT(stream->position >= 0); assertion in streams.c */
431433
return -1;
432434
}
433-
if (temp < data->zero) {
434-
*newoffset = -1;
435+
if (temp_signed < data->zero) {
436+
*newoffset = -1; /* FIXME: this will invalidate the ZEND_ASSERT(stream->position >= 0); assertion in streams.c */
435437
return -1;
436438
}
437-
res = php_stream_seek(data->fp, temp, SEEK_SET);
439+
res = php_stream_seek(data->fp, temp_signed, SEEK_SET);
438440
*newoffset = php_stream_tell(data->fp) - data->zero;
439441
data->position = *newoffset;
440442
return res;

ext/phar/tests/gh18642.phpt

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
--TEST--
2+
GH-18642 (Signed integer overflow in ext/phar fseek)
3+
--EXTENSIONS--
4+
phar
5+
--INI--
6+
phar.require_hash=0
7+
--FILE--
8+
<?php
9+
require_once __DIR__ . '/files/phar_oo_test.inc';
10+
class MyFile extends SplFileObject
11+
{
12+
}
13+
$phar = new Phar($fname);
14+
$phar->setInfoClass('MyFile');
15+
$f = $phar['a.php'];
16+
var_dump($f->fseek(PHP_INT_MAX));
17+
var_dump($f->fseek(0));
18+
var_dump($f->fseek(PHP_INT_MIN, SEEK_END));
19+
var_dump($f->fseek(0, SEEK_SET));
20+
var_dump($f->fseek(1, SEEK_CUR));
21+
var_dump($f->fseek(PHP_INT_MAX, SEEK_CUR));
22+
?>
23+
--EXPECT--
24+
int(-1)
25+
int(0)
26+
int(-1)
27+
int(0)
28+
int(0)
29+
int(-1)

0 commit comments

Comments
 (0)