From ef091dc575739bd05d45dbca063a59d167b596a9 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Sat, 29 Aug 2026 08:00:20 -0400 Subject: [PATCH] [mysqlnd] Fix result set field metadata length buffer over-read The rset_field metadata reader trusted each length-encoded string size: a hostile server could send a length marker at the end of a field packet whose value exceeds the remaining payload, advancing p past header.size and past the command buffer before the next dereference, and recording attacker-controlled lengths on pointers outside the packet that later feed memcpy() into the field memory pool. Bound each metadata string by bailing once p leaves the payload and rejecting lengths larger than the remaining bytes, matching php_mysqlnd_auth_response_read() from GHSA-h35g-vwh6-m678; an audit found no other users of the READ_RSET_FIELD macro and the trailing default-value check never dereferences its length. --- ext/mysqli/tests/fake_server.inc | 44 +++++++++++++++++++ .../mysqlnd_rset_field_len_over_read.phpt | 42 ++++++++++++++++++ .../mysqlnd_rset_field_len_past_packet.phpt | 40 +++++++++++++++++ ext/mysqlnd/mysqlnd_wireprotocol.c | 9 +++- 4 files changed, 134 insertions(+), 1 deletion(-) create mode 100644 ext/mysqli/tests/mysqlnd_rset_field_len_over_read.phpt create mode 100644 ext/mysqli/tests/mysqlnd_rset_field_len_past_packet.phpt diff --git a/ext/mysqli/tests/fake_server.inc b/ext/mysqli/tests/fake_server.inc index dad8bc52ddd1..b6a19e98a4e0 100644 --- a/ext/mysqli/tests/fake_server.inc +++ b/ext/mysqli/tests/fake_server.inc @@ -816,6 +816,50 @@ function my_mysqli_test_stmt_response_row_read_two_fields(my_mysqli_fake_server_ } } +function my_mysqli_test_rset_field_metadata_len_over_read(my_mysqli_fake_server_conn $conn): void +{ + $rh = $conn->packet_generator->server_tabular_query_response(); + + $qr2 = new my_mysqli_fake_packet(); + $qr2->packet_length = "0c0000"; + $qr2->packet_number = "02"; + $qr2->catalog_length_plus_name = "0161"; + $qr2->db_length_plus_name = "0162"; + $qr2->table_length_plus_name = "0163"; + $qr2->original_t = "0164"; + $qr2->name_length_plus_name = "0165"; + $qr2->original_n = "fcff"; + + $conn->send_server_greetings(); + $conn->read_packets(1); + $conn->send_server_ok(); + $conn->read_packets(1); + $conn->send($conn->packets_to_bytes([$rh[0], $qr2]), "Malicious Tabular Response [metadata string length past the packet size]"); + $conn->read(); +} + +function my_mysqli_test_rset_field_metadata_len_past_packet(my_mysqli_fake_server_conn $conn): void +{ + $rh = $conn->packet_generator->server_tabular_query_response(); + + $qr2 = new my_mysqli_fake_packet(); + $qr2->packet_length = "0c0000"; + $qr2->packet_number = "02"; + $qr2->catalog_length_plus_name = "0161"; + $qr2->db_length_plus_name = "0162"; + $qr2->table_length_plus_name = "0163"; + $qr2->original_t = "0164"; + $qr2->name_length_plus_name = "0165"; + $qr2->original_n = "0561"; + + $conn->send_server_greetings(); + $conn->read_packets(1); + $conn->send_server_ok(); + $conn->read_packets(1); + $conn->send($conn->packets_to_bytes([$rh[0], $qr2]), "Malicious Tabular Response [metadata string length past the packet size]"); + $conn->read(); +} + function my_mysqli_test_query_response_row_length_overflow(my_mysqli_fake_server_conn $conn): void { $rh = $conn->packet_generator->server_query_execute_data_response('strval'); diff --git a/ext/mysqli/tests/mysqlnd_rset_field_len_over_read.phpt b/ext/mysqli/tests/mysqlnd_rset_field_len_over_read.phpt new file mode 100644 index 000000000000..274468ded63d --- /dev/null +++ b/ext/mysqli/tests/mysqlnd_rset_field_len_over_read.phpt @@ -0,0 +1,42 @@ +--TEST-- +mysqlnd result set field metadata string length buffer over-read (len clamped to packet size) +--EXTENSIONS-- +mysqli +--FILE-- +wait(); + +try { + $conn = new mysqli( $servername, $username, $password, "", $process->getPort()); + var_dump($conn->query("SELECT * from users")); +} catch (Exception $e) { + echo $e::class, ": ", $e->getMessage(), PHP_EOL; +} + +$conn->close(); + +$process->terminate(); + +print "done!"; +?> +--EXPECTF-- +[*] Server started on 127.0.0.1:%d +[*] Connection established +[*] Sending - Server Greeting: 580000000a352e352e352d31302e352e31382d4d6172696144420003000000473e3f6047257c6700fef7080200ff81150000000000000f0000006c6b55463f49335f686c6431006d7973716c5f6e61746976655f70617373776f7264 +[*] Received: %s +[*] Sending - Server OK: 0700000200000002000000 +[*] Received: %s +[*] Sending - Malicious Tabular Response [metadata string length past the packet size]: 01000001010c00000201610162016301640165fcff + +Warning: mysqli::query(): Premature end of data (mysqlnd_wireprotocol.c:%d) in %s on line %d + +Warning: mysqli::query(): Result set field packet %d bytes shorter than expected in %s on line %d +bool(false) +done! diff --git a/ext/mysqli/tests/mysqlnd_rset_field_len_past_packet.phpt b/ext/mysqli/tests/mysqlnd_rset_field_len_past_packet.phpt new file mode 100644 index 000000000000..020d28d95b25 --- /dev/null +++ b/ext/mysqli/tests/mysqlnd_rset_field_len_past_packet.phpt @@ -0,0 +1,40 @@ +--TEST-- +mysqlnd result set field metadata string length exceeds remaining packet bytes +--EXTENSIONS-- +mysqli +--FILE-- +wait(); + +try { + $conn = new mysqli( $servername, $username, $password, "", $process->getPort()); + var_dump($conn->query("SELECT * from users")); +} catch (Exception $e) { + echo $e::class, ": ", $e->getMessage(), PHP_EOL; +} + +$conn->close(); + +$process->terminate(); + +print "done!"; +?> +--EXPECTF-- +[*] Server started on 127.0.0.1:%d +[*] Connection established +[*] Sending - Server Greeting: %s +[*] Received: %s +[*] Sending - Server OK: %s +[*] Received: %s +[*] Sending - Malicious Tabular Response [metadata string length past the packet size]: %s + +Warning: mysqli::query(): Result set field metadata string length is past the packet size in %s on line %d +bool(false) +done! diff --git a/ext/mysqlnd/mysqlnd_wireprotocol.c b/ext/mysqlnd/mysqlnd_wireprotocol.c index 64c2c7969619..a47884698d9c 100644 --- a/ext/mysqlnd/mysqlnd_wireprotocol.c +++ b/ext/mysqlnd/mysqlnd_wireprotocol.c @@ -1171,10 +1171,17 @@ void php_mysqlnd_rset_header_free_mem(void * _packet) /* }}} */ #define READ_RSET_FIELD(field_name) do { \ + BAIL_IF_NO_MORE_DATA; \ len = php_mysqlnd_net_field_length(&p); \ if (UNEXPECTED(len == MYSQLND_NULL_LENGTH)) { \ goto faulty_or_fake; \ } else if (len != 0) { \ + BAIL_IF_NO_MORE_DATA; \ + if (UNEXPECTED((p - begin) > packet->header.size || packet->header.size - (p - begin) < len)) { \ + DBG_ERR_FMT("Result set field metadata string length is past the packet size"); \ + php_error_docref(NULL, E_WARNING, "Result set field metadata string length is past the packet size"); \ + DBG_RETURN(FAIL); \ + } \ meta->field_name = (const char *)p; \ meta->field_name ## _length = len; \ p += len; \ @@ -1243,7 +1250,7 @@ php_mysqlnd_rset_field_read(MYSQLND_CONN_DATA * conn, void * _packet) READ_RSET_FIELD(name); READ_RSET_FIELD(org_name); - /* 1 byte length */ + BAIL_IF_NO_MORE_DATA; if (UNEXPECTED(12 != *p)) { DBG_ERR_FMT("Protocol error. Server sent false length. Expected 12 got %d", (int) *p); php_error_docref(NULL, E_WARNING, "Protocol error. Server sent false length. Expected 12");