diff --git a/mysql-test/suite/rpl/r/rpl_table_map_name_overflow.result b/mysql-test/suite/rpl/r/rpl_table_map_name_overflow.result new file mode 100644 index 0000000000000..5b8c22ff4981e --- /dev/null +++ b/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 diff --git a/mysql-test/suite/rpl/t/rpl_table_map_name_overflow-master.opt b/mysql-test/suite/rpl/t/rpl_table_map_name_overflow-master.opt new file mode 100644 index 0000000000000..dc66bb4c46a2a --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_table_map_name_overflow-master.opt @@ -0,0 +1 @@ +--binlog-checksum=NONE diff --git a/mysql-test/suite/rpl/t/rpl_table_map_name_overflow.test b/mysql-test/suite/rpl/t/rpl_table_map_name_overflow.test new file mode 100644 index 0000000000000..b92f31123fb3f --- /dev/null +++ b/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 diff --git a/sql/log_event.cc b/sql/log_event.cc index f72423718db59..92ff61b953ebc 100644 --- a/sql/log_event.cc +++ b/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(m_dbnam), (const char*)ptr_dblen + 1, m_dblen + 1); strncpy(const_cast(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(m_dbnam)[m_dblen]= '\0'; + const_cast(m_tblnam)[m_tbllen]= '\0'; if (unlikely(ptr_after_colcnt + m_colcnt > buf + event_len)) { my_free(m_memory); diff --git a/sql/sql_repl.cc b/sql/sql_repl.cc index 3571990082f1b..2dbde8e63ad60 100644 --- a/sql/sql_repl.cc +++ b/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