From: Andrey Hristov <andrey@php.net>
Date: Tue, 27 Apr 2010 12:32:34 +0000 (+0000)
Subject: Fixed very rare memory leak in mysqlnd, when binding thousands of columns
X-Git-Tag: php-5.3.3RC1~250
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=3d900d4cadc5ef8ea74911095b0a608e76c9fbee;p=php

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

diff --git a/NEWS b/NEWS
index 260bbea5b3..0e6b61433d 100644
--- 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
index 0000000000..ce966d8761
--- /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 940d7cf02d..d66e90c039 100644
--- 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 */