From 07426130c1f91aa13e76af32b9d0d13c8e14d73a Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Sat, 7 Nov 2026 13:21:55 -0500 Subject: [PATCH] ext/mysqlnd: Fix crash after metadata EOF failure on prepared statements When the EOF packet after result set metadata could not be read, both mysqlnd_stmt_prepare_read_eof() and mysqlnd_query_read_result_set_header() memset the whole statement, clearing error_info, upsert_status and conn, so the next errno or error access dereferenced NULL. Free only the result and keep the statement intact. --- NEWS | 4 ++ ext/mysqli/tests/fake_server.inc | 43 +++++++++++++++++++ ...sqlnd_stmt_execute_metadata_eof_close.phpt | 35 +++++++++++++++ ...sqlnd_stmt_execute_metadata_eof_short.phpt | 38 ++++++++++++++++ .../mysqlnd_stmt_prepare_metadata_eof.phpt | 38 ++++++++++++++++ ext/mysqlnd/mysqlnd_ps.c | 11 +---- ext/mysqlnd/mysqlnd_result.c | 7 --- 7 files changed, 159 insertions(+), 17 deletions(-) create mode 100644 ext/mysqli/tests/mysqlnd_stmt_execute_metadata_eof_close.phpt create mode 100644 ext/mysqli/tests/mysqlnd_stmt_execute_metadata_eof_short.phpt create mode 100644 ext/mysqli/tests/mysqlnd_stmt_prepare_metadata_eof.phpt diff --git a/NEWS b/NEWS index 668b5898c62c..57e952d5d400 100644 --- a/NEWS +++ b/NEWS @@ -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) diff --git a/ext/mysqli/tests/fake_server.inc b/ext/mysqli/tests/fake_server.inc index 2a4d17f946e8..5e8e8e568238 100644 --- a/ext/mysqli/tests/fake_server.inc +++ b/ext/mysqli/tests/fake_server.inc @@ -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'; diff --git a/ext/mysqli/tests/mysqlnd_stmt_execute_metadata_eof_close.phpt b/ext/mysqli/tests/mysqlnd_stmt_execute_metadata_eof_close.phpt new file mode 100644 index 000000000000..0aa91172b709 --- /dev/null +++ b/ext/mysqli/tests/mysqlnd_stmt_execute_metadata_eof_close.phpt @@ -0,0 +1,35 @@ +--TEST-- +mysqlnd: connection closed before metadata EOF on mysqli_stmt::execute() sets an error +--EXTENSIONS-- +mysqli +--FILE-- +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! diff --git a/ext/mysqli/tests/mysqlnd_stmt_execute_metadata_eof_short.phpt b/ext/mysqli/tests/mysqlnd_stmt_execute_metadata_eof_short.phpt new file mode 100644 index 000000000000..084d6a0fc551 --- /dev/null +++ b/ext/mysqli/tests/mysqlnd_stmt_execute_metadata_eof_short.phpt @@ -0,0 +1,38 @@ +--TEST-- +mysqlnd: short metadata EOF packet on mysqli_stmt::execute() +--EXTENSIONS-- +mysqli +--FILE-- +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! diff --git a/ext/mysqli/tests/mysqlnd_stmt_prepare_metadata_eof.phpt b/ext/mysqli/tests/mysqlnd_stmt_prepare_metadata_eof.phpt new file mode 100644 index 000000000000..deadce7076e6 --- /dev/null +++ b/ext/mysqli/tests/mysqlnd_stmt_prepare_metadata_eof.phpt @@ -0,0 +1,38 @@ +--TEST-- +mysqlnd: short metadata EOF packet on mysqli_stmt::prepare() after stmt_init() +--EXTENSIONS-- +mysqli +--FILE-- +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! diff --git a/ext/mysqlnd/mysqlnd_ps.c b/ext/mysqlnd/mysqlnd_ps.c index 9254c58e58d7..0d773c748359 100644 --- a/ext/mysqlnd/mysqlnd_ps.c +++ b/ext/mysqlnd/mysqlnd_ps.c @@ -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); diff --git a/ext/mysqlnd/mysqlnd_result.c b/ext/mysqlnd/mysqlnd_result.c index f43b1d7f59d3..7814d95e230b 100644 --- a/ext/mysqlnd/mysqlnd_result.c +++ b/ext/mysqlnd/mysqlnd_result.c @@ -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);