Skip to content

Commit 087a08d

Browse files
committed
Fix session save-handler argv leak on recursive rejection
ps_call_handler() returned on the recursive-call rejection branch before reaching the argv cleanup loop, leaking one ref per owned argument. The read/write/destroy/validate_sid/update_timestamp callers copy the session id and data into argv and rely on ps_call_handler() to release them, so a handler that re-enters session machinery (for example calling session_destroy() from within a write handler) leaks those strings. Fold the handler call into an else branch so the cleanup loop always runs. Closes GH-22382
1 parent b15a786 commit 087a08d

2 files changed

Lines changed: 40 additions & 9 deletions

File tree

ext/session/mod_user.c

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,16 +30,16 @@ static void ps_call_handler(zval *func, int argc, zval *argv, zval *retval)
3030
PS(in_save_handler) = 0;
3131
ZVAL_UNDEF(retval);
3232
php_error_docref(NULL, E_WARNING, "Cannot call session save handler in a recursive manner");
33-
return;
34-
}
35-
PS(in_save_handler) = 1;
36-
if (call_user_function(NULL, NULL, func, retval, argc, argv) == FAILURE) {
37-
zval_ptr_dtor(retval);
38-
ZVAL_UNDEF(retval);
39-
} else if (Z_ISUNDEF_P(retval)) {
40-
ZVAL_NULL(retval);
33+
} else {
34+
PS(in_save_handler) = 1;
35+
if (call_user_function(NULL, NULL, func, retval, argc, argv) == FAILURE) {
36+
zval_ptr_dtor(retval);
37+
ZVAL_UNDEF(retval);
38+
} else if (Z_ISUNDEF_P(retval)) {
39+
ZVAL_NULL(retval);
40+
}
41+
PS(in_save_handler) = 0;
4142
}
42-
PS(in_save_handler) = 0;
4343
for (i = 0; i < argc; i++) {
4444
zval_ptr_dtor(&argv[i]);
4545
}
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
--TEST--
2+
ps_call_handler() releases argv when a recursive save-handler call is rejected
3+
--INI--
4+
session.save_path=
5+
session.name=PHPSESSID
6+
--EXTENSIONS--
7+
session
8+
--FILE--
9+
<?php
10+
class H extends SessionHandler {
11+
public bool $tripped = false;
12+
public function write($id, $data): bool {
13+
if (!$this->tripped) {
14+
$this->tripped = true;
15+
session_destroy();
16+
}
17+
return true;
18+
}
19+
}
20+
21+
session_set_save_handler(new H, true);
22+
session_start();
23+
$_SESSION['x'] = 1;
24+
session_write_close();
25+
echo "done\n";
26+
?>
27+
--EXPECTF--
28+
Warning: session_destroy(): Cannot call session save handler in a recursive manner in %s on line %d
29+
30+
Warning: session_destroy(): Session object destruction failed in %s on line %d
31+
done

0 commit comments

Comments
 (0)