From: Martin Hansson
Date: December 15 2010 11:00am
Subject: bzr commit into mysql-5.1-bugteam branch (martin.hansson:3525) Bug#58165
List-Archive: http://lists.mysql.com/commits/126915
X-Bug: 58165
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============1753400070=="

--===============1753400070==
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

#At file:///data0/martin/bzrroot/bug58165/5.1bt/ based on revid:gleb.shchepa@stripped

 3525 Martin Hansson	2010-12-15
      Bug#58165: "my_empty_string" gets modified and causes LOAD DATA to fail and
      other crashes
      
      The string manipulating function insert() used a shared string buffer intended
      to always contain an empty string. Fixed by copying the buffer before
      modifying it.
      
      Relevant code has also been documented.

    modified:
      client/sql_string.cc
      mysql-test/r/func_str.result
      mysql-test/t/func_str.test
      sql/item_strfunc.cc
=== modified file 'client/sql_string.cc'
--- a/client/sql_string.cc	2010-07-09 12:00:17 +0000
+++ b/client/sql_string.cc	2010-12-15 11:00:25 +0000
@@ -58,11 +58,28 @@ bool String::real_alloc(uint32 arg_lengt
 }
 
 
-/*
-** Check that string is big enough. Set string[alloc_length] to 0
-** (for C functions)
-*/
+/**
+   Allocates a new buffer on the heap for this String.
+
+   - If the String's internal buffer is privately owned and heap allocated, a
+     new buffer will be allocated, data moved and the old buffer freed.
+
+   - If the String does not keep a private buffer on the heap, such a buffer
+     will be allocated and the string copied accoring to its length, as found
+     in String::length().
 
+   For C compatibility, the new string buffer is null terminated.
+
+   @param alloc_length The size of new buffer to be allocated, excluding null
+   terminator. If the current string buffer is larger than this value, no
+   allocation occurs.
+
+   @retval false Either the copy operation is complete or, if the size of the
+   new buffer is smaller than the currently allocated buffer (if one exists),
+   no allocation occured.
+
+   @retval true An error occured when attempting to allocate memory.
+*/
 bool String::realloc(uint32 alloc_length)
 {
   uint32 len=ALIGN_SIZE(alloc_length+1);
@@ -206,6 +223,17 @@ bool String::copy()
   return FALSE;
 }
 
+/**
+   Copies the internal buffer from str. If this String has a private heap
+   allocated buffer where new data does not fit, a new buffer is allocated
+   before copying and the old buffer freed. Character set information is also
+   copied.
+   
+   @param str The string whose internal buffer was duplicated.
+   
+   @retval false Success.
+   @retval true Memory allocation failed.
+ */
 bool String::copy(const String &str)
 {
   if (alloc(str.str_length))

=== modified file 'mysql-test/r/func_str.result'
--- a/mysql-test/r/func_str.result	2010-12-14 16:08:25 +0000
+++ b/mysql-test/r/func_str.result	2010-12-15 11:00:25 +0000
@@ -2612,4 +2612,20 @@ CONVERT(('' IN (REVERSE(CAST(('') AS DEC
 1
 Warnings:
 Warning	1292	Truncated incorrect DECIMAL value: ''
+# 
+# Bug#58165: "my_empty_string" gets modified and causes LOAD DATA to fail
+# and other crashes
+#
+CREATE TABLE t1 ( a TEXT );
+SELECT 'aaaaaaaaaaaaaa' INTO OUTFILE 'bug58165.txt';
+SELECT insert( substring_index( 'a', 'a', 'b' ), 1, 0, 'x' );
+insert( substring_index( 'a', 'a', 'b' ), 1, 0, 'x' )
+x
+Warnings:
+Warning	1292	Truncated incorrect INTEGER value: 'b'
+LOAD DATA INFILE 'bug58165.txt' INTO TABLE t1;
+SELECT * FROM t1;
+a
+aaaaaaaaaaaaaa
+DROP TABLE t1;
 End of 5.1 tests

=== modified file 'mysql-test/t/func_str.test'
--- a/mysql-test/t/func_str.test	2010-12-14 16:08:25 +0000
+++ b/mysql-test/t/func_str.test	2010-12-15 11:00:25 +0000
@@ -1369,4 +1369,15 @@ DROP TABLE t1;
 SELECT '1' IN ('1', SUBSTRING(-9223372036854775809, 1));
 SELECT CONVERT(('' IN (REVERSE(CAST(('') AS DECIMAL)), '')), CHAR(3));
 
+--echo # 
+--echo # Bug#58165: "my_empty_string" gets modified and causes LOAD DATA to fail
+--echo # and other crashes
+--echo #
+CREATE TABLE t1 ( a TEXT );
+SELECT 'aaaaaaaaaaaaaa' INTO OUTFILE 'bug58165.txt';
+SELECT insert( substring_index( 'a', 'a', 'b' ), 1, 0, 'x' );
+LOAD DATA INFILE 'bug58165.txt' INTO TABLE t1;
+SELECT * FROM t1;
+DROP TABLE t1;
+
 --echo End of 5.1 tests

=== modified file 'sql/item_strfunc.cc'
--- a/sql/item_strfunc.cc	2010-10-12 19:28:03 +0000
+++ b/sql/item_strfunc.cc	2010-12-15 11:00:25 +0000
@@ -39,6 +39,12 @@ C_MODE_START
 #include "../mysys/my_static.h"			// For soundex_map
 C_MODE_END
 
+/*
+  This is an inherently dangerous design and should be abandoned. Our String
+  class does not support safely mutable strings. Functions that return
+  non-const String objects should either allocate buffers or use pre-allocated
+  ones.
+ */
 String my_empty_string("",default_charset_info);
 
 
@@ -1047,6 +1053,12 @@ String *Item_func_insert::val_str(String
     goto null;
   }
   res=copy_if_not_alloced(str,res,res->length());
+  /* This shows the dangers of returning my_empty_string. */
+  if (res == &my_empty_string)
+  {
+    str->copy("", 0, default_charset_info);
+    res= str;
+  }
   res->replace((uint32) start,(uint32) length,*res2);
   return res;
 null:


--===============1753400070==
MIME-Version: 1.0
Content-Type: text/bzr-bundle; charset="us-ascii";
	name="bzr/martin.hansson@stripped"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: martin.hansson@stripped\
#   vabuljud8s1t594x
# target_branch: file:///data0/martin/bzrroot/bug58165/5.1bt/
# testament_sha1: 2ae8bb0eee278b9498da11175e870f84f268c42a
# timestamp: 2010-12-15 12:00:28 +0100
# base_revision_id: gleb.shchepa@stripped\
#   zmp7zt8io3j83oi0
# 
# Begin bundle
IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWS3XvvMABOXfgGAwWff//3/n
3sC////6YAuH3jzeu6e5oDL0DiXbbG2LbW2bMDFVJJDCUQgJkwSekeKabUxPUZTE9TQyekaaPUDQ
Gg0k1PU02jSaNAAAAAAAAAAA1TyNKbSn6kxGQPUYANB6oZPUAxGEGBAkSEEE2jRDRgkeqb1Gp+pl
PFDT1GQD1NMnqAcZMmhiMTRgEYCYQBgJpo0yNAMJIgIBMmgECYhqbQmo8o0GMoABspkHkO/JdSJ7
V+9j1lt/mpPZiVsa/Xf/E33ySwkfn40/mZOBVWDOM8p4V36KcsPFX5RaacZbFPipHYOc80CvKyPn
0Ela6mME3s64ZRErGAu9jDW+yEZYeEpxytnTnuk2e1LQYFNAmNoG022MbMq6Q49a1r8SkLwBq46n
5dCKxswmvGuIeQc4MGxtsbWEN3xRwpj/qIqXGRurIoMHXKT2Tkba4JDihCh6o2bse/Klu2GnRP3Q
BLoWsoZRzRgYwgdA3MFOUuYZtOoPgQi6PxvHO7LIQ3PKSaWuKRE4OhUFItAdA3xAdIWfcU1RoJgc
2gjiDG/Vh3Is8Xg4T3/liVjMyuTjtZiUxwsgXxJUxUMQ4DbnpSWyasEZ8FGDjZwyq0DMLpMsc5lg
GH66Ad6sTio4BDBvdQxiVVGa68nFlApO2UsSE7gjcTvskh50sqKx6DtnXnqWF86upqD92Pmy3AXG
TKr5RfNjw3CyJEYdaA4grvb9KaTCcLYfIkCvLilxUgSdBvewDCdQR6dgFTQZU0D8Btt3J+dGmTMh
aJlACpqiZJcj9clTwjFphRvgOs90DTWv6hXrwWea2ZMkVPZzbKpCsLQjDoRAXN7COPFqz+oIWrgX
jw+U+tpQ58UwV15vUuxutuGFsCLNJ2epDhaeE9zDGeuogrcRQrZnenctDrXHkwwUraHoCnpmBwYZ
mZC3DR6g0hnCJgudnpvLaFQNAUEQDEocIURpQC1leTTqwmQqRmFJ3BE2n7BOrLmJdChxLiT0v3Du
2wCl3MhizNG2UYp13Z1BXyZJH7YuECDSedDmZ5UNoJjIjLW4SJfldUizHrMcZUP5SGlBPfmSE6Ix
Dxw2yAJaFNEsLpn4anaxDxB9G4CtFzK6Zg1FhxiBzO8FyJLXEMWSDpqtzNgcpoRcNiSu3F5vBNgS
DqK0mORo5wwdB1F5jcOW+bEDPA4nMjn9jIysyMM4RuhIJbCAZJ1GNlqPcLbFKoxlzLhsQsgygwKB
pF/Z7zYnDhKXJWQksBWTgXDkytQvhYTgsATjCiUXgxFtma17FE8sKHMibTtj9TTd7B3LQVbFoXdT
0EvFoBlWYBNZ6hC4OaUG0BoCHCUl0BJIZIKYeCQBmKKEAUACnB0suxd3YSWR9ChxMqeJ9h7bCzHm
1uVxBonYa9CkdhOkrB5Jdh201kYRfhtFxMSuMD1lxbtJ5mfO3azZsF5DUmT26lDxPHuNbFmTM4ct
w83FfAc2Y0c8LzyVcsyekOJAtL0tZwlReSzHCwluUS605Z4aTqsYm46KgLTwplhgF0MotdfQ0aJU
F4UqHXoWhUoSOhKsDQwNLzv7LV1RqZNFpwjAxsbZhVPTwyIIq4majEURalZa6js1RCZ2jjsRr782
qovGxtdoa0EV8/cG/29PA9CwIVgsXikfgj4I+8ocl6Y2m39wpzNrmSFBNDRI+YWjF4lkCl5i+UFf
mQQjA/mDJH/Q8xRFeRWwyeaVQvRiEVMr/MuMy5KYET6jBoiAmMnP0HB5svzuIFxu+aODhwD3p3S7
o9PwgCRtViNqnsyMxwpDRv7PWKI4mIXYw9ptJCA6qKDtF7UoEomO6icp1gjBsO1ECTL19g0ciVBb
0VwfPpeH3sgoMSGqUBqwORB0jdgW/pNCtyBYaJ5RtI0/n/pPywoUH8ZMCkWUh8hj6WxGZw3SwNbC
wfiq3OR0cidlwVIbjwN8Y5xq3dvxMfhA/jLdZruIGoVGV3ffLuJkvmvZVINQoC6EPuAND1MHNLYa
7/WSI+WelTA4QSgy1YMVn6Re01smrSwtJfQxwISw+3qXI1DCWbiDzvhmCObkWD/jGWDq2yAz5qFB
13ldQ2g6GxkKHA02mGgGg2HKS6bbTj6mEpcV2SiRkYlo1wI8gcdQZYiwrheyqxNRd5ImF/hGgUwn
DsXrrvaA/AH9w0mVnQOcoqjzh7xiEXROybiRgWm3q49WnXgXG4r0+9saObCRTMPMLDFlc+YWpaId
dpzp07g2fdccc+oOftkvTvewZaM9GTuhbYaoDhKleu+hkKGkhjJJjk0a20SqYKd8sz4qdeaMPWjZ
bO5aOdosZpTFVWsQ200iyzbOYcu6rhQ9HYukkWZtbIK1WEFT1rPnJ8em2iGkqmeCgJ2SsSqIkmIk
CAixSKp4Ixc4EJyDDYcg7McTio1GhxOJpHqOSPDArBIMkdSQeR1QX1D2hgihM5kMRnGUBGpLtJYK
CRiTA6oD6ZBqKF1rScyxQYjOSJrL4CK4o2OQiOKbluUXpF0iDSVBNzXUmkfcnHEvmwxpIPjRSQ+N
VAoN8JnkMFJnQbf2JFXwTm3k/J8d/mgb0ck02pt/y3kg1NnYtO24FD07i4tf+UhCrJ2JXLlcs1/e
0WOHK6JTgQTpW0svq1T4KESpEWHpIgVUVVtRPSyt1pz3DO51fHqLzgToPGkR+czwpyl8WxJoQRdJ
NNm2zkWdE168CrCyb0IpggqjaykPUVKMimyQRmszTbp2DcBqBwkIHA6Jszd425a5Yj22a4ITjeMA
X5soZlqSgEA0IVibDx4kOMDIlE3AiAzLhp/U8EMG719YWBPOW46yUzjsyPUbPReuWg9vcp4oa+aX
pCyu1fV/tIppYyytMOuTshmGY1mhOkFFaEB4iKk4wTpDpVZPTuZVD2sSQgkWDlE10FHImK6NpbUK
CTtYk7QBkNirIXHRo6NwapMkwnUtZ0bTx1mKSSa+jqqC+Smuc8DmXeTKrEkXcgpmWBSQnLBuiVRX
JwdzDjK+hn2iBpSKmD4GaY3zZFvtOnhQwYoLao162p6gmUFLI9LgwoiuLmXAhyzGOkKhWMEt/WWB
1TM/sOXXu5KjRnS9E+vCcxnOYyZuSQlJlGENtOXXDqBsLx+EZCCjJMbqSOpUmNjjzswOhCCGm4vh
8vBajYF22SMyVtrWeSC5oC5ZKTBmqUKuQ5PrpnZZnZbYOms74xRASew+PYQR5E0vQNRxmOFvSJ/G
RcFE+A0GNLZQjKse35Atg8J9/hwrrP7jbp+obotAhW3TRv0hPYuZQsWhKCgeq8PYydxmEu0O5ttt
49UQQY1jMySYrksHpeQbVgQKENrgoysNtV4wsld6tmHGxpWxhpP5BILIESeLRvAofgC+Xcr+ltgY
32ANMnKEAlcTgkS0eK4xzfEgpdFeRAe5QkFl6cLtrm8+gYlG142DpiLPY0UzJTVUIXxYMbYNtobC
VDFeqqYahWVVC0GUMc2hjdvtWHa53gt2I3Eb3ckg/JMF5Ew3kQW0871rueCDBvhXoZzK/Swl1jqn
MmHlOXxJngCqFTr1Us1oa9eNEu493nBK6iZfJqxLuRgPby9p4O6Peqpw0MNvw94xO/9ZXUjYER0H
Etv3u7ZtTfB4lBuaIIlOZ/OtBpsniTQZS8verRKY0o93UXpkL9t5/xdyRThQkC3XvvM=


--===============1753400070==--