Skip to content

Commit 225fa9e

Browse files
committed
Fix GH-21700: assertion failure in _php_stream_seek with user stream returning negative position
When a user stream's stream_tell() returns a negative offset after a stream_seek() reported success, _php_stream_seek would store that value into stream->position and then trip ZEND_ASSERT(stream->position >= 0) on the next SEEK_CUR. Three changes: - After the SEEK_CUR -> SEEK_SET conversion, fall through to the existing 'offset < 0' rejection so a SEEK_CUR whose absolute target would be negative is refused before invoking the user seek hook. - After stream->ops->seek returns success, validate stream->position is non-negative. If a user stream lied (e.g. stream_tell() returns -42), restore the prior position and return -1, matching POSIX fseek()'s contract that the file-position indicator is unchanged on failure. - In php_stream_open_wrapper_ex's append-mode position fixup, ignore a negative newpos returned by stream->ops->seek (defense in depth for the only other direct caller of the seek hook in this layer). Includes phpt regression tests covering both _php_stream_seek paths and the append-mode open path.
1 parent 1462499 commit 225fa9e

4 files changed

Lines changed: 94 additions & 5 deletions

File tree

NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,8 @@ PHP NEWS
195195
. Allowed filtered streams to be casted as fd for select. (Jakub Zelenka)
196196
. Fixed bug GH-21221 (Prevent closing of innerstream of php://temp stream).
197197
(ilutov)
198+
. Fixed bug GH-21700 (Assertion failure in _php_stream_seek when a user
199+
stream returns a negative position). (lacatoire)
198200
. Improved stream_socket_server() bind failure error reporting. (ilutov)
199201
. Fixed bug #49874 (ftell() and fseek() inconsistency when using stream
200202
filters). (Jakub Zelenka)
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
--TEST--
2+
GH-21700: Append-mode open ignores a negative position returned by a user stream
3+
--FILE--
4+
<?php
5+
class lyingstream {
6+
public function stream_open($path, $mode, $options, &$opened_path) { return true; }
7+
public function stream_seek($offset, $whence) { return true; }
8+
public function stream_tell(): int { return -42; }
9+
public function stream_eof() { return false; }
10+
public function stream_read($count) { return ''; }
11+
}
12+
13+
stream_wrapper_register("test", "lyingstream");
14+
15+
// Opening in append mode triggers an internal SEEK_CUR to revise the initial
16+
// position; if the user stream lies, ftell() must still report a non-negative
17+
// value and a subsequent SEEK_CUR must not trip the position invariant.
18+
$fp = fopen("test://foo", "ab");
19+
var_dump(ftell($fp));
20+
var_dump(fseek($fp, 0, SEEK_CUR));
21+
fclose($fp);
22+
23+
stream_wrapper_unregister("test");
24+
?>
25+
--EXPECT--
26+
int(0)
27+
int(-1)
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
--TEST--
2+
GH-21700: User stream returning negative position must not trigger an assertion
3+
--FILE--
4+
<?php
5+
class mystream {
6+
public int $pos = 0;
7+
public function stream_open($path, $mode, $options, &$opened_path) { return true; }
8+
public function stream_seek($offset, $whence) {
9+
// Blindly store the offset, mimicking a buggy user implementation.
10+
$this->pos = $offset;
11+
return true;
12+
}
13+
public function stream_tell() { return $this->pos; }
14+
public function stream_eof() { return false; }
15+
public function stream_read($count) { return ''; }
16+
}
17+
18+
class lyingstream extends mystream {
19+
public function stream_tell(): int { return -42; }
20+
}
21+
22+
// 1. SEEK_CUR with a negative resulting absolute position is rejected
23+
// before invoking the user seek hook, so stream->position stays valid.
24+
stream_wrapper_register("test", "mystream");
25+
$fp = fopen("test://foo", "rb");
26+
var_dump(fseek($fp, -1, SEEK_CUR));
27+
var_dump(ftell($fp));
28+
var_dump(fseek($fp, 0, SEEK_CUR));
29+
fclose($fp);
30+
stream_wrapper_unregister("test");
31+
32+
// 2. A user stream_tell() returning a negative position is rejected,
33+
// keeping stream->position non-negative.
34+
stream_wrapper_register("test", "lyingstream");
35+
$fp = fopen("test://foo", "rb");
36+
var_dump(fseek($fp, 100, SEEK_SET));
37+
var_dump(ftell($fp));
38+
var_dump(fseek($fp, 0, SEEK_CUR));
39+
fclose($fp);
40+
stream_wrapper_unregister("test");
41+
?>
42+
--EXPECT--
43+
int(-1)
44+
int(0)
45+
int(0)
46+
int(-1)
47+
int(0)
48+
int(-1)

main/streams/streams.c

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1466,19 +1466,31 @@ PHPAPI int _php_stream_seek(php_stream *stream, zend_off_t offset, int whence)
14661466
} else {
14671467
offset = stream->position + offset;
14681468
}
1469-
whence = SEEK_SET;
1470-
break;
1469+
whence = SEEK_SET;
1470+
ZEND_FALLTHROUGH;
14711471
case SEEK_SET:
14721472
if (offset < 0) {
14731473
return -1;
14741474
}
1475+
break;
14751476
}
1477+
zend_off_t saved_position = stream->position;
14761478
ret = stream->ops->seek(stream, offset, whence, &stream->position);
14771479

14781480
if (((stream->flags & PHP_STREAM_FLAG_NO_SEEK) == 0) || ret == 0) {
14791481
if (ret == 0) {
1480-
stream->eof = 0;
1481-
stream->fatal_error = 0;
1482+
if (UNEXPECTED(stream->position < 0)) {
1483+
/* The seek operation reported success but updated
1484+
* stream->position to a negative value (e.g. a user
1485+
* stream's stream_tell() returned a negative offset).
1486+
* Treat as failure and restore the original position
1487+
* to keep the invariant stream->position >= 0. */
1488+
stream->position = saved_position;
1489+
ret = -1;
1490+
} else {
1491+
stream->eof = 0;
1492+
stream->fatal_error = 0;
1493+
}
14821494
}
14831495

14841496
/* invalidate the buffer contents */
@@ -2392,7 +2404,7 @@ PHPAPI php_stream *_php_stream_open_wrapper_ex(const char *path, const char *mod
23922404
zend_off_t newpos = 0;
23932405

23942406
/* if opened for append, we need to revise our idea of the initial file position */
2395-
if (0 == stream->ops->seek(stream, 0, SEEK_CUR, &newpos)) {
2407+
if (0 == stream->ops->seek(stream, 0, SEEK_CUR, &newpos) && newpos >= 0) {
23962408
stream->position = newpos;
23972409
}
23982410
}

0 commit comments

Comments
 (0)