Fixed very rare memory leak in mysqlnd, when binding thousands of columns

diff --git a/NEWS b/NEWS
index 260bbea5b3bc6572ce4ee6eecf35abb2510e76d7..0e6b61433d24cc6fa7d66223d998481c54f0069a 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -16,7 +16,9 @@ PHP                                                                        NEWS
 
 - Implemented FR#35638 (Adding udate to imap_fetch_overview results).
   (Charles_Duffy at dell dot com )
-- Fixed possible buffer overflows in mysqlnd_list_fields,  mysqlnd_change_user
+- Fixed possible buffer overflows in mysqlnd_list_fields,  mysqlnd_change_user.
+  (Andrey)
+- Fixed very rare memory leak in mysqlnd, when binding thousands of columns.
   (Andrey)
 
 - Fixed handling of session variable serialization on certain prefix

diff --git a/ext/mysqli/tests/mysqli_stmt_bind_param_many_columns.phpt b/ext/mysqli/tests/mysqli_stmt_bind_param_many_columns.phpt
new file mode 100644 (file)
index 0000000..ce966d8
--- /dev/null
+++ b/ext/mysqli/tests/mysqli_stmt_bind_param_many_columns.phpt
@@ -0,0 +1,76 @@
+--TEST--
+mysqli_stmt_bind_param() - Binding with very high number of columns
+--SKIPIF--
+<?php
+require_once('skipif.inc');
+require_once('skipifemb.inc');
+require_once('skipifconnectfailure.inc');
+?>
+--FILE--
+<?php
+       /*
+       The way we test the INSERT and data types overlaps with
+       the mysqli_stmt_bind_result test in large parts. There is only
+       one difference. This test uses mysqli_query()/mysqli_fetch_assoc() to
+       fetch the inserted values. This way we test
+       mysqli_query()/mysqli_fetch_assoc() for all possible data types
+       in this file and we test mysqli_stmt_bind_result() in the other
+       test -- therefore the "duplicate" makes some sense to me.
+       */
+       require_once("connect.inc");
+
+       if (!$link = my_mysqli_connect($host, $user, $passwd, $db, $port, $socket)) {
+               printf("Cannot connect to the server using host=%s, user=%s, passwd=***, dbname=%s, port=%s, socket=%s\n",
+                       $host, $user, $db, $port, $socket);
+               exit(1);
+       }
+
+       if (!mysqli_query($link, 'DROP TABLE IF EXISTS ps_test')) {
+               printf("Failed to drop old test table: [%d] %s\n", mysqli_errno($link), mysqli_error($link));
+               exit(1);
+       }
+
+       $cols = 2500;
+       $str = array();
+       for ($i = 1; $i <= $cols; $i++) {
+               $str[] ="a$i INT";
+       }
+       $link->query("CREATE TABLE ps_test(" . implode(" , ", $str) . ")");
+       if (mysqli_errno($link)) {
+               printf("Failed to create the test table: [%d] %s\n", mysqli_errno($link), mysqli_error($link));
+               die("");
+       }
+       $stmt = $link->prepare("INSERT INTO ps_test VALUES(".str_repeat("?, ", $cols-1) . "?)");
+       var_dump($stmt->id);
+       $eval_str="\$stmt->bind_param(\"".str_repeat("i",$cols)."\", ";
+       for ($i = 1; $i < $cols; $i++) {
+               $eval_str.="\$i,";
+       }
+       $eval_str.="\$i";
+       $eval_str.=");";
+       eval($eval_str);
+       var_dump($stmt->execute());
+
+       mysqli_stmt_close($stmt);
+
+
+       mysqli_close($link);
+
+       print "done!";
+?>
+--CLEAN--
+<?php
+       if (!$link = my_mysqli_connect($host, $user, $passwd, $db, $port, $socket)) {
+               printf("Cannot connect to the server using host=%s, user=%s, passwd=***, dbname=%s, port=%s, socket=%s\n",
+                       $host, $user, $db, $port, $socket);
+               exit(1);
+       }
+       if (!mysqli_query($link, 'DROP TABLE IF EXISTS ps_test')) {
+               printf("Failed to drop the test table: [%d] %s\n", mysqli_errno($link), mysqli_error($link));
+               exit(1);
+       }
+?>
+--EXPECTF--
+int(1)
+bool(true)
+done!

diff --git a/ext/mysqlnd/mysqlnd_ps_codec.c b/ext/mysqlnd/mysqlnd_ps_codec.c
index 940d7cf02d98b0918d614f2b2b78ec809a6ed975..d66e90c039e7ef8f7931e11148c8f34c4660b041 100644 (file)
--- a/ext/mysqlnd/mysqlnd_ps_codec.c
+++ b/ext/mysqlnd/mysqlnd_ps_codec.c
@@ -599,6 +599,7 @@ mysqlnd_stmt_execute_store_params(MYSQLND_STMT * s, zend_uchar **buf, zend_uchar
 {
        MYSQLND_STMT_DATA * stmt = s->data;
        unsigned int i = 0;
+       zend_uchar * provided_buffer = *buf;
        size_t left = (*buf_len - (*p - *buf));
        size_t data_size = 0;
        zval **copies = NULL;/* if there are different types */
@@ -714,9 +715,17 @@ mysqlnd_stmt_execute_store_params(MYSQLND_STMT * s, zend_uchar **buf, zend_uchar
                *buf_len = offset + data_size + 10; /* Allocate + 10 for safety */
                tmp_buf = mnd_emalloc(*buf_len);
                memcpy(tmp_buf, *buf, offset);
+               /*
+                 When too many columns the buffer provided to the function might not be sufficient.
+                 In this case new buffer has been allocated above. When we allocate a buffer and then
+                 allocate a bigger one here, we should free the first one.
+               */
+               if (*buf != provided_buffer) {
+                       mnd_efree(*buf);
+               }
                *buf = tmp_buf;
                /* Update our pos pointer */
-               *p = *buf + offset;     
+               *p = *buf + offset;
        }
 
        /* 2.3 Store the actual data */