Skip to content

Commit 127ea1e

Browse files
committed
ext/session: do not reuse session state a save handler tore down
session_regenerate_id() runs the userland write or destroy handler and then the close handler, and keeps operating on PS(id) afterwards. Any of them may call session_destroy(): php_session_destroy() runs php_rshutdown_session_globals() even when the recursive call was rejected, releasing PS(id) and setting it to NULL, so the following zend_string_release_ex() dereferences NULL. Check that the session is still active once every handler has run. Closes GH-22926
1 parent 2804d09 commit 127ea1e

3 files changed

Lines changed: 110 additions & 0 deletions

File tree

ext/session/session.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2408,8 +2408,14 @@ PHP_FUNCTION(session_regenerate_id)
24082408
RETURN_FALSE;
24092409
}
24102410
}
2411+
24112412
PS(mod)->s_close(&PS(mod_data));
24122413

2414+
if (PS(session_status) != php_session_active) {
2415+
php_error_docref(NULL, E_WARNING, "Session ID cannot be regenerated because the save handler closed the session");
2416+
RETURN_FALSE;
2417+
}
2418+
24132419
/* New session data */
24142420
if (PS(session_vars)) {
24152421
zend_string_release_ex(PS(session_vars), 0);
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
--TEST--
2+
session_regenerate_id() when the close handler destroys the session
3+
--INI--
4+
session.save_handler=files
5+
session.name=PHPSESSID
6+
session.gc_probability=0
7+
--EXTENSIONS--
8+
session
9+
--FILE--
10+
<?php
11+
12+
ob_start();
13+
14+
class MySessionHandler implements SessionHandlerInterface
15+
{
16+
private bool $destroyed = false;
17+
18+
public function open(string $path, string $name): bool
19+
{
20+
return true;
21+
}
22+
23+
public function close(): bool
24+
{
25+
if (!$this->destroyed) {
26+
$this->destroyed = true;
27+
session_destroy();
28+
}
29+
return true;
30+
}
31+
32+
public function read(string $id): string|false
33+
{
34+
return '';
35+
}
36+
37+
public function write(string $id, string $data): bool
38+
{
39+
return true;
40+
}
41+
42+
public function destroy(string $id): bool
43+
{
44+
return true;
45+
}
46+
47+
public function gc(int $max_lifetime): int|false
48+
{
49+
return 0;
50+
}
51+
}
52+
53+
session_set_save_handler(new MySessionHandler(), true);
54+
session_start();
55+
56+
var_dump(session_regenerate_id(false));
57+
var_dump(session_status() === PHP_SESSION_NONE);
58+
59+
?>
60+
--EXPECTF--
61+
Warning: session_destroy(): Cannot call session save handler in a recursive manner in %s on line %d
62+
63+
Warning: session_destroy(): Session object destruction failed in %s on line %d
64+
65+
Warning: session_regenerate_id(): Session ID cannot be regenerated because the save handler closed the session in %s on line %d
66+
bool(false)
67+
bool(true)
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
--TEST--
2+
session_regenerate_id() when the save handler destroys the session
3+
--INI--
4+
session.save_handler=files
5+
session.name=PHPSESSID
6+
session.gc_probability=0
7+
--EXTENSIONS--
8+
session
9+
--FILE--
10+
<?php
11+
12+
ob_start();
13+
14+
class MySessionHandler extends SessionHandler
15+
{
16+
public function destroy(string $id): bool
17+
{
18+
session_destroy();
19+
return true;
20+
}
21+
}
22+
23+
session_set_save_handler(new MySessionHandler(), true);
24+
session_start();
25+
26+
var_dump(session_regenerate_id(true));
27+
var_dump(session_status() === PHP_SESSION_NONE);
28+
29+
?>
30+
--EXPECTF--
31+
Warning: session_destroy(): Cannot call session save handler in a recursive manner in %s on line %d
32+
33+
Warning: session_destroy(): Session object destruction failed in %s on line %d
34+
35+
Warning: session_regenerate_id(): Session ID cannot be regenerated because the save handler closed the session in %s on line %d
36+
bool(false)
37+
bool(true)

0 commit comments

Comments
 (0)