From f2a42aee2e42edb95fddbdcba7b11f6967221de1 Mon Sep 17 00:00:00 2001 From: Magne Mahre Date: Thu, 17 Feb 2011 12:43:53 +0100 Subject: [PATCH] Bug#48053 String::c_ptr has a race and/or does an invalid memory reference There are two issues present here. 1) There is a possibility that we test a byte beyond the allocated buffer 2) We compare a byte that might never have been initalized to see if it's 0. The first issue is not triggered by existing code, but an ASSERT has been added to safe-guard against introducing new code that triggers it. The second issue is what triggers the Valgrind warnings reported in the bug report. A buffer is allocated in class String to hold the value. This buffer is populated by the character data constituting the string, but is not zero-terminated in most cases. Testing if it is indeed zero-terminated means that we check a byte that has never been explicitly set, thus causing Valgrind to trigger. Note that issue 2 is not a serious problem. The variable is read, and if it's not zero, we will set it to zero. There are no further consequences. Note that this patch does not fix the underlying problems with issue 1, as it is deemed too risky to fix at this point (as noted in the bug report). As discussed in the report, the c_ptr() method should probably be replaced, but this requires a thorough analysis of the ~200 calls to the method. --- mysql-test/r/ctype_cp1250_ch.result | 3 +++ mysql-test/r/ctype_cp1251.result | 2 ++ mysql-test/r/ctype_eucjpms.result | 2 ++ mysql-test/t/ctype_cp1250_ch.test | 10 ++++++++++ mysql-test/t/ctype_cp1251.test | 10 ++++++++++ mysql-test/t/ctype_eucjpms.test | 8 ++++++++ sql/set_var.cc | 4 ++-- sql/sql_string.h | 3 +++ 8 files changed, 40 insertions(+), 2 deletions(-) mode change 100755 => 100644 mysql-test/r/ctype_eucjpms.result diff --git a/mysql-test/r/ctype_cp1250_ch.result b/mysql-test/r/ctype_cp1250_ch.result index 7f0cdf3f17b..46ca1f25ef4 100644 --- a/mysql-test/r/ctype_cp1250_ch.result +++ b/mysql-test/r/ctype_cp1250_ch.result @@ -238,3 +238,6 @@ select a from t1 where a like "abcdefgh a abcdefghá drop table t1; +set global LC_MESSAGES=convert((@@global.log_bin_trust_function_creators) +using cp1250); +ERROR HY000: Unknown system variable 'LC_MESSAGES' diff --git a/mysql-test/r/ctype_cp1251.result b/mysql-test/r/ctype_cp1251.result index dc12f9ceb03..2e91ecb7bc0 100644 --- a/mysql-test/r/ctype_cp1251.result +++ b/mysql-test/r/ctype_cp1251.result @@ -375,6 +375,8 @@ FD FD FD D18D FD FE FE FE D18E FE FF FF FF D18F FF DROP TABLE t1; +set global LC_TIME_NAMES=convert((-8388608) using cp1251); +ERROR HY000: Unknown locale: '-8388608' # # End of 5.1 tests # diff --git a/mysql-test/r/ctype_eucjpms.result b/mysql-test/r/ctype_eucjpms.result old mode 100755 new mode 100644 index 21aa38b7fe6..21109f596c1 --- a/mysql-test/r/ctype_eucjpms.result +++ b/mysql-test/r/ctype_eucjpms.result @@ -9859,3 +9859,5 @@ hex(convert(_eucjpms 0xA5FE41 using ucs2)) select hex(convert(_eucjpms 0x8FABF841 using ucs2)); hex(convert(_eucjpms 0x8FABF841 using ucs2)) 003F0041 +set global LC_TIME_NAMES=convert((convert((0x63) using eucjpms)) using utf8); +ERROR HY000: Unknown locale: 'c' diff --git a/mysql-test/t/ctype_cp1250_ch.test b/mysql-test/t/ctype_cp1250_ch.test index 1fb656f2a01..3e17ee52164 100644 --- a/mysql-test/t/ctype_cp1250_ch.test +++ b/mysql-test/t/ctype_cp1250_ch.test @@ -72,3 +72,13 @@ select a from t1 where a like "abcdefgh drop table t1; # End of 4.1 tests + +# +# Bug #48053 String::c_ptr has a race and/or does an invalid +# memory reference +# (triggered by Valgrind tests) +# (see also ctype_eucjpms.test, ctype_cp1250.test, ctype_cp1251.test) +# +--error 1193 +set global LC_MESSAGES=convert((@@global.log_bin_trust_function_creators) + using cp1250); diff --git a/mysql-test/t/ctype_cp1251.test b/mysql-test/t/ctype_cp1251.test index 2331c731061..bde72d04ba7 100644 --- a/mysql-test/t/ctype_cp1251.test +++ b/mysql-test/t/ctype_cp1251.test @@ -55,6 +55,16 @@ drop table t1; --source include/ctype_8bit.inc +# +# Bug #48053 String::c_ptr has a race and/or does an invalid +# memory reference +# (triggered by Valgrind tests) +# (see also ctype_eucjpms.test, ctype_cp1250.test, ctype_cp1251.test) +# +--error 1105 +set global LC_TIME_NAMES=convert((-8388608) using cp1251); + + --echo # --echo # End of 5.1 tests --echo # diff --git a/mysql-test/t/ctype_eucjpms.test b/mysql-test/t/ctype_eucjpms.test index ec358d94900..165cfba897a 100644 --- a/mysql-test/t/ctype_eucjpms.test +++ b/mysql-test/t/ctype_eucjpms.test @@ -381,3 +381,11 @@ select hex(convert(_eucjpms 0xA5FE41 using ucs2)); # the next character, which is a single byte character 0x41. select hex(convert(_eucjpms 0x8FABF841 using ucs2)); +# +# Bug #48053 String::c_ptr has a race and/or does an invalid +# memory reference +# (triggered by Valgrind tests) +# (see also ctype_eucjpms.test, ctype_cp1250.test, ctype_cp1251.test) +# +--error 1105 +set global LC_TIME_NAMES=convert((convert((0x63) using eucjpms)) using utf8); diff --git a/sql/set_var.cc b/sql/set_var.cc index d297be3fc10..26c9b06a912 100644 --- a/sql/set_var.cc +++ b/sql/set_var.cc @@ -1828,7 +1828,7 @@ bool sys_var::check_set(THD *thd, set_var *var, TYPELIB *enum_names) } var->save_result.ulong_value= ((ulong) - find_set(enum_names, res->c_ptr(), + find_set(enum_names, res->c_ptr_safe(), res->length(), NULL, &error, &error_len, @@ -2941,7 +2941,7 @@ bool sys_var_thd_lc_time_names::check(THD *thd, set_var *var) my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), name, "NULL"); return 1; } - const char *locale_str= res->c_ptr(); + const char *locale_str= res->c_ptr_safe(); if (!(locale_match= my_locale_by_name(locale_str))) { my_printf_error(ER_UNKNOWN_ERROR, diff --git a/sql/sql_string.h b/sql/sql_string.h index 092e194646f..c56c69493d4 100644 --- a/sql/sql_string.h +++ b/sql/sql_string.h @@ -106,6 +106,9 @@ public: inline const char *ptr() const { return Ptr; } inline char *c_ptr() { + DBUG_ASSERT(!alloced || !Ptr || !Alloced_length || + (Alloced_length >= (str_length + 1))); + if (!Ptr || Ptr[str_length]) /* Should be safe */ (void) realloc(str_length); return Ptr;