Skip to content

Commit

Permalink
Fix bug GH-8058 - mysqlnd segfault when prepare fails
Browse files Browse the repository at this point in the history
Closes GH-8061
  • Loading branch information
kamil-tekiela committed Feb 14, 2022
1 parent 61b276c commit 93a8d5c
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 54 deletions.
3 changes: 3 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ PHP NEWS
. Fixed bug GH-7953 (ob_clean() only does not set Content-Encoding). (cmb)
. Fixed bug GH-7980 (Unexpected result for iconv_mime_decode). (cmb)

- MySQLnd:
. Fixed bug GH-8058 (NULL pointer dereference in mysqlnd package). (Kamil Tekiela)

- Zlib:
. Fixed bug GH-7953 (ob_clean() only does not set Content-Encoding). (cmb)

Expand Down
39 changes: 39 additions & 0 deletions ext/mysqli/tests/gh8058.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
--TEST--
GH-8058 (NULL pointer dereference in mysqlnd package (#81706))
--SKIPIF--
<?php
require_once 'skipif.inc';
require_once 'skipifconnectfailure.inc';
?>
--FILE--
<?php
require_once "connect.inc";

mysqli_report(MYSQLI_REPORT_OFF);
$mysqli = new my_mysqli($host, $user, $passwd, $db, $port, $socket);

// There should be no segfault due to NULL deref
$stmt = $mysqli->prepare("select 1,2,3");
$stmt->bind_result($a,$a,$a);
$stmt->prepare("");
$stmt->prepare("select ".str_repeat("'A',", 0x1201)."1");
unset($stmt); // trigger dtor

// There should be no memory leak
$stmt = $mysqli->prepare("select 1,2,3");
$stmt->bind_result($a,$a,$a);
$stmt->prepare("");
$stmt->prepare("select 1");
unset($stmt); // trigger dtor

mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
$stmt = $mysqli->prepare("select 1,2,3");
try {
// We expect an exception to be thrown
$stmt->prepare("");
} catch (mysqli_sql_exception $e) {
var_dump($e->getMessage());
}
?>
--EXPECT--
string(15) "Query was empty"
11 changes: 3 additions & 8 deletions ext/mysqli/tests/mysqli_stmt_affected_rows.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -196,18 +196,13 @@ require_once('skipifconnectfailure.inc');
/* stmt_affected_rows is not really meant for SELECT! */
if (mysqli_stmt_prepare($stmt, 'SELECT unknown_column FROM this_table_does_not_exist') ||
mysqli_stmt_execute($stmt))
printf("[041] The invalid SELECT statement is issued on purpose\n");
printf("[041] Expecting SELECT statement to fail on purpose\n");

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

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

if (0 !== ($tmp = mysqli_stmt_num_rows($stmt)))
printf("[044] Expecting int/0, got %s/%s\n", gettype($tmp), $tmp);
Expand Down
60 changes: 14 additions & 46 deletions ext/mysqlnd/mysqlnd_ps.c
Original file line number Diff line number Diff line change
Expand Up @@ -391,12 +391,10 @@ mysqlnd_stmt_prepare_read_eof(MYSQLND_STMT * s)

/* {{{ mysqlnd_stmt::prepare */
static enum_func_status
MYSQLND_METHOD(mysqlnd_stmt, prepare)(MYSQLND_STMT * const s, const char * const query, const size_t query_len)
MYSQLND_METHOD(mysqlnd_stmt, prepare)(MYSQLND_STMT * s, const char * const query, const size_t query_len)
{
MYSQLND_STMT_DATA * stmt = s? s->data : NULL;
MYSQLND_CONN_DATA * conn = stmt? stmt->conn : NULL;
MYSQLND_STMT * s_to_prepare = s;
MYSQLND_STMT_DATA * stmt_to_prepare = stmt;

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

if (stmt->state > MYSQLND_STMT_INITTED) {
/* See if we have to clean the wire */
if (stmt->state == MYSQLND_STMT_WAITING_USE_OR_STORE) {
/* Do implicit use_result and then flush the result */
stmt->default_rset_handler = s->m->use_result;
stmt->default_rset_handler(s);
}
/* No 'else' here please :) */
if (stmt->state > MYSQLND_STMT_WAITING_USE_OR_STORE && stmt->result) {
stmt->result->m.skip_result(stmt->result);
}
/*
Create a new test statement, which we will prepare, but if anything
fails, we will scrap it.
Create a new prepared statement and destroy the previous one.
*/
s_to_prepare = conn->m->stmt_init(conn);
if (!s_to_prepare) {
s->m->dtor(s, TRUE);
s = conn->m->stmt_init(conn);
if (!s) {
goto fail;
}
stmt_to_prepare = s_to_prepare->data;
stmt = s->data;
}

{
Expand All @@ -444,13 +432,13 @@ MYSQLND_METHOD(mysqlnd_stmt, prepare)(MYSQLND_STMT * const s, const char * const
}
}

if (FAIL == mysqlnd_stmt_read_prepare_response(s_to_prepare)) {
if (FAIL == mysqlnd_stmt_read_prepare_response(s)) {
goto fail;
}

if (stmt_to_prepare->param_count) {
if (FAIL == mysqlnd_stmt_skip_metadata(s_to_prepare) ||
FAIL == mysqlnd_stmt_prepare_read_eof(s_to_prepare))
if (stmt->param_count) {
if (FAIL == mysqlnd_stmt_skip_metadata(s) ||
FAIL == mysqlnd_stmt_prepare_read_eof(s))
{
goto fail;
}
Expand All @@ -461,51 +449,31 @@ MYSQLND_METHOD(mysqlnd_stmt, prepare)(MYSQLND_STMT * const s, const char * const
Beware that SHOW statements bypass the PS framework and thus they send
no metadata at prepare.
*/
if (stmt_to_prepare->field_count) {
MYSQLND_RES * result = conn->m->result_init(stmt_to_prepare->field_count);
if (stmt->field_count) {
MYSQLND_RES * result = conn->m->result_init(stmt->field_count);
if (!result) {
SET_OOM_ERROR(conn->error_info);
goto fail;
}
/* Allocate the result now as it is needed for the reading of metadata */
stmt_to_prepare->result = result;
stmt->result = result;

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

result->type = MYSQLND_RES_PS_BUF;

if (FAIL == result->m.read_result_metadata(result, conn) ||
FAIL == mysqlnd_stmt_prepare_read_eof(s_to_prepare))
FAIL == mysqlnd_stmt_prepare_read_eof(s))
{
goto fail;
}
}

if (stmt_to_prepare != stmt) {
/* swap */
size_t real_size = sizeof(MYSQLND_STMT) + mysqlnd_plugin_count() * sizeof(void *);
char * tmp_swap = mnd_malloc(real_size);
memcpy(tmp_swap, s, real_size);
memcpy(s, s_to_prepare, real_size);
memcpy(s_to_prepare, tmp_swap, real_size);
mnd_free(tmp_swap);
{
MYSQLND_STMT_DATA * tmp_swap_data = stmt_to_prepare;
stmt_to_prepare = stmt;
stmt = tmp_swap_data;
}
s_to_prepare->m->dtor(s_to_prepare, TRUE);
}
stmt->state = MYSQLND_STMT_PREPARED;
DBG_INF("PASS");
DBG_RETURN(PASS);

fail:
if (stmt_to_prepare != stmt && s_to_prepare) {
s_to_prepare->m->dtor(s_to_prepare, TRUE);
}
stmt->state = MYSQLND_STMT_INITTED;

DBG_INF("FAIL");
DBG_RETURN(FAIL);
}
Expand Down

0 comments on commit 93a8d5c

Please sign in to comment.