Skip to content

Commit 93a8d5c

Browse files
kamil-tekielakamil-tekiela
committed
Fix bug GH-8058 - mysqlnd segfault when prepare fails
Closes GH-8061
1 parent 61b276c commit 93a8d5c

File tree

4 files changed

+59
-54
lines changed

4 files changed

+59
-54
lines changed

‎NEWS‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ PHP NEWS
1616
. Fixed bug GH-7953 (ob_clean() only does not set Content-Encoding). (cmb)
1717
. Fixed bug GH-7980 (Unexpected result for iconv_mime_decode). (cmb)
1818

19+
- MySQLnd:
20+
. Fixed bug GH-8058 (NULL pointer dereference in mysqlnd package). (Kamil Tekiela)
21+
1922
- Zlib:
2023
. Fixed bug GH-7953 (ob_clean() only does not set Content-Encoding). (cmb)
2124

‎ext/mysqli/tests/gh8058.phpt‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
--TEST--
2+
GH-8058 (NULL pointer dereference in mysqlnd package (#81706))
3+
--SKIPIF--
4+
<?php
5+
require_once 'skipif.inc';
6+
require_once 'skipifconnectfailure.inc';
7+
?>
8+
--FILE--
9+
<?php
10+
require_once "connect.inc";
11+
12+
mysqli_report(MYSQLI_REPORT_OFF);
13+
$mysqli = new my_mysqli($host, $user, $passwd, $db, $port, $socket);
14+
15+
// There should be no segfault due to NULL deref
16+
$stmt = $mysqli->prepare("select 1,2,3");
17+
$stmt->bind_result($a,$a,$a);
18+
$stmt->prepare("");
19+
$stmt->prepare("select ".str_repeat("'A',", 0x1201)."1");
20+
unset($stmt); // trigger dtor
21+
22+
// There should be no memory leak
23+
$stmt = $mysqli->prepare("select 1,2,3");
24+
$stmt->bind_result($a,$a,$a);
25+
$stmt->prepare("");
26+
$stmt->prepare("select 1");
27+
unset($stmt); // trigger dtor
28+
29+
mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
30+
$stmt = $mysqli->prepare("select 1,2,3");
31+
try {
32+
// We expect an exception to be thrown
33+
$stmt->prepare("");
34+
} catch (mysqli_sql_exception $e) {
35+
var_dump($e->getMessage());
36+
}
37+
?>
38+
--EXPECT--
39+
string(15) "Query was empty"

‎ext/mysqli/tests/mysqli_stmt_affected_rows.phpt‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -196,18 +196,13 @@ require_once('skipifconnectfailure.inc');
196196
/* stmt_affected_rows is not really meant for SELECT! */
197197
if (mysqli_stmt_prepare($stmt, 'SELECT unknown_column FROM this_table_does_not_exist') ||
198198
mysqli_stmt_execute($stmt))
199-
printf("[041] The invalid SELECT statement is issued on purpose\n");
199+
printf("[041] Expecting SELECT statement to fail on purpose\n");
200200

201201
if (-1 !== ($tmp = mysqli_stmt_affected_rows($stmt)))
202202
printf("[042] Expecting int/-1, got %s/%s\n", gettype($tmp), $tmp);
203203

204-
if ($IS_MYSQLND) {
205-
if (false !== ($tmp = mysqli_stmt_store_result($stmt)))
206-
printf("[043] Expecting boolean/false, got %s\%s\n", gettype($tmp), $tmp);
207-
} else {
208-
if (true !== ($tmp = mysqli_stmt_store_result($stmt)))
209-
printf("[043] Libmysql does not care if the previous statement was bogus, expecting boolean/true, got %s\%s\n", gettype($tmp), $tmp);
210-
}
204+
if (true !== ($tmp = mysqli_stmt_store_result($stmt)))
205+
printf("[043] Expecting boolean/true, got %s/%s\n", gettype($tmp), $tmp);
211206

212207
if (0 !== ($tmp = mysqli_stmt_num_rows($stmt)))
213208
printf("[044] Expecting int/0, got %s/%s\n", gettype($tmp), $tmp);

‎ext/mysqlnd/mysqlnd_ps.c‎

Lines changed: 14 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -391,12 +391,10 @@ mysqlnd_stmt_prepare_read_eof(MYSQLND_STMT * s)
391391

392392
/* {{{ mysqlnd_stmt::prepare */
393393
static enum_func_status
394-
MYSQLND_METHOD(mysqlnd_stmt, prepare)(MYSQLND_STMT * const s, const char * const query, const size_t query_len)
394+
MYSQLND_METHOD(mysqlnd_stmt, prepare)(MYSQLND_STMT * s, const char * const query, const size_t query_len)
395395
{
396396
MYSQLND_STMT_DATA * stmt = s? s->data : NULL;
397397
MYSQLND_CONN_DATA * conn = stmt? stmt->conn : NULL;
398-
MYSQLND_STMT * s_to_prepare = s;
399-
MYSQLND_STMT_DATA * stmt_to_prepare = stmt;
400398

401399
DBG_ENTER("mysqlnd_stmt::prepare");
402400
if (!stmt || !conn) {
@@ -412,25 +410,15 @@ MYSQLND_METHOD(mysqlnd_stmt, prepare)(MYSQLND_STMT * const s, const char * const
412410
SET_EMPTY_ERROR(conn->error_info);
413411

414412
if (stmt->state > MYSQLND_STMT_INITTED) {
415-
/* See if we have to clean the wire */
416-
if (stmt->state == MYSQLND_STMT_WAITING_USE_OR_STORE) {
417-
/* Do implicit use_result and then flush the result */
418-
stmt->default_rset_handler = s->m->use_result;
419-
stmt->default_rset_handler(s);
420-
}
421-
/* No 'else' here please :) */
422-
if (stmt->state > MYSQLND_STMT_WAITING_USE_OR_STORE && stmt->result) {
423-
stmt->result->m.skip_result(stmt->result);
424-
}
425413
/*
426-
Create a new test statement, which we will prepare, but if anything
427-
fails, we will scrap it.
414+
Create a new prepared statement and destroy the previous one.
428415
*/
429-
s_to_prepare = conn->m->stmt_init(conn);
430-
if (!s_to_prepare) {
416+
s->m->dtor(s, TRUE);
417+
s = conn->m->stmt_init(conn);
418+
if (!s) {
431419
goto fail;
432420
}
433-
stmt_to_prepare = s_to_prepare->data;
421+
stmt = s->data;
434422
}
435423

436424
{
@@ -444,13 +432,13 @@ MYSQLND_METHOD(mysqlnd_stmt, prepare)(MYSQLND_STMT * const s, const char * const
444432
}
445433
}
446434

447-
if (FAIL == mysqlnd_stmt_read_prepare_response(s_to_prepare)) {
435+
if (FAIL == mysqlnd_stmt_read_prepare_response(s)) {
448436
goto fail;
449437
}
450438

451-
if (stmt_to_prepare->param_count) {
452-
if (FAIL == mysqlnd_stmt_skip_metadata(s_to_prepare) ||
453-
FAIL == mysqlnd_stmt_prepare_read_eof(s_to_prepare))
439+
if (stmt->param_count) {
440+
if (FAIL == mysqlnd_stmt_skip_metadata(s) ||
441+
FAIL == mysqlnd_stmt_prepare_read_eof(s))
454442
{
455443
goto fail;
456444
}
@@ -461,51 +449,31 @@ MYSQLND_METHOD(mysqlnd_stmt, prepare)(MYSQLND_STMT * const s, const char * const
461449
Beware that SHOW statements bypass the PS framework and thus they send
462450
no metadata at prepare.
463451
*/
464-
if (stmt_to_prepare->field_count) {
465-
MYSQLND_RES * result = conn->m->result_init(stmt_to_prepare->field_count);
452+
if (stmt->field_count) {
453+
MYSQLND_RES * result = conn->m->result_init(stmt->field_count);
466454
if (!result) {
467455
SET_OOM_ERROR(conn->error_info);
468456
goto fail;
469457
}
470458
/* Allocate the result now as it is needed for the reading of metadata */
471-
stmt_to_prepare->result = result;
459+
stmt->result = result;
472460

473461
result->conn = conn->m->get_reference(conn);
474462

475463
result->type = MYSQLND_RES_PS_BUF;
476464

477465
if (FAIL == result->m.read_result_metadata(result, conn) ||
478-
FAIL == mysqlnd_stmt_prepare_read_eof(s_to_prepare))
466+
FAIL == mysqlnd_stmt_prepare_read_eof(s))
479467
{
480468
goto fail;
481469
}
482470
}
483471

484-
if (stmt_to_prepare != stmt) {
485-
/* swap */
486-
size_t real_size = sizeof(MYSQLND_STMT) + mysqlnd_plugin_count() * sizeof(void *);
487-
char * tmp_swap = mnd_malloc(real_size);
488-
memcpy(tmp_swap, s, real_size);
489-
memcpy(s, s_to_prepare, real_size);
490-
memcpy(s_to_prepare, tmp_swap, real_size);
491-
mnd_free(tmp_swap);
492-
{
493-
MYSQLND_STMT_DATA * tmp_swap_data = stmt_to_prepare;
494-
stmt_to_prepare = stmt;
495-
stmt = tmp_swap_data;
496-
}
497-
s_to_prepare->m->dtor(s_to_prepare, TRUE);
498-
}
499472
stmt->state = MYSQLND_STMT_PREPARED;
500473
DBG_INF("PASS");
501474
DBG_RETURN(PASS);
502475

503476
fail:
504-
if (stmt_to_prepare != stmt && s_to_prepare) {
505-
s_to_prepare->m->dtor(s_to_prepare, TRUE);
506-
}
507-
stmt->state = MYSQLND_STMT_INITTED;
508-
509477
DBG_INF("FAIL");
510478
DBG_RETURN(FAIL);
511479
}

0 commit comments

Comments
 (0)