MDEV-40644: Slave SQL Thread Overflow on Malformed Table_map_log_event

mysql-test/suite/rpl/r/rpl_table_map_name_overflow.result

@@ -0,0 +1,68 @@
+include/master-slave.inc
+[connection master]
+connection slave;
+call mtr.add_suppression("Error in Log_event::read_log_event.*Found invalid event");
+call mtr.add_suppression("Relay log read failure");
+#
+# Initialize test data
+#
+connection master;
+create table t1 (a int);
+insert into t1 values (1);
+include/save_master_gtid.inc
+connection slave;
+include/sync_with_master_gtid.inc
+include/stop_slave.inc
+CHANGE MASTER TO MASTER_USE_GTID=slave_pos;
+include/start_slave.inc
+connection master;
+set @saved_dbug= @@global.debug_dbug;
+#
+# Test Case: a Table_map event naming a table with an oversized name
+#
+connection master;
+set @@global.debug_dbug= "+d,binlog_sender_oversized_table_name";
+connection slave;
+include/stop_slave.inc
+include/start_slave.inc
+connection master;
+insert into t1 values (2);
+# Waiting for the SQL thread to reject the oversized Table_map event
+connection slave;
+include/wait_for_slave_sql_error.inc [errno=1594]
+# Ensure the SQL thread rejected the oversized Table_map event
+include/assert_grep.inc [The SQL thread reported the oversized Table_map event as invalid]
+# Ensure the rejected event delivered nothing to the replica
+include/assert.inc [The replica has not applied the insert the event carried]
+connection master;
+set @@global.debug_dbug= @saved_dbug;
+#
+# Ensure the replica holds the row once it refetches the relay log
+#
+connection slave;
+include/stop_slave_io.inc
+include/start_slave.inc
+connection master;
+include/save_master_gtid.inc
+connection slave;
+include/sync_with_master_gtid.inc
+include/diff_tables.inc [master:t1,slave:t1]
+#
+# Ensure replication works after the rejected event
+#
+connection master;
+insert into t1 values (3);
+include/save_master_gtid.inc
+connection slave;
+include/sync_with_master_gtid.inc
+include/diff_tables.inc [master:t1,slave:t1]
+#
+# Cleanup
+#
+connection master;
+drop table t1;
+include/save_master_gtid.inc
+connection slave;
+include/sync_with_master_gtid.inc
+include/rpl_end.inc
+# End of rpl_table_map_name_overflow.test

mysql-test/suite/rpl/t/rpl_table_map_name_overflow-master.opt

@@ -0,0 +1 @@
+--binlog-checksum=NONE

mysql-test/suite/rpl/t/rpl_table_map_name_overflow.test

@@ -0,0 +1,127 @@
+#
+#   This test verifies that a replica rejects a Table_map event whose
+# database or table name is longer than an identifier can be, when the SQL
+# thread reads the event from the relay log and constructs it.
+#
+#   A replica copies each name into a NAME_LEN+1 byte buffer with strmov(),
+# which stops only at a terminating null, so without the bound a 255 byte
+# name overruns that buffer as the SQL thread applies the event. The event's
+# constructor rejects the oversized name before that copy, so the SQL thread
+# stops with ER_SLAVE_RELAY_LOG_READ_FAILURE instead.
+#
+#   Note rpl_table_map_name_overflow-master.opt sets binlog_checksum=NONE on
+# the master to simplify creating a corrupted event to send to the slave.
+#
+# References:
+#   * MDEV-40644: Slave SQL Thread Overflow on Malformed Table_map_log_event
+#
+
+--source include/have_debug.inc
+# A Table_map event is only produced in row format.
+--source include/have_binlog_format_row.inc
+--source include/master-slave.inc
+
+--connection slave
+call mtr.add_suppression("Error in Log_event::read_log_event.*Found invalid event");
+call mtr.add_suppression("Relay log read failure");
+
+--echo #
+--echo # Initialize test data
+--echo #
+--connection master
+create table t1 (a int);
+insert into t1 values (1);
+--source include/save_master_gtid.inc
+
+--connection slave
+--source include/sync_with_master_gtid.inc
+
+# Use slave_pos so we can easily reset the relay log after it receives a
+# corrupted event
+--source include/stop_slave.inc
+CHANGE MASTER TO MASTER_USE_GTID=slave_pos;
+--source include/start_slave.inc
+
+--connection master
+set @saved_dbug= @@global.debug_dbug;
+
+--echo #
+--echo # Test Case: a Table_map event naming a table with an oversized name
+--echo #
+--connection master
+set @@global.debug_dbug= "+d,binlog_sender_oversized_table_name";
+
+# The dump thread reads the global setting when the replica reconnects, so
+# restart the replica to open a fresh dump thread that carries the injection.
+--connection slave
+--source include/stop_slave.inc
+--source include/start_slave.inc
+
+--connection master
+insert into t1 values (2);
+
+--echo # Waiting for the SQL thread to reject the oversized Table_map event
+--connection slave
+--let $slave_sql_errno= 1594
+--source include/wait_for_slave_sql_error.inc
+
+# The injection grows the table name to 255 bytes and leaves the database
+# name "test" at its four bytes. The constructor rejects the oversized name,
+# so read_log_event() reports the event as invalid.
+--echo # Ensure the SQL thread rejected the oversized Table_map event
+--let $assert_text= The SQL thread reported the oversized Table_map event as invalid
+--let $assert_file= $MYSQLTEST_VARDIR/log/mysqld.2.err
+--let $assert_select= Found invalid event in binary log
+--let $assert_count= 1
+--source include/assert_grep.inc
+
+# The constructor rejects the event before do_apply_event() copies the name,
+# so the SQL thread stops without applying the row the event carried.
+--echo # Ensure the rejected event delivered nothing to the replica
+--let $slave_row_count= query_get_value(SELECT COUNT(*) AS c FROM t1, c, 1)
+--let $assert_text= The replica has not applied the insert the event carried
+--let $assert_cond= $slave_row_count = 1
+--source include/assert.inc
+
+--connection master
+set @@global.debug_dbug= @saved_dbug;
+
+--echo #
+--echo # Ensure the replica holds the row once it refetches the relay log
+--echo #
+--connection slave
+--source include/stop_slave_io.inc
+--source include/start_slave.inc
+
+--connection master
+--source include/save_master_gtid.inc
+
+--connection slave
+--source include/sync_with_master_gtid.inc
+--let $diff_tables=master:t1,slave:t1
+--source include/diff_tables.inc
+
+--echo #
+--echo # Ensure replication works after the rejected event
+--echo #
+--connection master
+insert into t1 values (3);
+--source include/save_master_gtid.inc
+
+--connection slave
+--source include/sync_with_master_gtid.inc
+--let $diff_tables=master:t1,slave:t1
+--source include/diff_tables.inc
+
+--echo #
+--echo # Cleanup
+--echo #
+--connection master
+drop table t1;
+--source include/save_master_gtid.inc
+
+--connection slave
+--source include/sync_with_master_gtid.inc
+
+--source include/rpl_end.inc
+--echo # End of rpl_table_map_name_overflow.test

sql/log_event.cc

@@ -3728,6 +3728,14 @@ Table_map_log_event::Table_map_log_event(const uchar *buf, uint event_len,
   VALIDATE_BYTES_READ(ptr_tbllen, buf, event_len);
   m_tbllen= *(uchar*) ptr_tbllen;
 
+  /*
+    If either the database or table name is too long, return before allocating
+    memory for the event, so future validity checks on this event (i.e.
+    is_valid()) return false.
+  */
+  if (unlikely(m_dblen > NAME_LEN || m_tbllen > NAME_LEN))
+    DBUG_VOID_RETURN;
+
   /* Length of table name + counter + terminating null */
   uchar const *const ptr_colcnt= ptr_tbllen + m_tbllen + 2;
   uchar *ptr_after_colcnt= (uchar*) ptr_colcnt;
@@ -3753,6 +3761,13 @@ Table_map_log_event::Table_map_log_event(const uchar *buf, uint event_len,
     /* Copy the different parts into their memory */
     strncpy(const_cast<char*>(m_dbnam), (const char*)ptr_dblen  + 1, m_dblen + 1);
     strncpy(const_cast<char*>(m_tblnam), (const char*)ptr_tbllen + 1, m_tbllen + 1);
+    /*
+      Future uses of the database and table name require a null-terminating
+      string, but the source string from the above copy may not have one, and
+      strncpy will not add one. So always add it to ensure it exists.
+    */
+    const_cast<char*>(m_dbnam)[m_dblen]= '\0';
+    const_cast<char*>(m_tblnam)[m_tbllen]= '\0';
     if (unlikely(ptr_after_colcnt + m_colcnt > buf + event_len))
     {
       my_free(m_memory);

sql/sql_repl.cc

@@ -2107,6 +2107,44 @@ send_event_to_slave(binlog_send_info *info, Log_event_type event_type,
     }
   }
 
+  /*
+    For a Table_map event, rewrite the copy sent to the replica so its table
+    name is 255 bytes, the largest a one-byte length can express, with no
+    terminating null. Growing the name shifts the column count and metadata
+    after it to the right and lengthens the event, so the event's declared
+    length is rewritten to match. Note that, for simplicity, the test that
+    invokes this is configured without checksumming.
+  */
+  DBUG_EXECUTE_IF("binlog_sender_oversized_table_name",
+  {
+    if (event_type == TABLE_MAP_EVENT)
+    {
+      uchar *base= (uchar*) packet->ptr();
+      uchar *ev= base + ev_offset;
+      uchar *dbnam_len_ptr= ev + LOG_EVENT_HEADER_LEN + TABLE_MAP_HEADER_LEN;
+      uint dblen= *dbnam_len_ptr;
+      uchar *tblnam_len_ptr= dbnam_len_ptr + dblen + 2;
+      uint tbllen= *tblnam_len_ptr;
+      size_t tblnam_len_off= (size_t) (tblnam_len_ptr - base);
+      size_t tblnam_off= tblnam_len_off + 1;
+      size_t tail_off= tblnam_off + tbllen + 1;
+      size_t tail_len= len - tail_off;
+      long delta= 256 - (long) (tbllen + 1);
+
+      if (!packet->realloc((size_t) ((long) len + delta)))
+      {
+        base= (uchar*) packet->ptr();
+        memmove(base + tail_off + delta, base + tail_off, tail_len);
+        base[tblnam_len_off]= 255;
+        memset(base + tblnam_off, 'a', 256);
+        len= (size_t) ((long) len + delta);
+        packet->length(len);
+        int4store(base + ev_offset + EVENT_LEN_OFFSET,
+                  (uint32) (len - ev_offset));
+      }
+    }
+  });
+
   /*
     Put a second copy of the event in the packet behind the first, the
     other shape the same disagreement takes on the wire. This injection