Skip to content

Commit 2cfc3fa

Browse files
derickrderickr
committed
PHPC-1243: Calling session methods after Session::endSession() causes use-after-free crash
1 parent 575e6a4 commit 2cfc3fa

File tree

4 files changed

+164
-5
lines changed

4 files changed

+164
-5
lines changed

‎src/MongoDB/Session.c‎

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,12 @@
2929

3030
zend_class_entry* php_phongo_session_ce;
3131

32+
#define SESSION_CHECK_LIVELINESS(i,m) \
33+
if (!(i)->client_session) { \
34+
phongo_throw_exception(PHONGO_ERROR_LOGIC TSRMLS_CC, "Cannot call '%s', as the session has already been ended.", (m)); \
35+
return; \
36+
}
37+
3238
static bool php_phongo_session_get_timestamp_parts(zval* obj, uint32_t* timestamp, uint32_t* increment TSRMLS_DC)
3339
{
3440
bool retval = false;
@@ -95,6 +101,7 @@ static PHP_METHOD(Session, advanceClusterTime)
95101
SUPPRESS_UNUSED_WARNING(return_value_used)
96102

97103
intern = Z_SESSION_OBJ_P(getThis());
104+
SESSION_CHECK_LIVELINESS(intern, "advanceClusterTime")
98105

99106
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "A", &zcluster_time) == FAILURE) {
100107
return;
@@ -125,6 +132,7 @@ static PHP_METHOD(Session, advanceOperationTime)
125132
SUPPRESS_UNUSED_WARNING(return_value_used)
126133

127134
intern = Z_SESSION_OBJ_P(getThis());
135+
SESSION_CHECK_LIVELINESS(intern, "advanceOperationTime")
128136

129137
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "O", &ztimestamp, php_phongo_timestamp_interface_ce) == FAILURE) {
130138
return;
@@ -148,6 +156,7 @@ static PHP_METHOD(Session, getClusterTime)
148156
SUPPRESS_UNUSED_WARNING(return_value_used)
149157

150158
intern = Z_SESSION_OBJ_P(getThis());
159+
SESSION_CHECK_LIVELINESS(intern, "getClusterTime")
151160

152161
if (zend_parse_parameters_none() == FAILURE) {
153162
return;
@@ -183,6 +192,7 @@ static PHP_METHOD(Session, getLogicalSessionId)
183192
SUPPRESS_UNUSED_WARNING(return_value_used)
184193

185194
intern = Z_SESSION_OBJ_P(getThis());
195+
SESSION_CHECK_LIVELINESS(intern, "getLogicalSessionId")
186196

187197
if (zend_parse_parameters_none() == FAILURE) {
188198
return;
@@ -213,6 +223,7 @@ static PHP_METHOD(Session, getOperationTime)
213223
SUPPRESS_UNUSED_WARNING(return_value_used)
214224

215225
intern = Z_SESSION_OBJ_P(getThis());
226+
SESSION_CHECK_LIVELINESS(intern, "getOperationTime")
216227

217228
if (zend_parse_parameters_none() == FAILURE) {
218229
return;
@@ -310,6 +321,7 @@ static PHP_METHOD(Session, startTransaction)
310321
SUPPRESS_UNUSED_WARNING(return_value_used)
311322

312323
intern = Z_SESSION_OBJ_P(getThis());
324+
SESSION_CHECK_LIVELINESS(intern, "startTransaction")
313325

314326
if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "|a", &options) == FAILURE) {
315327
return;
@@ -342,6 +354,7 @@ static PHP_METHOD(Session, commitTransaction)
342354
SUPPRESS_UNUSED_WARNING(return_value_used)
343355

344356
intern = Z_SESSION_OBJ_P(getThis());
357+
SESSION_CHECK_LIVELINESS(intern, "commitTransaction")
345358

346359
if (zend_parse_parameters_none() == FAILURE) {
347360
return;
@@ -363,6 +376,7 @@ static PHP_METHOD(Session, abortTransaction)
363376
SUPPRESS_UNUSED_WARNING(return_value_used)
364377

365378
intern = Z_SESSION_OBJ_P(getThis());
379+
SESSION_CHECK_LIVELINESS(intern, "abortTransaction")
366380

367381
if (zend_parse_parameters_none() == FAILURE) {
368382
return;
@@ -388,6 +402,7 @@ static PHP_METHOD(Session, endSession)
388402
}
389403

390404
mongoc_client_session_destroy(intern->client_session);
405+
intern->client_session = NULL;
391406
} /* }}} */
392407

393408
/* {{{ MongoDB\Driver\Session function entries */
@@ -477,7 +492,7 @@ static HashTable* php_phongo_session_get_debug_info(zval* object, int* is_temp T
477492

478493
array_init(&retval);
479494

480-
{
495+
if (intern->client_session) {
481496
const bson_t* lsid;
482497

483498
php_phongo_bson_state state = PHONGO_BSON_STATE_INITIALIZER;
@@ -494,9 +509,11 @@ static HashTable* php_phongo_session_get_debug_info(zval* object, int* is_temp T
494509
#else
495510
ADD_ASSOC_ZVAL_EX(&retval, "logicalSessionId", state.zchild);
496511
#endif
512+
} else {
513+
ADD_ASSOC_NULL_EX(&retval, "logicalSessionId");
497514
}
498515

499-
{
516+
if (intern->client_session) {
500517
const bson_t* cluster_time;
501518

502519
php_phongo_bson_state state = PHONGO_BSON_STATE_INITIALIZER;
@@ -517,12 +534,18 @@ static HashTable* php_phongo_session_get_debug_info(zval* object, int* is_temp T
517534
} else {
518535
ADD_ASSOC_NULL_EX(&retval, "clusterTime");
519536
}
537+
} else {
538+
ADD_ASSOC_NULL_EX(&retval, "clusterTime");
520539
}
521540

522-
cs_opts = mongoc_client_session_get_opts(intern->client_session);
523-
ADD_ASSOC_BOOL_EX(&retval, "causalConsistency", mongoc_session_opts_get_causal_consistency(cs_opts));
541+
if (intern->client_session) {
542+
cs_opts = mongoc_client_session_get_opts(intern->client_session);
543+
ADD_ASSOC_BOOL_EX(&retval, "causalConsistency", mongoc_session_opts_get_causal_consistency(cs_opts));
544+
} else {
545+
ADD_ASSOC_NULL_EX(&retval, "causalConsistency");
546+
}
524547

525-
{
548+
if (intern->client_session) {
526549
uint32_t timestamp, increment;
527550

528551
mongoc_client_session_get_operation_time(intern->client_session, &timestamp, &increment);
@@ -543,6 +566,8 @@ static HashTable* php_phongo_session_get_debug_info(zval* object, int* is_temp T
543566
} else {
544567
ADD_ASSOC_NULL_EX(&retval, "operationTime");
545568
}
569+
} else {
570+
ADD_ASSOC_NULL_EX(&retval, "operationTime");
546571
}
547572

548573
return Z_ARRVAL(retval);

‎tests/session/session-debug-004.phpt‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
--TEST--
2+
MongoDB\Driver\Session debug output (after ending session)
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
5+
<?php skip_if_not_libmongoc_crypto(); ?>
6+
<?php skip_if_not_live(); ?>
7+
<?php skip_if_server_version('<', '3.6'); ?>
8+
--FILE--
9+
<?php
10+
require_once __DIR__ . "/../utils/basic.inc";
11+
12+
$manager = new MongoDB\Driver\Manager(URI);
13+
$session = $manager->startSession();
14+
15+
$command = new MongoDB\Driver\Command(['ping' => 1]);
16+
$manager->executeCommand(DATABASE_NAME, $command, ['session' => $session]);
17+
18+
$session->endSession();
19+
20+
var_dump($session);
21+
22+
?>
23+
===DONE===
24+
<?php exit(0); ?>
25+
--EXPECTF--
26+
object(MongoDB\Driver\Session)#%d (%d) {
27+
["logicalSessionId"]=>
28+
NULL
29+
["clusterTime"]=>
30+
NULL
31+
["causalConsistency"]=>
32+
NULL
33+
["operationTime"]=>
34+
NULL
35+
}
36+
===DONE===

‎tests/session/session-endSession-001.phpt‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
--TEST--
2+
MongoDB\Driver\Session::endSession() Calling methods after session has been ended
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
5+
<?php skip_if_not_replica_set(); ?>
6+
<?php skip_if_server_version('<', '4.0'); ?>
7+
--FILE--
8+
<?php
9+
require_once __DIR__ . "/../utils/basic.inc";
10+
11+
$manager = new MongoDB\Driver\Manager(URI);
12+
13+
$sessionA = $manager->startSession();
14+
$sessionA->endSession();
15+
16+
echo throws(function() use ($sessionA) {
17+
$sessionA->startTransaction();
18+
}, 'MongoDB\Driver\Exception\LogicException'), "\n";
19+
20+
echo throws(function() use ($sessionA) {
21+
$sessionA->abortTransaction();
22+
}, 'MongoDB\Driver\Exception\LogicException'), "\n";
23+
24+
/* The reason that startTransaction is in here twice is that this script can run without exception
25+
* if the endSession() call is taken out. */
26+
echo throws(function() use ($sessionA) {
27+
$sessionA->startTransaction();
28+
}, 'MongoDB\Driver\Exception\LogicException'), "\n";
29+
30+
echo throws(function() use ($sessionA) {
31+
$sessionA->commitTransaction();
32+
}, 'MongoDB\Driver\Exception\LogicException'), "\n";
33+
34+
echo throws(function() use ($sessionA) {
35+
$sessionA->advanceOperationTime(new \MongoDB\BSON\Timestamp(1900123000, 1900123000));
36+
}, 'MongoDB\Driver\Exception\LogicException'), "\n";
37+
38+
echo throws(function() use ($sessionA) {
39+
$sessionA->advanceClusterTime([]);
40+
}, 'MongoDB\Driver\Exception\LogicException'), "\n";
41+
42+
echo throws(function() use ($sessionA) {
43+
var_dump($sessionA->getClusterTime());
44+
}, 'MongoDB\Driver\Exception\LogicException'), "\n";
45+
46+
echo throws(function() use ($sessionA) {
47+
var_dump($sessionA->getLogicalSessionId());
48+
}, 'MongoDB\Driver\Exception\LogicException'), "\n";
49+
50+
echo throws(function() use ($sessionA) {
51+
var_dump($sessionA->getOperationTime());
52+
}, 'MongoDB\Driver\Exception\LogicException'), "\n";
53+
54+
?>
55+
===DONE===
56+
<?php exit(0); ?>
57+
--EXPECT--
58+
OK: Got MongoDB\Driver\Exception\LogicException
59+
Cannot call 'startTransaction', as the session has already been ended.
60+
OK: Got MongoDB\Driver\Exception\LogicException
61+
Cannot call 'abortTransaction', as the session has already been ended.
62+
OK: Got MongoDB\Driver\Exception\LogicException
63+
Cannot call 'startTransaction', as the session has already been ended.
64+
OK: Got MongoDB\Driver\Exception\LogicException
65+
Cannot call 'commitTransaction', as the session has already been ended.
66+
OK: Got MongoDB\Driver\Exception\LogicException
67+
Cannot call 'advanceOperationTime', as the session has already been ended.
68+
OK: Got MongoDB\Driver\Exception\LogicException
69+
Cannot call 'advanceClusterTime', as the session has already been ended.
70+
OK: Got MongoDB\Driver\Exception\LogicException
71+
Cannot call 'getClusterTime', as the session has already been ended.
72+
OK: Got MongoDB\Driver\Exception\LogicException
73+
Cannot call 'getLogicalSessionId', as the session has already been ended.
74+
OK: Got MongoDB\Driver\Exception\LogicException
75+
Cannot call 'getOperationTime', as the session has already been ended.
76+
===DONE===

‎tests/session/session-endSession-002.phpt‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
--TEST--
2+
MongoDB\Driver\Session::endSession() Calling method multiple times
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
5+
<?php skip_if_not_libmongoc_crypto(); ?>
6+
<?php skip_if_not_live(); ?>
7+
<?php skip_if_server_version('<', '3.6'); ?>
8+
--FILE--
9+
<?php
10+
require_once __DIR__ . "/../utils/basic.inc";
11+
12+
$manager = new MongoDB\Driver\Manager(URI);
13+
14+
$sessionA = $manager->startSession();
15+
$sessionA->endSession();
16+
$sessionA->endSession();
17+
$sessionA->endSession();
18+
?>
19+
===DONE===
20+
<?php exit(0); ?>
21+
--EXPECT--
22+
===DONE===

0 commit comments

Comments
 (0)