1berry/MariaDB-server
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Bug#58165: "my_empty_string" gets modified and causes LOAD DATA to fail and

Browse Source 
other crashes

Some string manipulating SQL functions use a shared string object intended to
contain an immutable empty string. This object was used by the SQL function
SUBSTRING_INDEX() to return an empty string when one argument was of the wrong
datatype. If the string object was then modified by the sql function INSERT(),
undefined behavior ensued.

Fixed by instead modifying the string object representing the function's
result value whenever string manipulating SQL functions return an empty
string.

Relevant code has also been documented.
This commit is contained in:
Martin Hansson 2011-01-13 08:57:15 +01:00
parent f23725f2bb
commit 3ef71bfac7
6 changed files with 107 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
mysql-test/r/func_str.result
View File

@ -2612,4 +2612,20 @@ CONVERT(('' IN (REVERSE(CAST(('') AS DECIMAL)), '')), CHAR(3))
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

11
mysql-test/t/func_str.test
View File

@ -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

45
sql/item_strfunc.cc
View File

@ -39,6 +39,9 @@ C_MODE_START
#include "../mysys/my_static.h" // For soundex_map
C_MODE_END
/**
@todo Remove this. It is not safe to use a shared String object.
*/
String my_empty_string("",default_charset_info);
@ -461,7 +464,7 @@ String *Item_func_des_encrypt::val_str(String *str)
if ((null_value= args[0]->null_value))
return 0; // ENCRYPT(NULL) == NULL
if ((res_length=res->length()) == 0)
return &my_empty_string;
return make_empty_result();
if (arg_count == 1)
{
@ -652,7 +655,7 @@ String *Item_func_concat_ws::val_str(String *str)
}
if (i == arg_count)
return &my_empty_string;
return make_empty_result();
for (i++; i < arg_count ; i++)
{
@ -803,7 +806,7 @@ String *Item_func_reverse::val_str(String *str)
return 0;
/* An empty string is a special case as the string pointer may be null */
if (!res->length())
return &my_empty_string;
return make_empty_result();
if (tmp_value.alloced_length() < res->length() &&
tmp_value.realloc(res->length()))
{
@ -1143,8 +1146,7 @@ String *Item_func_left::val_str(String *str)
/* if "unsigned_flag" is set, we have a *huge* positive number. */
if ((length <= 0) && (!args[1]->unsigned_flag))
return &my_empty_string;
return make_empty_result();
if ((res->length() <= (ulonglong) length) ||
(res->length() <= (char_pos= res->charpos((int) length))))
return res;
@ -1187,7 +1189,7 @@ String *Item_func_right::val_str(String *str)
/* 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 make_empty_result(); /* purecov: inspected */
if (res->length() <= (ulonglong) length)
return res; /* purecov: inspected */
@ -1226,7 +1228,7 @@ String *Item_func_substr::val_str(String *str)
/* 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 make_empty_result();
/* 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. */
@ -1237,12 +1239,12 @@ String *Item_func_substr::val_str(String *str)
/* 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 make_empty_result();
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 make_empty_result();
length= res->charpos((int) length, (uint32) start);
tmp_length= res->length() - start;
@ -1305,7 +1307,7 @@ String *Item_func_substr_index::val_str(String *str)
null_value=0;
uint delimiter_length= delimiter->length();
if (!res->length() || !delimiter_length || !count)
return &my_empty_string; // Wrong parameters
return make_empty_result(); // Wrong parameters
res->set_charset(collation.collation);
@ -1654,7 +1656,7 @@ String *Item_func_password::val_str(String *str)
if ((null_value=args[0]->null_value))
return 0;
if (res->length() == 0)
return &my_empty_string;
return make_empty_result();
my_make_scrambled_password(tmp_value, res->ptr(), res->length());
str->set(tmp_value, SCRAMBLED_PASSWORD_CHAR_LENGTH, res->charset());
return str;
@ -1678,7 +1680,7 @@ String *Item_func_old_password::val_str(String *str)
if ((null_value=args[0]->null_value))
return 0;
if (res->length() == 0)
return &my_empty_string;
return make_empty_result();
my_make_scrambled_password_323(tmp_value, res->ptr(), res->length());
str->set(tmp_value, SCRAMBLED_PASSWORD_CHAR_LENGTH_323, res->charset());
return str;
@ -1706,8 +1708,7 @@ String *Item_func_encrypt::val_str(String *str)
if ((null_value=args[0]->null_value))
return 0;
if (res->length() == 0)
return &my_empty_string;
return make_empty_result();
if (arg_count == 1)
{ // generate random salt
time_t timestamp=current_thd->query_start();
@ -1967,7 +1968,7 @@ String *Item_func_soundex::val_str(String *str)
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 make_empty_result(); /* EOL or invalid byte sequence */
if (rc == 1 && cs->ctype)
{
@ -1992,7 +1993,7 @@ String *Item_func_soundex::val_str(String *str)
{
/* Extra safety - should not really happen */
DBUG_ASSERT(false);
return &my_empty_string;
return make_empty_result();
}
to+= rc;
break;
@ -2289,7 +2290,7 @@ String *Item_func_make_set::val_str(String *str)
else
{
if (tmp_str.copy(*res)) // Don't use 'str'
return &my_empty_string;
return make_empty_result();
result= &tmp_str;
}
}
@ -2299,11 +2300,11 @@ String *Item_func_make_set::val_str(String *str)
{ // Copy data to tmp_str
if (tmp_str.alloc(result->length()+res->length()+1) ||
tmp_str.copy(*result))
return &my_empty_string;
return make_empty_result();
result= &tmp_str;
}
if (tmp_str.append(STRING_WITH_LEN(","), &my_charset_bin) || tmp_str.append(*res))
return &my_empty_string;
return make_empty_result();
}
}
}
@ -2442,7 +2443,7 @@ String *Item_func_repeat::val_str(String *str)
null_value= 0;
if (count <= 0 && (count == 0 || !args[1]->unsigned_flag))
return &my_empty_string;
return make_empty_result();
/* Assumes that the maximum length of a String is < INT_MAX32. */
/* Bounds check on count: If this is triggered, we will error. */
@ -2750,7 +2751,7 @@ String *Item_func_conv::val_str(String *str)
ptr= longlong2str(dec, ans, to_base);
if (str->copy(ans, (uint32) (ptr-ans), default_charset()))
return &my_empty_string;
return make_empty_result();
return str;
}
@ -2917,7 +2918,7 @@ String *Item_func_hex::val_str(String *str)
return 0;
ptr= longlong2str(dec,ans,16);
if (str->copy(ans,(uint32) (ptr-ans),default_charset()))
return &my_empty_string; // End of memory
return make_empty_result(); // End of memory
return str;
}

10
sql/item_strfunc.h
View File

@ -22,6 +22,16 @@
class Item_str_func :public Item_func
{
protected:
/**
Sets the result value of the function an empty string, using the current
character set. No memory is allocated.
@retval A pointer to the str_value member.
*/
String *make_empty_result() {
str_value.set("", 0, collation.collation);
return &str_value;
}
public:
Item_str_func() :Item_func() { decimals=NOT_FIXED_DEC; }
Item_str_func(Item *a) :Item_func(a) {decimals=NOT_FIXED_DEC; }

41
sql/sql_string.cc
View File

@ -58,11 +58,33 @@ bool String::real_alloc(uint32 arg_length)
}
/*
** 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,
one of the following is performed.
- If the requested length is greater than what fits in the buffer, a new
buffer is allocated, data moved and the old buffer freed.
- If the requested length is less or equal to what fits in the buffer, a
null character is inserted at the appropriate position.
- 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 requested string size in characters, excluding any
null terminator.
@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);
@ -196,6 +218,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 is to be copied.
@retval false Success.
@retval true Memory allocation failed.
*/
bool String::copy(const String &str)
{
if (alloc(str.str_length))

10
sql/sql_string.h
View File

@ -136,6 +136,16 @@ public:
Alloced_length=0;
str_charset=str.str_charset;
}
/**
Points the internal buffer to the supplied one. The old buffer is freed.
@param str Pointer to the new buffer.
@param arg_length Length of the new buffer in characters, excluding any
null character.
@param cs Character set to use for interpreting string data.
@note The new buffer will not be null terminated.
*/
inline void set(char *str,uint32 arg_length, CHARSET_INFO *cs)
{
free();