Skip to content

Commit 6b819bd

Browse files
committed
[PDO] Fix leak of driver_params argument of bindParam() and bindColumn()
register_bound_param() took a reference on the driver_params argument but never released it: really_register_bound_param() ADDREFs another reference for the bound-params hash, yet its early failure paths after that point (rewrite_name_to_position(), PDO_PARAM_EVT_NORMALIZE hook) and every return path of register_bound_param() itself dropped only param.parameter, leaking one or two references per call. 200k failing bindParam() calls grow memory by ~128MB. The transient copy is now released on both failure and success, and really_register_bound_param() releases its hash-bound reference on early failures; the PDO_PARAM_EVT_ALLOC hook failure path already released it via the hash dtor. Sibling audit: bindValue() and execute()'s input_params loop leave driver_params undefined so the new releases are no-ops there. Closes GH-23463
1 parent b2956e0 commit 6b819bd

3 files changed

Lines changed: 95 additions & 0 deletions

File tree

‎NEWS‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,8 @@ PHP NEWS
4646
- PDO:
4747
. Fixed a leak when a persistent connection failed a liveness check
4848
with no other live PDO handle. (iliaal)
49+
. Fixed a leak of the driver_params argument in bindParam() and
50+
bindColumn(). (iliaal)
4951

5052
- Standard:
5153
. Fixed a memory leak in array_merge_recursive() when the recursive merge of

‎ext/pdo/pdo_stmt.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,7 @@ static bool really_register_bound_param(struct pdo_bound_param_data *param, pdo_
330330
zend_string_release_ex(param->name, 0);
331331
param->name = NULL;
332332
}
333+
zval_ptr_dtor(&param->driver_params);
333334
return 0;
334335
}
335336

@@ -344,6 +345,7 @@ static bool really_register_bound_param(struct pdo_bound_param_data *param, pdo_
344345
zend_string_release_ex(param->name, 0);
345346
param->name = NULL;
346347
}
348+
zval_ptr_dtor(&param->driver_params);
347349
return 0;
348350
}
349351
}
@@ -1461,9 +1463,11 @@ static void register_bound_param(INTERNAL_FUNCTION_PARAMETERS, int is_param) /*
14611463
if (!Z_ISUNDEF(param.parameter)) {
14621464
zval_ptr_dtor(&(param.parameter));
14631465
}
1466+
zval_ptr_dtor(&param.driver_params);
14641467

14651468
RETURN_FALSE;
14661469
}
1470+
zval_ptr_dtor(&param.driver_params);
14671471

14681472
RETURN_TRUE;
14691473
} /* }}} */
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
--TEST--
2+
PDO: bindParam() must not leak driver_params
3+
--EXTENSIONS--
4+
pdo
5+
pdo_sqlite
6+
--FILE--
7+
<?php
8+
class C {}
9+
$db = new PDO('sqlite::memory:');
10+
$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
11+
$stmt = $db->prepare('SELECT ?');
12+
13+
$n = 20000;
14+
$dp = str_repeat('a', 1024);
15+
$obj = new C();
16+
try {
17+
$stmt->bindParam(1, $obj, PDO::PARAM_STR, 0, $dp);
18+
} catch (Throwable $e) {
19+
echo $e::class, ': ', $e->getMessage(), "\n";
20+
}
21+
for ($i = 0; $i < $n; $i++) {
22+
$dp = str_repeat('a', 1024);
23+
$obj = new C();
24+
try {
25+
$stmt->bindParam(1, $obj, PDO::PARAM_STR, 0, $dp);
26+
} catch (Error $e) {
27+
}
28+
}
29+
$before = memory_get_usage();
30+
for ($i = 0; $i < $n; $i++) {
31+
$dp = str_repeat('b', 1024);
32+
$obj = new C();
33+
try {
34+
$stmt->bindParam(1, $obj, PDO::PARAM_STR, 0, $dp);
35+
} catch (Error $e) {
36+
}
37+
}
38+
$diff = memory_get_usage() - $before;
39+
if ($diff > 1000) {
40+
echo "LEAK\n";
41+
} else {
42+
echo "OK\n";
43+
}
44+
45+
$stmt2 = $db->prepare('SELECT :bar');
46+
$v = 'x';
47+
for ($i = 0; $i < $n; $i++) {
48+
$dp = str_repeat('c', 1024);
49+
try {
50+
$stmt2->bindParam(':missing', $v, PDO::PARAM_STR, 0, $dp);
51+
} catch (PDOException $e) {
52+
}
53+
}
54+
$before = memory_get_usage();
55+
for ($i = 0; $i < $n; $i++) {
56+
$dp = str_repeat('d', 1024);
57+
try {
58+
$stmt2->bindParam(':missing', $v, PDO::PARAM_STR, 0, $dp);
59+
} catch (PDOException $e) {
60+
}
61+
}
62+
$diff = memory_get_usage() - $before;
63+
if ($diff > 1000) {
64+
echo "LEAK\n";
65+
} else {
66+
echo "OK\n";
67+
}
68+
69+
for ($i = 0; $i < $n; $i++) {
70+
$dp = str_repeat('e', 1024);
71+
$stmt->bindParam(1, $v, PDO::PARAM_STR, 0, $dp);
72+
}
73+
$before = memory_get_usage();
74+
for ($i = 0; $i < $n; $i++) {
75+
$dp = str_repeat('f', 1024);
76+
$stmt->bindParam(1, $v, PDO::PARAM_STR, 0, $dp);
77+
}
78+
$diff = memory_get_usage() - $before;
79+
if ($diff > 1000) {
80+
echo "LEAK\n";
81+
} else {
82+
echo "OK\n";
83+
}
84+
?>
85+
--EXPECT--
86+
Error: Object of class C could not be converted to string
87+
OK
88+
OK
89+
OK

0 commit comments

Comments
 (0)