Skip to content

ext/mysqlnd: Fix crash after metadata EOF failure on prepared statements #25297

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
iliaal wants to merge 1 commit into
base: PHP-8.4
Choose a base branch
from
+159 −17
Open

ext/mysqlnd: Fix crash after metadata EOF failure on prepared statements #25297

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions NEWS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ PHP NEWS
. Fixed stack overflow in mb_check_encoding() and mb_convert_encoding() with
deeply nested arrays. (Eyüp Can Akman)

- MySQLnd:
. Fixed crash when a server sends a truncated or missing metadata EOF packet
for a prepared statement. (Ilia Alshanetsky)

- Opcache:
. Fixed bug GH-25165 (Tracing JIT loops forever once opcache.jit_buffer_size
is exhausted while compiling side traces). (Ilia Alshanetsky)
Expand Down
43 changes: 43 additions & 0 deletions ext/mysqli/tests/fake_server.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1445,6 +1445,49 @@ function my_mysqli_test_stmt_response_field_len_premature(my_mysqli_fake_server_
$conn->read(65536);
}

function my_mysqli_test_stmt_execute_metadata_eof_short(my_mysqli_fake_server_conn $conn): void
{
$rh = $conn->packet_generator->server_stmt_execute_items_response();
$rh[2]->packet_length = '040000';
$rh[2]->server_status = '02';

$conn->send_server_greetings();
$conn->read();
$conn->send_server_ok();
$conn->read();
$conn->send_server_stmt_prepare_items_response();
$conn->read();
$conn->send($conn->packets_to_bytes(array_slice($rh, 0, 3)), "Stmt Execute metadata with short EOF");
$conn->read(65536);
}

function my_mysqli_test_stmt_execute_metadata_then_close(my_mysqli_fake_server_conn $conn): void
{
$rh = $conn->packet_generator->server_stmt_execute_items_response();

$conn->send_server_greetings();
$conn->read();
$conn->send_server_ok();
$conn->read();
$conn->send_server_stmt_prepare_items_response();
$conn->read();
$conn->send($conn->packets_to_bytes(array_slice($rh, 0, 2)), "Stmt Execute metadata without EOF");
}

function my_mysqli_test_stmt_prepare_metadata_eof_short(my_mysqli_fake_server_conn $conn): void
{
$packets = $conn->packet_generator->server_stmt_prepare_items_response();
$packets[2]->server_status = '02';
$packets[2]->packet_length = '040000';

$conn->send_server_greetings();
$conn->read();
$conn->send_server_ok();
$conn->read();
$conn->send($conn->packets_to_bytes($packets), "Stmt prepare items with short EOF");
$conn->read();
}

function run_fake_server(string $test_function, int|string $port = 0): void
{
$host = '127.0.0.1';
Expand Down
35 changes: 35 additions & 0 deletions ext/mysqli/tests/mysqlnd_stmt_execute_metadata_eof_close.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
--TEST--
mysqlnd: connection closed before metadata EOF on mysqli_stmt::execute() sets an error
--EXTENSIONS--
mysqli
--FILE--
<?php
require_once 'fake_server.inc';

$process = run_fake_server_in_background('stmt_execute_metadata_then_close');
$process->wait();

$conn = new mysqli('127.0.0.1', 'root', '', '', $process->getPort());
$stmt = $conn->prepare('SELECT item FROM items');
try {
$stmt->execute();
} catch (mysqli_sql_exception $e) {
echo $e::class, ": ", $e->getMessage(), PHP_EOL;
}

$process->terminate();

print "done!";
?>
--EXPECTF--
[*] Server started on 127.0.0.1:%d
[*] Connection established
[*] Sending - Server Greeting: 580000000a352e352e352d31302e352e31382d4d6172696144420003000000473e3f6047257c6700fef7080200ff81150000000000000f0000006c6b55463f49335f686c6431006d7973716c5f6e61746976655f70617373776f7264
[*] Received: 6900000185a21a00000000c0080000000000000000000000000000000000000000000000726f6f7400006d7973716c5f6e61746976655f70617373776f7264002c0c5f636c69656e745f6e616d65076d7973716c6e640c5f7365727665725f686f7374093132372e302e302e31
[*] Sending - Server OK: 0700000200000002000000
[*] Received: 170000001653454c454354206974656d2046524f4d206974656d73
[*] Sending - Stmt prepare items: 0c0000010001000000010000000000003000000203646566087068705f74657374056974656d73056974656d73046974656d046974656d0ce000c8000000fd011000000005000003fe00000200
[*] Received: 0a00000017010000000001000000
[*] Sending - Stmt Execute metadata without EOF: 01000001013000000203646566087068705f74657374056974656d73056974656d73046974656d046974656d0ce000c8000000fd0110000000
mysqli_sql_exception: MySQL server has gone away
done!
38 changes: 38 additions & 0 deletions ext/mysqli/tests/mysqlnd_stmt_execute_metadata_eof_short.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
--TEST--
mysqlnd: short metadata EOF packet on mysqli_stmt::execute()
--EXTENSIONS--
mysqli
--FILE--
<?php
require_once 'fake_server.inc';

$process = run_fake_server_in_background('stmt_execute_metadata_eof_short');
$process->wait();

mysqli_report(MYSQLI_REPORT_OFF);
$conn = new mysqli('127.0.0.1', 'root', '', '', $process->getPort());
$stmt = $conn->prepare('SELECT item FROM items');
var_dump($stmt->execute());
var_dump($stmt->errno);

$process->terminate();

print "done!";
?>
--EXPECTF--
[*] Server started on 127.0.0.1:%d
[*] Connection established
[*] Sending - Server Greeting: 580000000a352e352e352d31302e352e31382d4d6172696144420003000000473e3f6047257c6700fef7080200ff81150000000000000f0000006c6b55463f49335f686c6431006d7973716c5f6e61746976655f70617373776f7264
[*] Received: 6900000185a21a00000000c0080000000000000000000000000000000000000000000000726f6f7400006d7973716c5f6e61746976655f70617373776f7264002c0c5f636c69656e745f6e616d65076d7973716c6e640c5f7365727665725f686f7374093132372e302e302e31
[*] Sending - Server OK: 0700000200000002000000
[*] Received: 170000001653454c454354206974656d2046524f4d206974656d73
[*] Sending - Stmt prepare items: 0c0000010001000000010000000000003000000203646566087068705f74657374056974656d73056974656d73046974656d046974656d0ce000c8000000fd011000000005000003fe00000200
[*] Received: 0a00000017010000000001000000
[*] Sending - Stmt Execute metadata with short EOF: 01000001013000000203646566087068705f74657374056974656d73056974656d73046974656d046974656d0ce000c8000000fd011000000004000003fe000002

Warning: mysqli_stmt::execute(): Premature end of data (mysqlnd_wireprotocol.c:%d) in %s on line %d

Warning: mysqli_stmt::execute(): EOF packet shorter than expected in %s on line %d
bool(false)
int(0)
done!
38 changes: 38 additions & 0 deletions ext/mysqli/tests/mysqlnd_stmt_prepare_metadata_eof.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
--TEST--
mysqlnd: short metadata EOF packet on mysqli_stmt::prepare() after stmt_init()
--EXTENSIONS--
mysqli
--FILE--
<?php
require_once 'fake_server.inc';

$process = run_fake_server_in_background('stmt_prepare_metadata_eof_short');
$process->wait();

$conn = new mysqli('127.0.0.1', 'root', '', '', $process->getPort());
$stmt = $conn->stmt_init();
var_dump($stmt->prepare('SELECT item FROM items'));
var_dump($stmt->errno);
unset($stmt);

$process->terminate(true);

print "done!";
?>
--EXPECTF--
[*] Server started on 127.0.0.1:%d
[*] Connection established
[*] Sending - Server Greeting: 580000000a352e352e352d31302e352e31382d4d6172696144420003000000473e3f6047257c6700fef7080200ff81150000000000000f0000006c6b55463f49335f686c6431006d7973716c5f6e61746976655f70617373776f7264
[*] Received: 6900000185a21a00000000c0080000000000000000000000000000000000000000000000726f6f7400006d7973716c5f6e61746976655f70617373776f7264002c0c5f636c69656e745f6e616d65076d7973716c6e640c5f7365727665725f686f7374093132372e302e302e31
[*] Sending - Server OK: 0700000200000002000000
[*] Received: 170000001653454c454354206974656d2046524f4d206974656d73
[*] Sending - Stmt prepare items with short EOF: 0c0000010001000000010000000000003000000203646566087068705f74657374056974656d73056974656d73046974656d046974656d0ce000c8000000fd011000000004000003fe000002

Warning: mysqli_stmt::prepare(): Premature end of data (mysqlnd_wireprotocol.c:%d) in %s on line %d

Warning: mysqli_stmt::prepare(): EOF packet shorter than expected in %s on line %d
bool(false)
int(0)
[*] Received: 050000001901000000
[*] Server finished
done!
11 changes: 1 addition & 10 deletions ext/mysqlnd/mysqlnd_ps.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,16 +348,7 @@ mysqlnd_stmt_prepare_read_eof(MYSQLND_STMT * s)
if (FAIL == (ret = PACKET_READ(conn, &fields_eof))) {
if (stmt->result) {
stmt->result->m.free_result_contents(stmt->result);
/* The memset() below resets the statement, so release what it still owns first. */
conn->m->free_reference(conn);
mnd_efree(stmt->execute_cmd_buffer.buffer);
/* XXX: This will crash, because we will null also the methods.
But seems it happens in extreme cases or doesn't. Should be fixed by exporting a function
(from mysqlnd_driver.c?) to do the reset.
This bad handling is also in mysqlnd_result.c
*/
memset(stmt, 0, sizeof(MYSQLND_STMT_DATA));
stmt->state = MYSQLND_STMT_INITTED;
stmt->result = NULL;
}
} else {
UPSERT_STATUS_SET_SERVER_STATUS(stmt->upsert_status, fields_eof.server_status);
Expand Down
7 changes: 0 additions & 7 deletions ext/mysqlnd/mysqlnd_result.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,13 +316,6 @@ mysqlnd_query_read_result_set_header(MYSQLND_CONN_DATA * conn, MYSQLND_STMT * s)
conn->current_result = NULL;
} else {
stmt->result = NULL;
/* XXX: This will crash, because we will null also the methods.
But seems it happens in extreme cases or doesn't. Should be fixed by exporting a function
(from mysqlnd_driver.c?) to do the reset.
This is done also in mysqlnd_ps.c
*/
memset(stmt, 0, sizeof(*stmt));
stmt->state = MYSQLND_STMT_INITTED;
}
} else {
DBG_INF_FMT("warnings=%u server_status=%u", fields_eof.warning_count, fields_eof.server_status);
Expand Down
Loading