Skip to content

Commit 0406a55

Browse files
committed
Prevent resumption of generator suspended in yield from
Normally we prevent generators from being resumed while they are already running, but we failed to do so for generators delegating to non-Generators. As a result such generator can be resumed, terminated, which causes unexpected results (crashes) later. In gh19306.phpt in particular, the generator delegate It::getIterator() suspends while being called by generator g(). We then resume g(), which throws while trying to resume It::getIterator(). This causes g() and It::getIterator() to be released. We then UAF when resuming the Fiber in It::getIterator(). Fix this by ensuring that generators are marked as running while they fetch the next value from the delegate. Fixes GH-19306 Closes GH-19315
1 parent 5bd5f35 commit 0406a55

File tree

3 files changed

+45
-2
lines changed

3 files changed

+45
-2
lines changed

NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ PHP NEWS
1313
(Arnaud)
1414
. Fixed bug GH-19303 (Unpacking empty packed array into uninitialized array
1515
causes assertion failure). (nielsdos)
16+
. Fixed bug GH-19306 (Generator can be resumed while fetching next value from
17+
delegated Generator). (Arnaud)
1618

1719
- FTP:
1820
. Fix theoretical issues with hrtime() not being available. (nielsdos)

Zend/tests/gh19306.phpt

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
--TEST--
2+
GH-19306: Generator suspended in yield from may be resumed
3+
--FILE--
4+
<?php
5+
6+
class It implements IteratorAggregate
7+
{
8+
public function getIterator(): Generator
9+
{
10+
yield "";
11+
Fiber::suspend();
12+
}
13+
}
14+
function g()
15+
{
16+
yield from new It();
17+
}
18+
$a = g();
19+
$fiber = new Fiber(function () use ($a) {
20+
echo "Fiber start\n";
21+
$a->next();
22+
echo "Fiber return\n";
23+
});
24+
$fiber->start();
25+
echo "Fiber suspended\n";
26+
try {
27+
$a->next();
28+
} catch (Throwable $t) {
29+
echo $t->getMessage(), "\n";
30+
}
31+
echo "Destroying fiber\n";
32+
$fiber = null;
33+
echo "Shutdown\n";
34+
?>
35+
--EXPECT--
36+
Fiber start
37+
Fiber suspended
38+
Cannot resume an already running generator
39+
Destroying fiber
40+
Shutdown

Zend/zend_generators.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -787,6 +787,8 @@ ZEND_API void zend_generator_resume(zend_generator *orig_generator) /* {{{ */
787787
orig_generator->execute_fake.prev_execute_data = original_execute_data;
788788
}
789789

790+
generator->flags |= ZEND_GENERATOR_CURRENTLY_RUNNING;
791+
790792
/* Ensure this is run after executor_data swap to have a proper stack trace */
791793
if (UNEXPECTED(!Z_ISUNDEF(generator->values))) {
792794
if (EXPECTED(zend_generator_get_next_delegated_value(generator) == SUCCESS)) {
@@ -795,7 +797,7 @@ ZEND_API void zend_generator_resume(zend_generator *orig_generator) /* {{{ */
795797
EG(jit_trace_num) = original_jit_trace_num;
796798

797799
orig_generator->flags &= ~(ZEND_GENERATOR_DO_INIT | ZEND_GENERATOR_IN_FIBER);
798-
generator->flags &= ~ZEND_GENERATOR_IN_FIBER;
800+
generator->flags &= ~(ZEND_GENERATOR_CURRENTLY_RUNNING | ZEND_GENERATOR_IN_FIBER);
799801
return;
800802
}
801803
if (UNEXPECTED(EG(exception))) {
@@ -817,7 +819,6 @@ ZEND_API void zend_generator_resume(zend_generator *orig_generator) /* {{{ */
817819
}
818820

819821
/* Resume execution */
820-
generator->flags |= ZEND_GENERATOR_CURRENTLY_RUNNING;
821822
if (!ZEND_OBSERVER_ENABLED) {
822823
zend_execute_ex(generator->execute_data);
823824
} else {

0 commit comments

Comments
 (0)