MDEV-16932: ASAN heap-use-after-free in my_charlen_utf8 / my_well_formed_char_length_utf8 on 2nd execution of SP with ALTER trying to add bad CHECK

Make automatic name generation during execution (not prepare).

Check result of memory allocation operation.

mysql-test/r/constraints.result

@@ -130,3 +130,58 @@ t	CREATE TABLE `t` (
 ) ENGINE=MyISAM DEFAULT CHARSET=latin1
 DROP table test.t;
 SET @@SQL_MODE=@OLD_SQL_MODE;
+#
+# MDEV-16932 - ASAN heap-use-after-free in my_charlen_utf8 /
+# my_well_formed_char_length_utf8 on 2nd execution of SP with
+# ALTER trying to add bad CHECK
+#
+CREATE TABLE t1 (a INT);
+CREATE PROCEDURE sp() ALTER TABLE t1 ADD CONSTRAINT CHECK (b > 0);
+CALL sp;
+ERROR 42S22: Unknown column 'b' in 'CHECK'
+CALL sp;
+ERROR 42S22: Unknown column 'b' in 'CHECK'
+CALL sp;
+ERROR 42S22: Unknown column 'b' in 'CHECK'
+show create table t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `a` int(11) DEFAULT NULL
+) ENGINE=MyISAM DEFAULT CHARSET=latin1
+alter table t1 add column b int;
+CALL sp;
+show create table t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `a` int(11) DEFAULT NULL,
+  `b` int(11) DEFAULT NULL,
+  CONSTRAINT `CONSTRAINT_1` CHECK (`b` > 0)
+) ENGINE=MyISAM DEFAULT CHARSET=latin1
+CALL sp;
+show create table t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `a` int(11) DEFAULT NULL,
+  `b` int(11) DEFAULT NULL,
+  CONSTRAINT `CONSTRAINT_1` CHECK (`b` > 0),
+  CONSTRAINT `CONSTRAINT_2` CHECK (`b` > 0)
+) ENGINE=MyISAM DEFAULT CHARSET=latin1
+DROP PROCEDURE sp;
+DROP TABLE t1;
+CREATE TABLE t1 (a INT);
+CREATE PROCEDURE sp() ALTER TABLE t1 ADD CONSTRAINT CHECK (b > 0);
+CALL sp;
+ERROR 42S22: Unknown column 'b' in 'CHECK'
+alter table t1 add column b int, add constraint check (b < 10);
+CALL sp;
+show create table t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `a` int(11) DEFAULT NULL,
+  `b` int(11) DEFAULT NULL,
+  CONSTRAINT `CONSTRAINT_1` CHECK (`b` < 10),
+  CONSTRAINT `CONSTRAINT_2` CHECK (`b` > 0)
+) ENGINE=MyISAM DEFAULT CHARSET=latin1
+DROP PROCEDURE sp;
+DROP TABLE t1;
+# End of 10.2 tests

mysql-test/t/constraints.test

@@ -119,3 +119,40 @@ CREATE TABLE test.t (f int foo=bar check(f>0));
 SHOW CREATE TABLE t;
 DROP table test.t;
 SET @@SQL_MODE=@OLD_SQL_MODE;
+
+--echo #
+--echo # MDEV-16932 - ASAN heap-use-after-free in my_charlen_utf8 /
+--echo # my_well_formed_char_length_utf8 on 2nd execution of SP with
+--echo # ALTER trying to add bad CHECK
+--echo #
+
+CREATE TABLE t1 (a INT);
+CREATE PROCEDURE sp() ALTER TABLE t1 ADD CONSTRAINT CHECK (b > 0);
+--error ER_BAD_FIELD_ERROR
+CALL sp;
+--error ER_BAD_FIELD_ERROR
+CALL sp;
+--error ER_BAD_FIELD_ERROR
+CALL sp;
+show create table t1;
+alter table t1 add column b int;
+CALL sp;
+show create table t1;
+CALL sp;
+show create table t1;
+# Cleanup
+DROP PROCEDURE sp;
+DROP TABLE t1;
+
+CREATE TABLE t1 (a INT);
+CREATE PROCEDURE sp() ALTER TABLE t1 ADD CONSTRAINT CHECK (b > 0);
+--error ER_BAD_FIELD_ERROR
+CALL sp;
+alter table t1 add column b int, add constraint check (b < 10);
+CALL sp;
+show create table t1;
+# Cleanup
+DROP PROCEDURE sp;
+DROP TABLE t1;
+
+--echo # End of 10.2 tests

sql/field.h

@@ -628,6 +628,7 @@ public:
   /* Flag indicating  that the field is physically stored in the database */
   bool stored_in_db;
   bool utf8;                                    /* Already in utf8 */
+  bool automatic_name;
   Item *expr;
   LEX_STRING name;                              /* Name of constraint */
   /* see VCOL_* (VCOL_FIELD_REF, ...) */
@@ -637,7 +638,7 @@ public:
   : vcol_type((enum_vcol_info_type)VCOL_TYPE_NONE),
     field_type((enum enum_field_types)MYSQL_TYPE_VIRTUAL),
     in_partitioning_expr(FALSE), stored_in_db(FALSE),
-    utf8(TRUE), expr(NULL), flags(0)
+    utf8(TRUE), automatic_name(FALSE), expr(NULL), flags(0)
   {
     name.str= NULL;
     name.length= 0;

sql/sql_table.cc

@@ -64,7 +64,7 @@ const char *primary_key_name="PRIMARY";
 static bool check_if_keyname_exists(const char *name,KEY *start, KEY *end);
 static char *make_unique_key_name(THD *thd, const char *field_name, KEY *start,
                                   KEY *end);
-static void make_unique_constraint_name(THD *thd, LEX_STRING *name,
+static bool make_unique_constraint_name(THD *thd, LEX_STRING *name,
                                         List<Virtual_column_info> *vcol,
                                         uint *nr);
 static int copy_data_between_tables(THD *thd, TABLE *from,TABLE *to,
@@ -78,6 +78,8 @@ static bool prepare_blob_field(THD *thd, Column_definition *sql_field);
 static int mysql_prepare_create_table(THD *, HA_CREATE_INFO *, Alter_info *,
                                       uint *, handler *, KEY **, uint *, int);
 static uint blob_length_by_type(enum_field_types type);
+static bool fix_constraints_names(THD *thd, List<Virtual_column_info>
+                                  *check_constraint_list);
 
 /**
   @brief Helper function for explain_filename
@@ -4178,15 +4180,13 @@ mysql_prepare_create_table(THD *thd, HA_CREATE_INFO *create_info,
   /* Check table level constraints */
   create_info->check_constraint_list= &alter_info->check_constraint_list;
   {
-    uint nr= 1;
     List_iterator_fast<Virtual_column_info> c_it(alter_info->check_constraint_list);
     Virtual_column_info *check;
     while ((check= c_it++))
     {
-      if (!check->name.length)
+      if (!check->name.length || check->automatic_name)
-        make_unique_constraint_name(thd, &check->name,
+        continue;
-                                    &alter_info->check_constraint_list,
+
-                                    &nr);
       {
         /* Check that there's no repeating constraint names. */
         List_iterator_fast<Virtual_column_info>
@@ -4722,6 +4722,9 @@ int create_table_impl(THD *thd,
   DBUG_PRINT("enter", ("db: '%s'  table: '%s'  tmp: %d  path: %s",
                        db, table_name, internal_tmp_table, path));
 
+  if (fix_constraints_names(thd, &alter_info->check_constraint_list))
+    DBUG_RETURN(1);
+
   if (thd->variables.sql_mode & MODE_NO_DIR_IN_CREATE)
   {
     if (create_info->data_file_name)
@@ -5185,7 +5188,7 @@ make_unique_key_name(THD *thd, const char *field_name,KEY *start,KEY *end)
    Make an unique name for constraints without a name
 */
 
-static void make_unique_constraint_name(THD *thd, LEX_STRING *name,
+static bool make_unique_constraint_name(THD *thd, LEX_STRING *name,
                                         List<Virtual_column_info> *vcol,
                                         uint *nr)
 {
@@ -5208,9 +5211,10 @@ static void make_unique_constraint_name(THD *thd, LEX_STRING *name,
     {
       name->length= (size_t) (real_end - buff);
       name->str= thd->strmake(buff, name->length);
-      return;
+      return (name->str == NULL);
     }
   }
+  return FALSE;
 }
 
 
@@ -5793,10 +5797,11 @@ static bool is_candidate_key(KEY *key)
      from the list if existing found.
 
    RETURN VALUES
-     NONE
+     TRUE error
+     FALSE OK
 */
 
-static void
+static bool
 handle_if_exists_options(THD *thd, TABLE *table, Alter_info *alter_info)
 {
   Field **f_ptr;
@@ -6199,6 +6204,7 @@ remove_key:
     Virtual_column_info *check;
     TABLE_SHARE *share= table->s;
     uint c;
+
     while ((check=it++))
     {
       if (!(check->flags & Alter_info::CHECK_CONSTRAINT_IF_NOT_EXISTS) &&
@@ -6226,10 +6232,44 @@ remove_key:
     }
   }
 
-  DBUG_VOID_RETURN;
+  DBUG_RETURN(FALSE);
 }
 
 
+static bool fix_constraints_names(THD *thd, List<Virtual_column_info>
+                                  *check_constraint_list)
+{
+  List_iterator<Virtual_column_info> it((*check_constraint_list));
+  Virtual_column_info *check;
+  uint nr= 1;
+  DBUG_ENTER("fix_constraints_names");
+  if (!check_constraint_list)
+    DBUG_RETURN(FALSE);
+  // Prevent accessing freed memory during generating unique names
+  while ((check=it++))
+  {
+    if (check->automatic_name)
+    {
+      check->name.str= NULL;
+      check->name.length= 0;
+    }
+  }
+  it.rewind();
+  // Generate unique names if needed
+  while ((check=it++))
+  {
+    if (!check->name.length)
+    {
+      check->automatic_name= TRUE;
+      if (make_unique_constraint_name(thd, &check->name,
+                                      check_constraint_list,
+                                      &nr))
+        DBUG_RETURN(TRUE);
+    }
+  }
+  DBUG_RETURN(FALSE);
+}
+
 /**
   Get Create_field object for newly created table by field index.
 
@@ -9076,7 +9116,10 @@ do_continue:;
     }
   }
 
-  handle_if_exists_options(thd, table, alter_info);
+  if (handle_if_exists_options(thd, table, alter_info) ||
+      fix_constraints_names(thd, &alter_info->check_constraint_list))
+    DBUG_RETURN(true);
+
 
   /*
     Look if we have to do anything at all.