MDEV-4244 [PATCH] Buffer overruns and use-after-free errors

fixes for gcc 4.8 - compilation warnings and -fsanitize=address

client/mysqlbinlog.cc

@@ -662,6 +662,81 @@ write_event_header_and_base64(Log_event *ev, FILE *result_file,
 }
 
 
+static bool print_base64(PRINT_EVENT_INFO *print_event_info, Log_event *ev)
+{
+  /*
+    These events must be printed in base64 format, if printed.
+    base64 format requires a FD event to be safe, so if no FD
+    event has been printed, we give an error.  Except if user
+    passed --short-form, because --short-form disables printing
+    row events.
+  */
+  if (!print_event_info->printed_fd_event && !short_form &&
+      opt_base64_output_mode != BASE64_OUTPUT_DECODE_ROWS)
+  {
+    const char* type_str= ev->get_type_str();
+    if (opt_base64_output_mode == BASE64_OUTPUT_NEVER)
+      error("--base64-output=never specified, but binlog contains a "
+            "%s event which must be printed in base64.",
+            type_str);
+    else
+      error("malformed binlog: it does not contain any "
+            "Format_description_log_event. I now found a %s event, which "
+            "is not safe to process without a "
+            "Format_description_log_event.",
+            type_str);
+    return 1;
+  }
+  ev->print(result_file, print_event_info);
+  return print_event_info->head_cache.error == -1;
+}
+
+
+static bool print_row_event(PRINT_EVENT_INFO *print_event_info, Log_event *ev,
+                            ulong table_id, bool is_stmt_end)
+{
+  Table_map_log_event *ignored_map= 
+    print_event_info->m_table_map_ignored.get_table(table_id);
+  bool skip_event= (ignored_map != NULL);
+
+  /* 
+     end of statement check:
+       i) destroy/free ignored maps
+      ii) if skip event, flush cache now
+   */
+  if (is_stmt_end)
+  {
+    /* 
+      Now is safe to clear ignored map (clear_tables will also
+      delete original table map events stored in the map).
+    */
+    if (print_event_info->m_table_map_ignored.count() > 0)
+      print_event_info->m_table_map_ignored.clear_tables();
+
+    /* 
+       One needs to take into account an event that gets
+       filtered but was last event in the statement. If this is
+       the case, previous rows events that were written into
+       IO_CACHEs still need to be copied from cache to
+       result_file (as it would happen in ev->print(...) if
+       event was not skipped).
+    */
+    if (skip_event)
+    {
+      if ((copy_event_cache_to_file_and_reinit(&print_event_info->head_cache, result_file) ||
+          copy_event_cache_to_file_and_reinit(&print_event_info->body_cache, result_file)))
+        return 1;
+    }
+  }
+
+  /* skip the event check */
+  if (skip_event)
+    return 0;
+
+  return print_base64(print_event_info, ev);
+}
+
+
 /**
   Print the given event, and either delete it or delegate the deletion
   to someone else.
@@ -921,79 +996,29 @@ Exit_status process_event(PRINT_EVENT_INFO *print_event_info, Log_event *ev,
         destroy_evt= FALSE;
         goto end;
       }
+      if (print_base64(print_event_info, ev))
+        goto err;
+      break;
     }
     case WRITE_ROWS_EVENT:
     case DELETE_ROWS_EVENT:
     case UPDATE_ROWS_EVENT:
+    {
+      Rows_log_event *e= (Rows_log_event*) ev;
+      if (print_row_event(print_event_info, ev, e->get_table_id(),
+                          e->get_flags(Rows_log_event::STMT_END_F)))
+        goto err;
+      break;
+    }
     case PRE_GA_WRITE_ROWS_EVENT:
     case PRE_GA_DELETE_ROWS_EVENT:
     case PRE_GA_UPDATE_ROWS_EVENT:
     {
-      if (ev_type != TABLE_MAP_EVENT)
-      {
-        Rows_log_event *e= (Rows_log_event*) ev;
-        Table_map_log_event *ignored_map= 
-          print_event_info->m_table_map_ignored.get_table(e->get_table_id());
-        bool skip_event= (ignored_map != NULL);
-
-        /* 
-           end of statement check:
-             i) destroy/free ignored maps
-            ii) if skip event, flush cache now
-         */
-        if (e->get_flags(Rows_log_event::STMT_END_F))
-        {
-          /* 
-            Now is safe to clear ignored map (clear_tables will also
-            delete original table map events stored in the map).
-          */
-          if (print_event_info->m_table_map_ignored.count() > 0)
-            print_event_info->m_table_map_ignored.clear_tables();
-
-          /* 
-             One needs to take into account an event that gets
-             filtered but was last event in the statement. If this is
-             the case, previous rows events that were written into
-             IO_CACHEs still need to be copied from cache to
-             result_file (as it would happen in ev->print(...) if
-             event was not skipped).
-          */
-          if (skip_event)
-          {
-            if ((copy_event_cache_to_file_and_reinit(&print_event_info->head_cache, result_file) ||
-                copy_event_cache_to_file_and_reinit(&print_event_info->body_cache, result_file)))
-              goto err;
-          }
-        }
-
-        /* skip the event check */
-        if (skip_event)
-          goto end;
-      }
-      /*
-        These events must be printed in base64 format, if printed.
-        base64 format requires a FD event to be safe, so if no FD
-        event has been printed, we give an error.  Except if user
-        passed --short-form, because --short-form disables printing
-        row events.
-      */
-      if (!print_event_info->printed_fd_event && !short_form &&
-          opt_base64_output_mode != BASE64_OUTPUT_DECODE_ROWS)
-      {
-        const char* type_str= ev->get_type_str();
-        if (opt_base64_output_mode == BASE64_OUTPUT_NEVER)
-          error("--base64-output=never specified, but binlog contains a "
-                "%s event which must be printed in base64.",
-                type_str);
-        else
-          error("malformed binlog: it does not contain any "
-                "Format_description_log_event. I now found a %s event, which "
-                "is not safe to process without a "
-                "Format_description_log_event.",
-                type_str);
+      Old_rows_log_event *e= (Old_rows_log_event*) ev;
+      if (print_row_event(print_event_info, ev, e->get_table_id(),
+                          e->get_flags(Old_rows_log_event::STMT_END_F)))
         goto err;
-      }
-      /* FALL THROUGH */
+      break;
     }
     default:
       ev->print(result_file, print_event_info);

include/my_global.h

@@ -496,7 +496,8 @@ C_MODE_END
 #define compile_time_assert(X)                                  \
   do                                                            \
   {                                                             \
-    typedef char compile_time_assert[(X) ? 1 : -1];             \
+    typedef char compile_time_assert[(X) ? 1 : -1]              \
+                                       __attribute__((unused)); \
   } while(0)
 #endif
 

sql/opt_range.cc

@@ -83,7 +83,12 @@
 
 static int sel_cmp(Field *f,uchar *a,uchar *b,uint8 a_flag,uint8 b_flag);
 
-static uchar is_null_string[2]= {1,0};
+/*
+  this should be long enough so that any memcmp with a string that
+  starts from '\0' won't cross is_null_string boundaries, even
+  if the memcmp is optimized to compare 4- 8- or 16- bytes at once
+*/
+static uchar is_null_string[20]= {1,0};
 
 class RANGE_OPT_PARAM;
 /*

strings/ctype-utf8.c

@@ -4261,6 +4261,10 @@ static char filename_safe_char[128]=
 
 #define MY_FILENAME_ESCAPE '@'
 
+/*
+  note, that we cannot trust 'e' here, it's may be fake,
+  see strconvert()
+*/
 static int
 my_mb_wc_filename(CHARSET_INFO *cs __attribute__((unused)),
                   my_wc_t *pwc, const uchar *s, const uchar *e)
@@ -4282,7 +4286,7 @@ my_mb_wc_filename(CHARSET_INFO *cs __attribute__((unused)),
     return MY_CS_TOOSMALL3;
   
   byte1= s[1];
-  byte2= s[2];
+  byte2= byte1 ? s[2] : 0;
   
   if (byte1 >= 0x30 && byte1 <= 0x7F &&
       byte2 >= 0x30 && byte2 <= 0x7F)
@@ -4307,7 +4311,7 @@ my_mb_wc_filename(CHARSET_INFO *cs __attribute__((unused)),
       (byte2= hexlo(byte2)) >= 0)
   {
     int byte3= hexlo(s[3]);
-    int byte4= hexlo(s[4]);
+    int byte4= hexlo(s[3] ? s[4] : 0);
     if (byte3 >=0 && byte4 >=0)
     {
       *pwc= (byte1 << 12) + (byte2 << 8) + (byte3 << 4) + byte4;

tests/mysql_client_test.c

@@ -5599,7 +5599,7 @@ static void test_date_dt()
 static void test_pure_coverage()
 {
   MYSQL_STMT *stmt;
-  MYSQL_BIND my_bind[1];
+  MYSQL_BIND my_bind[2];
   int        rc;
   ulong      length;
 
@@ -8275,7 +8275,7 @@ static void test_parse_error_and_bad_length()
   DIE_UNLESS(rc);
   if (!opt_silent)
     fprintf(stdout, "Got error (as expected): '%s'\n", mysql_error(mysql));
-  rc= mysql_real_query(mysql, "SHOW DATABASES", 100);
+  rc= mysql_real_query(mysql, STRING_WITH_LEN("SHOW DATABASES\0AAAAAAAA"));
   DIE_UNLESS(rc);
   if (!opt_silent)
     fprintf(stdout, "Got error (as expected): '%s'\n", mysql_error(mysql));
@@ -8286,7 +8286,7 @@ static void test_parse_error_and_bad_length()
     fprintf(stdout, "Got error (as expected): '%s'\n", mysql_error(mysql));
   stmt= mysql_stmt_init(mysql);
   DIE_UNLESS(stmt);
-  rc= mysql_stmt_prepare(stmt, "SHOW DATABASES", 100);
+  rc= mysql_stmt_prepare(stmt, STRING_WITH_LEN("SHOW DATABASES\0AAAAAAA"));
   DIE_UNLESS(rc != 0);
   if (!opt_silent)
     fprintf(stdout, "Got error (as expected): '%s'\n", mysql_stmt_error(stmt));
@@ -16244,7 +16244,8 @@ static void test_bug31669()
   rc= mysql_change_user(mysql, "", "", "");
   DIE_UNLESS(rc);
 
-  memset(buff, 'a', sizeof(buff));
+  memset(buff, 'a', sizeof(buff) -  1);
+  buff[sizeof(buff) -  1]= 0;
 
   rc= mysql_change_user(mysql, buff, buff, buff);
   DIE_UNLESS(rc);