From: Martin Hansson Date: December 16 2010 12:36pm Subject: bzr commit into mysql-5.1-bugteam branch (martin.hansson:3525) Bug#58165 List-Archive: http://lists.mysql.com/commits/127062 X-Bug: 58165 MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0236305646==" --===============0236305646== 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-lazy_copy/ based on revid:gleb.shchepa@stripped 3525 Martin Hansson 2010-12-16 Bug#58165: "my_empty_string" gets modified and causes LOAD DATA to fail and other crashes The string manipulating SQL function insert() used a shared string buffer intended to contain an immutable empty string causing errors. With our code base this design faced two obstacles. The implementations of string valued SQL functions return pointers to String objects rather than the objects themselves and even if this were rectified we still have no protection against manipulation of shared string buffers. Fixed by always allocating a new String object whenever a function returns an empty string. 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 sql/sql_string.h === 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-16 12:36:35 +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-16 12:36:35 +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-16 12:36:35 +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-16 12:36:35 +0000 @@ -461,7 +461,7 @@ String *Item_func_des_encrypt::val_str(S if ((null_value= args[0]->null_value)) return 0; // ENCRYPT(NULL) == NULL if ((res_length=res->length()) == 0) - return &my_empty_string; + return new String("", default_charset_info); if (arg_count == 1) { @@ -652,7 +652,7 @@ String *Item_func_concat_ws::val_str(Str } if (i == arg_count) - return &my_empty_string; + return new String("", default_charset_info); for (i++; i < arg_count ; i++) { @@ -803,7 +803,7 @@ String *Item_func_reverse::val_str(Strin return 0; /* An empty string is a special case as the string pointer may be null */ if (!res->length()) - return &my_empty_string; + return new String("", default_charset_info); if (tmp_value.alloced_length() < res->length() && tmp_value.realloc(res->length())) { @@ -1137,7 +1137,7 @@ String *Item_func_left::val_str(String * /* if "unsigned_flag" is set, we have a *huge* positive number. */ if ((length <= 0) && (!args[1]->unsigned_flag)) - return &my_empty_string; + return new String("", default_charset_info); if ((res->length() <= (ulonglong) length) || (res->length() <= (char_pos= res->charpos((int) length)))) @@ -1181,7 +1181,7 @@ String *Item_func_right::val_str(String /* if "unsigned_flag" is set, we have a *huge* positive number. */ if ((length <= 0) && (!args[1]->unsigned_flag)) - return &my_empty_string; /* purecov: inspected */ + return new String("", default_charset_info); /* purecov: inspected */ if (res->length() <= (ulonglong) length) return res; /* purecov: inspected */ @@ -1220,7 +1220,7 @@ String *Item_func_substr::val_str(String /* Negative or zero length, will return empty string. */ if ((arg_count == 3) && (length <= 0) && (length == 0 || !args[2]->unsigned_flag)) - return &my_empty_string; + return new String("", default_charset_info); /* Assumes that the maximum length of a String is < INT_MAX32. */ /* Set here so that rest of code sees out-of-bound value as such. */ @@ -1231,12 +1231,12 @@ String *Item_func_substr::val_str(String /* Assumes that the maximum length of a String is < INT_MAX32. */ if ((!args[1]->unsigned_flag && (start < INT_MIN32 || start > INT_MAX32)) || (args[1]->unsigned_flag && ((ulonglong) start > INT_MAX32))) - return &my_empty_string; + return new String("", default_charset_info); start= ((start < 0) ? res->numchars() + start : start - 1); start= res->charpos((int) start); if ((start < 0) || ((uint) start + 1 > res->length())) - return &my_empty_string; + return new String("", default_charset_info); length= res->charpos((int) length, (uint32) start); tmp_length= res->length() - start; @@ -1299,7 +1299,7 @@ String *Item_func_substr_index::val_str( null_value=0; uint delimiter_length= delimiter->length(); if (!res->length() || !delimiter_length || !count) - return &my_empty_string; // Wrong parameters + return new String("", default_charset_info); // Wrong parameters res->set_charset(collation.collation); @@ -1648,7 +1648,7 @@ String *Item_func_password::val_str(Stri if ((null_value=args[0]->null_value)) return 0; if (res->length() == 0) - return &my_empty_string; + return new String("", default_charset_info); my_make_scrambled_password(tmp_value, res->ptr(), res->length()); str->set(tmp_value, SCRAMBLED_PASSWORD_CHAR_LENGTH, res->charset()); return str; @@ -1672,7 +1672,7 @@ String *Item_func_old_password::val_str( if ((null_value=args[0]->null_value)) return 0; if (res->length() == 0) - return &my_empty_string; + return new String("", default_charset_info); my_make_scrambled_password_323(tmp_value, res->ptr(), res->length()); str->set(tmp_value, SCRAMBLED_PASSWORD_CHAR_LENGTH_323, res->charset()); return str; @@ -1700,7 +1700,7 @@ String *Item_func_encrypt::val_str(Strin if ((null_value=args[0]->null_value)) return 0; if (res->length() == 0) - return &my_empty_string; + return new String("", default_charset_info); if (arg_count == 1) { // generate random salt @@ -1961,7 +1961,7 @@ String *Item_func_soundex::val_str(Strin for ( ; ; ) /* Skip pre-space */ { if ((rc= cs->cset->mb_wc(cs, &wc, (uchar*) from, (uchar*) end)) <= 0) - return &my_empty_string; /* EOL or invalid byte sequence */ + return new String("", default_charset_info); /* EOL or invalid byte sequence */ if (rc == 1 && cs->ctype) { @@ -1986,7 +1986,7 @@ String *Item_func_soundex::val_str(Strin { /* Extra safety - should not really happen */ DBUG_ASSERT(false); - return &my_empty_string; + return new String("", default_charset_info); } to+= rc; break; @@ -2259,7 +2259,11 @@ String *Item_func_make_set::val_str(Stri ulonglong bits; bool first_found=0; Item **ptr=args; - String *result=&my_empty_string; + String *empty_string= new String("", default_charset_info); + if (empty_string == NULL) + /* Out of memory */ + return NULL; + String *result= empty_string; bits=item->val_int(); if ((null_value=item->null_value)) @@ -2283,7 +2287,7 @@ String *Item_func_make_set::val_str(Stri else { if (tmp_str.copy(*res)) // Don't use 'str' - return &my_empty_string; + return empty_string; result= &tmp_str; } } @@ -2293,11 +2297,11 @@ String *Item_func_make_set::val_str(Stri { // Copy data to tmp_str if (tmp_str.alloc(result->length()+res->length()+1) || tmp_str.copy(*result)) - return &my_empty_string; + return empty_string; result= &tmp_str; } if (tmp_str.append(STRING_WITH_LEN(","), &my_charset_bin) || tmp_str.append(*res)) - return &my_empty_string; + return empty_string; } } } @@ -2436,7 +2440,7 @@ String *Item_func_repeat::val_str(String null_value= 0; if (count <= 0 && (count == 0 || !args[1]->unsigned_flag)) - return &my_empty_string; + return new String("", default_charset_info); /* Assumes that the maximum length of a String is < INT_MAX32. */ /* Bounds check on count: If this is triggered, we will error. */ @@ -2744,7 +2748,7 @@ String *Item_func_conv::val_str(String * ptr= longlong2str(dec, ans, to_base); if (str->copy(ans, (uint32) (ptr-ans), default_charset())) - return &my_empty_string; + return new String("", default_charset_info); return str; } @@ -2911,7 +2915,7 @@ String *Item_func_hex::val_str(String *s return 0; ptr= longlong2str(dec,ans,16); if (str->copy(ans,(uint32) (ptr-ans),default_charset())) - return &my_empty_string; // End of memory + return new String("", default_charset_info); // End of memory return str; } === modified file 'sql/sql_string.h' --- a/sql/sql_string.h 2010-10-19 22:36:59 +0000 +++ b/sql/sql_string.h 2010-12-16 12:36:35 +0000 @@ -82,6 +82,10 @@ public: Alloced_length=str.Alloced_length; alloced=0; str_charset=str.str_charset; } + static void *operator new(size_t size) throw () + { + return sql_alloc(size); + } static void *operator new(size_t size, MEM_ROOT *mem_root) throw () { return (void*) alloc_root(mem_root, (uint) size); } static void operator delete(void *ptr_arg, size_t size) --===============0236305646== 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\ # 6imapwin9eq5uykz # target_branch: file:///data0/martin/bzrroot/bug58165/5.1bt-\ # lazy_copy/ # testament_sha1: fade7922d31a0630983b9948deea0d3649aa0762 # timestamp: 2010-12-16 13:36:39 +0100 # base_revision_id: gleb.shchepa@stripped\ # zmp7zt8io3j83oi0 # # Begin bundle IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWcWUgHAABszfgHEwWPf//3/n /sC////6YA49bu6LcFzvljKkBT1Wdd3drY1Sndi6mZDCqpOtIg4SSQjCaU8ExkKfoapsjNEEGBNG mRtRpobUEoQmjJpoSIzTJPUPRGjQNAAAADICKemJNQyNNNTIxoCYjNJkeoMExMIyGnqGEiIITSZM jIZEzVTPNVHtKemJNNNMgAAG1DgGEYTTEMAgGQAwjTJkwjAQ0EkgEE0wJompshomjVP0U8IxQHlP U0ABosIXGzuoPw6+tV2o+1n9+Fu3vnlg+6FF9MqjbS/665OcNv3fj/eq8vZw2pFRZvCW/xv/x9Xl OzNFSPel2JvLRwasJquNfq10tzH5PeyB2t5iS9gnuC9lmEXl4XUposAC5UGuVDD1qDM8Bmqu/SUV hQqMBpKRtZnKmOMYgy23qy2SQr6aYJV7SBIUBzYgY0IhIYwbG2dq8A2qheA+5dIMrxv87j2ZEIzF v2y1bH2tGA50eaMaGQNgm02Nib1rl/sQu+uvjEWXfNhqj8KQQ/KkLBlJnxvWg5d+V5bwd3g/JvGI fK8XxgnKk5Uhutr41ydhvKYweLMnjSLvEc5Wkq9OzYlu8ju50+9GutscPxdZgoDoIzUo+qHwJMtu Urw8PG84QPRPaTxJDuCbyoq6LcQIjxu+AvhOkfOffgYQGaIQELccoAhAXBcRC4uzjFIdJwzF6s9n sgteD3YThORFFLqIgRpS11mmAPpGD5ouA5DDLozvqnir3ErKa1UzxqOeV74/mhguA8UM4wL+Yi5K 1QW5ETcUaqxiTrB1xJVGvsuK0g6LJAlnm+0332TxQlUnTY01hYI62NHGEY1fpK2bzVgXK3AmjYH0 UXfvyEMkkirVMbmoPV7QpB3jhu2RuNE4BLXxSNi0GAkGcEWCMFI0OV3JmdmANfyZUMsbbQSeJfXy 8RB4CDzSMGg60wT/UNtvqWjY0yhsMhMsAWaqmSpEERsha/yenR9leU98qOYBSQg5nI8MDJZ/gLCu lPKm3bdMIGpl5KLBMF8iGIwqEhu4+Yl9DZ3/MJebR8x0yRMskeEDbEoBjI0UOxutmdPWBBiY437D xKZsMYZb0IrtJKKSsmd6c4jDIGQn7mxHYORHYso4kTX8EvGzRCJ2ZqmQ5hLsaHMArXICp6hiK4Ra 2EqgmkEoUA0A38blMsC3pdKz0l1Q9pQrcvbIxDCsWFcrpShfTnvMDvLmcX1gh6XCTdm+RHQ256FM rSnpvsQlc5yX5RjGZzOe2RixTEcGBKedloCkGQkxJt90eUsh+7GgbRga6Zs3E0oEWUnL032QiGP4 6GVdxYSoEjTyZfVBcpFjX0fbYPJlsL2JPQTI9TcRZIwKrtg82MWGSKzi4R6VAy3TztZ7nxwDWxId 10lzhRJGQ/mMwsjqGM0QqDYlLIXaaS0qrbRUmSSSOJe+/TYDUYmZGj4UNBJRX6G84kDdzNppfbls 4NHFOMhz3Pnc+IRyJDDUiJudk64TmOdQSfwYSkNyibaWVQtG62pU1B9icYHkXBA3n5se89q4Hl3H oW8oWzMi++ZpptUIM0HuOEEFSYVlTjPMMVaQIqY3G3Ps5m95SxnCzHkuqZgSXUbcin/jDU9o6DyZ 6Coyx0MaaDbIiVhhrAJ74uCEKNrmNHIyLQhpbVwrNwVTWqltO9ldE4h+xaGJawbzB6ycufO4vJMS HHZdLtO8dXyErTSsrta6kRt4Pv47aYmXSQJWsWegy76UxEsy8Fcx4F8RMi5NId2VO6SV48cM3DRL A5ctzzLLrPad4lganA0oYZWG7VzRzueJw4hMpqToSLChXvsKV3HYeg1grXTGpPIWYxqgxvF0mRMX DE5EjEudkKP21ONM1a1M86D+IOImRek6Eyd56VCkGZ1CQTqio16ExlJik5aIG+Rea0x24X1bBVXB EtrWxipujVfY5UoXNTlIXOcy0YUy03EzUiTKmnmbCspHfw8hLZ9HshU8cDB5Mqy+yuqLRt04KKeW wkWUJN3ZFRYgYYv3l2BoBww+LIaZ8Bwvzi4fX7MLQ9qaaOBGRIcV5+ZFWKKYzqNJQif60jtR3I7S AlVwZkwZ9wqUPB0JFBRDRJvDEYvuMIFP/BW/xIx+xhi1ZL+QVEOcrI5w7YkROguLUiZNFEUmFSKX hBTNpD8i4yLUpiSgfmMGaHiYxcXjgcVm7niwPj5IT4BB9AmciZGZOEDgOV23ucIk8VgGImi9fZmz TClti93MX5SMwWRgafU+J7T5KK9wyutPpbkX+eHxs+iERa/P8JHuLyqUwIhTh/E4L3o/Dv+ehCxG VTyLoZlJxUwG9ixPxqksNgPiVcmgvJhkLJ3m8fEG6FJOLstEifICcsmJBkzmlG7LbEhFw+D52ZiD jMIFtpcvjZLmGid1sfU+9pWSOh9in1HngMSOX7z1HP0FRM1VZr4TJETpcoVFD6Q+ypQQbg1HF4kF mp41OD0JMamt+BhEoPEp0FDHE5kIhqzRpDcYn0iZzJzNx1QasVennA2Ji4nP7lqwP4EaYeQg7lxN Asiwkfe3cbkkcYFk9WeUprFkqIOqpUeW8vZvNiOAyJQ4ZMMmSxQwHHjUtDQcU0qSqKJSJEMyaRC4 xLTmciZiVTEGTOVW08RSV/bGauqk/gurCrNoEcF5z5s7OloxrQLxaeb9RlKODm1apoxELpMzfrn3 WnSo6FJm3Kubosc7TUc8vJnsOss2Yo8kX4EmBskWgSzQ9XmHItO4Mz183XPGL8uYdfoivTvdUcLo uem6rObN7Zmk28dpqTks+ScRsKiA+hUzyOSWvOTZvRx2Yb8YylpNrPcWvdsTwbtJLlzRP2FkzMKU aumShkvESw9alBOEV1v9bQDlwl6SPnfYu6SMhiuAUotj1H2GW1ru3OSGzCiQTfioAk+7teAyQrSt QKkiZSy1My+yczeIjsKnHQ8DqIYj/YSeRGOMPK+AYTCCqZIWsksixiCAxDweqKURiiKymSCdDPqm Ajgl/MkwqCWaaQvGA+qUGvgajKUDRJiwUEq5DLGRYi4cr9AyjJrRmxtYYYdsGpBPqiS++Sgg2mgc qCwH8GhsZsqE+hdQiLqEfKh6DCvA3W4gAyDNQbOHdJQ7p3TO/2pD8u4Gg1MvNjogfmac1lfYkn/D QpzEmLE3/JOSURKhGoDBasIH56Kh8XJbWMGIMIRHSqgbHOW7N2jHBrUiCUBiQ2EKZgYxgczpshxW zo6opsukpMK1HfdEVrZTCpylRVK4ZDQWrRRz2yLOBVVxl6jToEuW/2JjFDDD96zCRUTYU8IhCSWJ licuw4HCiN6EMI602Jnr4W4UjftHaQrde5N4PSPqwPwK0k8HiRkPpBXlu/gQ4PhGBokh5zHOXDK/ AS/md4mRu9XMLUUVecqotzO19DqyMTxMauhcrgavopXCnGZcF5BjGnavnU0Y4vFw8CV4aq0tCqgG m3EDHtukoYk0IotSJIKCLXL1oEC82iQKtYcnOKSPVqoCKNIRkOqzkwNZdWqQZljIeJZGAA3iBLJQ wbSuQHr19fDkHgS5ZFr4iCa5j9TtPw6zYD0ky8ehJZPICocD4G9az6PP3mBlmIXVsQUGlCSccqyV in2RJ8WvdXOu+X9EIYB8mBo7M1EZurMj66zl5WPsoV0hidnYyi9JNSqah+jQ0qJVzWY12FDuvsjv oUWLpILeTsEuZWukjQ8zAS15dL5m7MF3wPZaV7TQctux2TCloYyrRDB+51UmszI31Qi7Aa3Wody3 XLs6M+6uRJwKSJtL5Lx/z+I2wPm0Q8W/EOnykWaW2xlo8SwZAjesIwBjkPSQ5THjHMhPL4V15jdd tC0GZ1gg8HocAPsPl2j0vMlYwu8ajuKtmwx7GmFg0FczOdFwzGx7UOvDbxR0W8pjhzG5dp1LAIJ9 4g8Yi4IgdFWtlMwIr5zQ3rzR7WI4mYBuPkj9Dbbbb27DhhSRsk6TpLIBo9rudQDGCKAzkDJmSIrF 4aTDwpvJX2+WzqqbfdCOdPkFwWPIEptfeBQ9/NT27BwgojaILKCGkSdVjlV6tJfXXW0F3Vdk0joJ WLtxWkqIfbiyUgyVqSL4E0Z1Z6txIBQ9lYbQ2+N+jIDsLGQFFeEj+o0mNjQwQ0mwJsaelWK52Swt YEp7jELiLIGNxrYtl7jO4bcMfp93UIPsmC0ibDI/RYGYg0c9BsZa9RfGcKqEuICctZB92PCRHvEv UfiNYqwvsXXkrZPimyzG65EAOw5lfl+uySQtxGtML5tqJWyJvL7zkW3cfe3sVaoXev38BiNvcbh9 AeOSOJgWljb2xfQwaBQbLrUEFxTwZhLA2z24VZ9xBZGZIwGcxw3xEDXbCu9mozq+Zs8mU/4u5Ipw oSGLKQDg --===============0236305646==--