From 17c75bd272f620a11dbb9fe0517b41da03d081f3 Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Thu, 17 Jan 2019 16:37:57 +0400 Subject: [PATCH] MDEV-18195 ASAN use-after-poison in my_strcasecmp_utf8 / Item::eq upon prepared statement with ORDER BY NAME_CONST ASAN noticed a freed memory access during EXECUTE in this script: PREPARE stmt FROM "SELECT 'x' ORDER BY NAME_CONST( 'f', 'foo' )"; EXECUTE stmt; In case of a PREPARE statement, all Items, including Item_name_const, are created on Prepared_statement::main_mem_root. Item_name_const::fix_fields() did not take this into account and could allocate the value of Item::name on a wrong memory root, in this code: if (is_autogenerated_name) { set_name(thd, item_name->c_ptr(), (uint) item_name->length(), system_charset_info); } When fix_fields() is called in the reported SQL script, THD's arena already points to THD::main_mem_root rather than to Prepared_statement::main_mem_root, so Item::name was allocated on THD::main_mem_root. Then, at the end of the dispatch_command() for the PREPARE statement, THD::main_mem_root got cleared. So during EXECUTE, Item::name pointed to an already freed memory. This patch changes the code to set the implicit name for Item_name_const at the constructor time rather than at fix_fields time. This guarantees that Item_name_const and its Item::name always reside on the same memory root. Note, this change makes the code for Item_name_const symmetric with other constant-alike items that set their default implicit names at the constructor call time rather than at fix_fields() time: - Item_string - Item_int - Item_real - Item_decimal - Item_null - Item_param --- mysql-test/r/func_misc.result | 8 ++++++++ mysql-test/t/func_misc.test | 9 +++++++++ sql/item.cc | 18 ++++++------------ 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/mysql-test/r/func_misc.result b/mysql-test/r/func_misc.result index 1f9aa212740..287a70f1f73 100644 --- a/mysql-test/r/func_misc.result +++ b/mysql-test/r/func_misc.result @@ -1471,3 +1471,11 @@ CALL p1(); ip_full_addr 2000:: DROP PROCEDURE p1; +# +# MDEV-18195 ASAN use-after-poison in my_strcasecmp_utf8 / Item::eq upon prepared statement with ORDER BY NAME_CONST +# +PREPARE stmt FROM "SELECT 'x' ORDER BY NAME_CONST( 'f', 'foo' )"; +EXECUTE stmt; +x +x +DEALLOCATE PREPARE stmt; diff --git a/mysql-test/t/func_misc.test b/mysql-test/t/func_misc.test index 2509f55ccbb..a8da9068ab8 100644 --- a/mysql-test/t/func_misc.test +++ b/mysql-test/t/func_misc.test @@ -1145,3 +1145,12 @@ $$ DELIMITER ;$$ CALL p1(); DROP PROCEDURE p1; + + +--echo # +--echo # MDEV-18195 ASAN use-after-poison in my_strcasecmp_utf8 / Item::eq upon prepared statement with ORDER BY NAME_CONST +--echo # + +PREPARE stmt FROM "SELECT 'x' ORDER BY NAME_CONST( 'f', 'foo' )"; +EXECUTE stmt; +DEALLOCATE PREPARE stmt; diff --git a/sql/item.cc b/sql/item.cc index 1a2c4eba5fb..d8f27bdb30e 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -1672,10 +1672,14 @@ bool Item_name_const::is_null() Item_name_const::Item_name_const(THD *thd, Item *name_arg, Item *val): Item(thd), value_item(val), name_item(name_arg) { + StringBuffer<128> name_buffer; + String *name_str; Item::maybe_null= TRUE; valid_args= true; - if (!name_item->basic_const_item()) + if (!name_item->basic_const_item() || + !(name_str= name_item->val_str(&name_buffer))) // Can't have a NULL name goto err; + set_name(name_str->ptr(), name_str->length(), name_str->charset()); if (value_item->basic_const_item()) return; // ok @@ -1737,24 +1741,14 @@ Item::Type Item_name_const::type() const bool Item_name_const::fix_fields(THD *thd, Item **ref) { - char buf[128]; - String *item_name; - String s(buf, sizeof(buf), &my_charset_bin); - s.length(0); - if (value_item->fix_fields(thd, &value_item) || name_item->fix_fields(thd, &name_item) || !value_item->const_item() || - !name_item->const_item() || - !(item_name= name_item->val_str(&s))) // Can't have a NULL name + !name_item->const_item()) { my_error(ER_RESERVED_SYNTAX, MYF(0), "NAME_CONST"); return TRUE; } - if (is_autogenerated_name) - { - set_name(item_name->ptr(), (uint) item_name->length(), system_charset_info); - } if (value_item->collation.derivation == DERIVATION_NUMERIC) collation.set_numeric(); else