Details
-
Bug
-
Status: Open (View Workflow)
-
Major
-
Resolution: Unresolved
-
None
-
None
-
Unexpected results
Description
The slave logic to parse a Table_map_log_event's (optional) metadata doesn't validate the structure of the event. For example, if the event is corrupted on column_name where there is no length or value after the type of COLUMN_NAME, the slave will error and write garbage memory as a COLUMN_NAME it thinks exists.
The following MTR test, .cnf, and patch (to corrupt a Table_map_log_event via the debug_dbug mechanism) reproduce this.
MTR test:
--source include/have_debug.inc
|
--source include/have_binlog_format_row.inc
|
--source include/master-slave.inc
|
|
|
--connection master
|
CREATE TABLE t1 (c1 INT PRIMARY KEY, c2 VARCHAR(50));
|
|
|
--let $saved_dbug= `SELECT @@SESSION.debug_dbug`
|
SET @@SESSION.debug_dbug="+d,corrupt_table_map_column_name";
|
INSERT INTO t1 VALUES (1, 'hello');
|
--source include/save_master_gtid.inc
|
SET @@SESSION.debug_dbug=$saved_dbug;
|
|
|
--echo #
|
--echo # Error on slave shows invalid column name
|
--connection slave
|
--let $slave_sql_errno=4254
|
--let $show_slave_sql_error= 1
|
--let $slave_timeout=5
|
--source include/wait_for_slave_sql_error.inc
|
.cnf
!include ../my.cnf
|
|
|
[mysqld.1]
|
binlog_row_metadata=FULL
|
|
|
[mysqld.2]
|
slave_type_conversions=ERROR_IF_MISSING_FIELD
|
Patch
diff --git a/sql/log_event_server.cc b/sql/log_event_server.cc
|
index ea04b6fc660..005fc6c6ded 100644
|
--- a/sql/log_event_server.cc
|
+++ b/sql/log_event_server.cc
|
@@ -6898,6 +6898,10 @@ bool write_tlv_field(String &str_buf,
|
/* type is stored in one byte, so it should never bigger than 255. */
|
DBUG_ASSERT(static_cast<int>(type) <= 255);
|
str_buf.append((char) type);
|
+ DBUG_EXECUTE_IF("corrupt_table_map_column_name", {
|
+ if (type == Table_map_log_event::COLUMN_NAME)
|
+ return 0;
|
+ });
|
store_compressed_length(str_buf, length);
|
return str_buf.append(reinterpret_cast<const char *>(value), length);
|
}
|
Output
#
|
# Error on slave shows invalid column name
|
connection slave;
|
include/wait_for_slave_sql_error.inc [errno=4254]
|
Last_SQL_Error = 'Table structure for binlog event is not compatible with the table definition on this slave: Column '???????????????????????????' missing from table 'test.t1''
|
The slave-side function which does this parsing does not validate that there is still more event data before moving up the pointer which reads in the event:
Table_map_log_event::Optional_metadata_fields::
|
Optional_metadata_fields(MEM_ROOT *root, uint master_columns,
|
uchar* optional_metadata,
|
size_t optional_metadata_len, |
bool only_column_names) |
{
|
unsigned int len; |
uchar *metadata_end;
|
|
|
allocation_error= 0;
|
m_column_name= 0;
|
if (optional_metadata == NULL) |
return; |
|
|
metadata_end= optional_metadata + optional_metadata_len;
|
for (uchar *field= optional_metadata ; field < metadata_end ; field+= len) |
{
|
Optional_metadata_field_type type=
|
static_cast<Optional_metadata_field_type>(field[0]); |
|
|
// Get length and move field to the value. |
field++;
|
The the last line of the above snippet, field++ shows this shortcoming. There should be validation that metadata_end contains enough information here.
Reported by letchu_pkt
Attachments
Issue Links
- relates to
-
MDEV-39672 Mariadb-binlog Overflow on Malformed Table_map_log_event
-
- Open
-
-
MDEV-39689 Slave Overflow on Malformed Table_map_log_event
-
- Open
-