From 06d075767187292f3952631c9d2e63a61ebc96aa Mon Sep 17 00:00:00 2001 From: Luke Lu Date: Wed, 24 Jun 2026 22:39:35 +0000 Subject: [PATCH] MDEV-39689: Fix OOB reads in Table_map_log_event parsing Add bounds checks to prevent out-of-bounds memory reads when parsing Table_map_log_event optional metadata and constructor fields. parse_* functions: Add if (unlikely(p > end)) return after every net_field_length() call. Ensures decoded values are validated before being stored to struct fields, preventing information disclosure from OOB reads. On early return, the slave gracefully falls back to positional column mapping. Table_map_log_event constructor: Add bounds checks before memcpy calls for m_colcnt, m_field_metadata_size, and m_null_bits. On overflow, frees allocated memory and sets m_memory=NULL causing is_valid() to return false. Adds read-side DBUG_EXECUTE_IF injection points for testing. Functions fixed: - parse_default_charset - parse_column_charset - parse_column_name - parse_set_str_value - parse_geometry_type - parse_simple_pk - parse_pk_with_prefix - Table_map_log_event constructor (m_colcnt, m_field_metadata, m_null_bits) MTR test rpl_table_map_log_event_overflow covers all affected code paths with 9 test cases using debug injection. --- .../t/rpl_table_map_log_event_overflow.cnf | 4 + .../t/rpl_table_map_log_event_overflow.test | 185 ++++++++++++++++++ sql/log_event.cc | 84 +++++++- sql/log_event_server.cc | 34 +++- 4 files changed, 290 insertions(+), 17 deletions(-) create mode 100644 mysql-test/suite/rpl/t/rpl_table_map_log_event_overflow.cnf create mode 100644 mysql-test/suite/rpl/t/rpl_table_map_log_event_overflow.test diff --git a/mysql-test/suite/rpl/t/rpl_table_map_log_event_overflow.cnf b/mysql-test/suite/rpl/t/rpl_table_map_log_event_overflow.cnf new file mode 100644 index 0000000000000..d38c155473c8c --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_table_map_log_event_overflow.cnf @@ -0,0 +1,4 @@ +!include ../my.cnf + +[mysqld.1] +binlog_row_metadata=FULL diff --git a/mysql-test/suite/rpl/t/rpl_table_map_log_event_overflow.test b/mysql-test/suite/rpl/t/rpl_table_map_log_event_overflow.test new file mode 100644 index 0000000000000..2952dc9cadf10 --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_table_map_log_event_overflow.test @@ -0,0 +1,185 @@ +--source include/have_debug.inc +--source include/have_binlog_format_row.inc +--source include/master-slave.inc + +--echo # +--echo # MDEV-39689: Slave Overflow on Malformed Table_map_log_event +--echo # +--echo # All tests verify that corrupted optional metadata lengths in +--echo # parse_* functions are handled gracefully. The slave should fall +--echo # back to positional column mapping and replicate successfully. +--echo # + +--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 # Test 1: parse_column_name - corrupted column name length +--echo # + +--connection master +CREATE TABLE t1 (c1 INT PRIMARY KEY); +SET @@SESSION.debug_dbug="+d,corrupt_table_map_column_name_length"; +INSERT INTO t1 VALUES (1); +SET @@SESSION.debug_dbug= ""; +--sync_slave_with_master +SELECT * FROM t1; + +--connection master +DROP TABLE t1; +--sync_slave_with_master + +--echo # +--echo # Test 2: parse_default_charset - corrupted default charset length +--echo # + +--connection master +CREATE TABLE t1 (c1 VARCHAR(10) CHARSET latin1, c2 VARCHAR(10) CHARSET latin1, c3 VARCHAR(10) CHARSET utf8mb4); +SET @@SESSION.debug_dbug="+d,corrupt_table_map_default_charset_length"; +INSERT INTO t1 VALUES ('a', 'b', 'c'); +SET @@SESSION.debug_dbug= ""; +--sync_slave_with_master +SELECT * FROM t1; + +--connection master +DROP TABLE t1; +--sync_slave_with_master + +--echo # +--echo # Test 3: parse_column_charset - corrupted column charset length +--echo # + +--connection master +CREATE TABLE t1 (c1 VARCHAR(10) CHARSET latin1); +SET @@SESSION.debug_dbug="+d,corrupt_table_map_column_charset_length"; +INSERT INTO t1 VALUES ('a'); +SET @@SESSION.debug_dbug= ""; +--sync_slave_with_master +SELECT * FROM t1; + +--connection master +DROP TABLE t1; +--sync_slave_with_master + +--echo # +--echo # Test 4: parse_set_str_value - corrupted SET string value length +--echo # + +--connection master +CREATE TABLE t1 (c1 SET('a','b','c')); +SET @@SESSION.debug_dbug="+d,corrupt_table_map_set_str_value_length"; +INSERT INTO t1 VALUES ('a'); +SET @@SESSION.debug_dbug= ""; +--sync_slave_with_master +SELECT * FROM t1; + +--connection master +DROP TABLE t1; +--sync_slave_with_master + +--echo # +--echo # Test 5: parse_geometry_type - corrupted geometry type length +--echo # + +--connection master +CREATE TABLE t1 (c1 GEOMETRY NOT NULL); +SET @@SESSION.debug_dbug="+d,corrupt_table_map_geometry_type_length"; +INSERT INTO t1 VALUES (PointFromText('POINT(0 0)')); +SET @@SESSION.debug_dbug= ""; +--sync_slave_with_master +SELECT ST_AsText(c1) FROM t1; + +--connection master +DROP TABLE t1; +--sync_slave_with_master + +--echo # +--echo # Test 6: parse_simple_pk - corrupted simple primary key length +--echo # + +--connection master +CREATE TABLE t1 (c1 INT PRIMARY KEY); +SET @@SESSION.debug_dbug="+d,corrupt_table_map_simple_pk_length"; +INSERT INTO t1 VALUES (1); +SET @@SESSION.debug_dbug= ""; +--sync_slave_with_master +SELECT * FROM t1; + +--connection master +DROP TABLE t1; +--sync_slave_with_master + +--echo # +--echo # Test 7: parse_pk_with_prefix - corrupted prefix primary key length +--echo # + +--connection master +CREATE TABLE t1 (c1 VARCHAR(100), PRIMARY KEY(c1(10))); +SET @@SESSION.debug_dbug="+d,corrupt_table_map_pk_prefix_length"; +INSERT INTO t1 VALUES ('hello'); +SET @@SESSION.debug_dbug= ""; +--sync_slave_with_master +SELECT * FROM t1; + +--connection master +DROP TABLE t1; +--sync_slave_with_master + +--echo # +--echo # Test 8: Table_map_log_event constructor - corrupted column count +--echo # (slave-side injection) - slave rejects event gracefully +--echo # + +--connection slave +--source include/stop_slave.inc +SET @saved_dbug= @@GLOBAL.debug_dbug; +SET @@GLOBAL.debug_dbug="+d,corrupt_table_map_colcnt_read"; +--source include/start_slave.inc + +--connection master +CREATE TABLE t1 (c1 INT PRIMARY KEY); +INSERT INTO t1 VALUES (1); +--source include/save_master_gtid.inc + +--connection slave +--let $slave_sql_errno= 1594 +--source include/wait_for_slave_sql_error.inc +SET @@GLOBAL.debug_dbug= @saved_dbug; +--source include/stop_slave_io.inc +SET GLOBAL sql_slave_skip_counter= 1; +--source include/start_slave.inc + +--connection master +DROP TABLE t1; +--sync_slave_with_master + +--echo # +--echo # Test 9: Table_map_log_event constructor - corrupted field metadata size +--echo # (slave-side injection) - slave rejects event gracefully +--echo # + +--connection slave +--source include/stop_slave.inc +SET @saved_dbug= @@GLOBAL.debug_dbug; +SET @@GLOBAL.debug_dbug="+d,corrupt_table_map_field_metadata_size_read"; +--source include/start_slave.inc + +--connection master +CREATE TABLE t1 (c1 INT PRIMARY KEY); +INSERT INTO t1 VALUES (1); +--source include/save_master_gtid.inc + +--connection slave +--let $slave_sql_errno= 1594 +--source include/wait_for_slave_sql_error.inc +SET @@GLOBAL.debug_dbug= @saved_dbug; +--source include/stop_slave_io.inc +SET GLOBAL sql_slave_skip_counter= 1; +--source include/start_slave.inc + +--connection master +DROP TABLE t1; +--sync_slave_with_master + +--source include/rpl_end.inc diff --git a/sql/log_event.cc b/sql/log_event.cc index 1175b3be7c79b..f72423718db59 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -3733,6 +3733,8 @@ Table_map_log_event::Table_map_log_event(const uchar *buf, uint event_len, uchar *ptr_after_colcnt= (uchar*) ptr_colcnt; VALIDATE_BYTES_READ(ptr_after_colcnt, buf, event_len); m_colcnt= net_field_length(&ptr_after_colcnt); + DBUG_EXECUTE_IF("corrupt_table_map_colcnt_read", + m_colcnt= (1 << 20);); DBUG_PRINT("info",("m_dblen: %lu off: %ld m_tbllen: %lu off: %ld m_colcnt: %lu off: %ld", (ulong) m_dblen, (long) (ptr_dblen - vpart), @@ -3751,11 +3753,19 @@ 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); + if (unlikely(ptr_after_colcnt + m_colcnt > buf + event_len)) + { + my_free(m_memory); + m_memory= NULL; + DBUG_VOID_RETURN; + } memcpy(m_coltype, ptr_after_colcnt, m_colcnt); ptr_after_colcnt= ptr_after_colcnt + m_colcnt; VALIDATE_BYTES_READ(ptr_after_colcnt, buf, event_len); m_field_metadata_size= net_field_length(&ptr_after_colcnt); + DBUG_EXECUTE_IF("corrupt_table_map_field_metadata_size_read", + m_field_metadata_size= (1 << 20);); if (m_field_metadata_size <= (m_colcnt * 2)) { uint num_null_bytes= (m_colcnt + 7) / 8; @@ -3763,8 +3773,24 @@ Table_map_log_event::Table_map_log_event(const uchar *buf, uint event_len, &m_null_bits, num_null_bytes, &m_field_metadata, m_field_metadata_size, NULL); + if (unlikely(ptr_after_colcnt + m_field_metadata_size > buf + event_len)) + { + my_free(m_meta_memory); + m_meta_memory= NULL; + my_free(m_memory); + m_memory= NULL; + DBUG_VOID_RETURN; + } memcpy(m_field_metadata, ptr_after_colcnt, m_field_metadata_size); ptr_after_colcnt= (uchar*)ptr_after_colcnt + m_field_metadata_size; + if (unlikely(ptr_after_colcnt + num_null_bytes > buf + event_len)) + { + my_free(m_meta_memory); + m_meta_memory= NULL; + my_free(m_memory); + m_memory= NULL; + DBUG_VOID_RETURN; + } memcpy(m_null_bits, ptr_after_colcnt, num_null_bytes); ptr_after_colcnt= (unsigned char*)ptr_after_colcnt + num_null_bytes; } @@ -3837,12 +3863,19 @@ static void parse_default_charset(Table_map_log_event::Optional_metadata_fields: unsigned char *field, unsigned int length) { unsigned char* p= field; + unsigned char* end= field + length; default_charset.default_charset= net_field_length(&p); - while (p < field + length) + if (unlikely(p > end)) + return; + while (p < end) { unsigned int col_index= net_field_length(&p); + if (unlikely(p > end)) + return; unsigned int col_charset= net_field_length(&p); + if (unlikely(p > end)) + return; default_charset.charset_pairs.push_back(std::make_pair(col_index, col_charset)); @@ -3860,9 +3893,15 @@ static void parse_column_charset(std::vector &vec, unsigned char *field, unsigned int length) { unsigned char* p= field; + unsigned char* end= field + length; - while (p < field + length) - vec.push_back(net_field_length(&p)); + while (p < end) + { + unsigned int charset= net_field_length(&p); + if (unlikely(p > end)) + return; + vec.push_back(charset); + } } /** @@ -3876,10 +3915,13 @@ static void parse_column_name(std::vector &vec, unsigned char *field, unsigned int length) { unsigned char* p= field; + unsigned char* end= field + length; - while (p < field + length) + while (p < end) { unsigned len= net_field_length(&p); + if (unlikely(p + len > end)) + return; vec.push_back(std::string(reinterpret_cast(p), len)); p+= len; } @@ -3900,15 +3942,20 @@ static void parse_set_str_value(std::vector end)) + return; vec.push_back(std::vector()); for (unsigned int i= 0; i < count; i++) { unsigned len1= net_field_length(&p); + if (unlikely(p + len1 > end)) + return; vec.back().push_back(std::string(reinterpret_cast(p), len1)); p+= len1; } @@ -3926,9 +3973,15 @@ static void parse_geometry_type(std::vector &vec, unsigned char *field, unsigned int length) { unsigned char* p= field; + unsigned char* end= field + length; - while (p < field + length) - vec.push_back(net_field_length(&p)); + while (p < end) + { + unsigned int geom_type= net_field_length(&p); + if (unlikely(p > end)) + return; + vec.push_back(geom_type); + } } /** @@ -3946,9 +3999,15 @@ static void parse_simple_pk(std::vector end)) + return; + vec.push_back(std::make_pair(col_index, (unsigned int) 0)); + } } /** @@ -3966,11 +4025,16 @@ static void parse_pk_with_prefix(std::vector end)) + return; unsigned int col_prefix= net_field_length(&p); + if (unlikely(p > end)) + return; vec.push_back(std::make_pair(col_index, col_prefix)); } } diff --git a/sql/log_event_server.cc b/sql/log_event_server.cc index 2bc64a5b3a1fd..ffe8d196da564 100644 --- a/sql/log_event_server.cc +++ b/sql/log_event_server.cc @@ -6947,7 +6947,10 @@ bool Table_map_log_event::init_charset_field( for (unsigned int i= 0 ; i < m_table->s->fields ; ++i) { if (include_type(binlog_type_info_array, m_table->field[i])) - store_compressed_length(buf, binlog_type_info_array[i].m_cs->number); + store_compressed_length( + buf, + DBUG_EVALUATE_IF("corrupt_table_map_column_charset_length", 1, 0) ? + (1 << 10) : binlog_type_info_array[i].m_cs->number); } return write_tlv_field(m_metadata_buf, column_charset_type, buf); } @@ -6977,7 +6980,10 @@ bool Table_map_log_event::init_charset_field( DBUG_ASSERT(cs); if (cs->number != default_collation) { - store_compressed_length(buf, char_column_index); + store_compressed_length( + buf, + DBUG_EVALUATE_IF("corrupt_table_map_default_charset_length", 1, 0) ? + (1 << 10) : char_column_index); store_compressed_length(buf, cs->number); } char_column_index++; @@ -6995,7 +7001,9 @@ bool Table_map_log_event::init_column_name_field() { size_t len= m_table->field[i]->field_name.length; - store_compressed_length(buf, len); + store_compressed_length( + buf, + DBUG_EVALUATE_IF("corrupt_table_map_column_name_length", 1, 0) ? (1 << 10) : len); buf.append(m_table->field[i]->field_name.str, len); } return write_tlv_field(m_metadata_buf, COLUMN_NAME, buf); @@ -7018,7 +7026,10 @@ bool Table_map_log_event::init_set_str_value_field() { if ((typelib= binlog_type_info_array[i].m_set_typelib)) { - store_compressed_length(buf, typelib->count); + store_compressed_length( + buf, + DBUG_EVALUATE_IF("corrupt_table_map_set_str_value_length", 1, 0) ? + (1 << 10) : typelib->count); for (unsigned int i= 0; i < typelib->count; i++) { store_compressed_length(buf, typelib->type_lengths[i]); @@ -7067,7 +7078,10 @@ bool Table_map_log_event::init_geometry_type_field() { geom_type= binlog_type_info_array[i].m_geom_type; DBUG_EXECUTE_IF("inject_invalid_geometry_type", geom_type= 100;); - store_compressed_length(buf, geom_type); + store_compressed_length( + buf, + DBUG_EVALUATE_IF("corrupt_table_map_geometry_type_length", 1, 0) ? + (1 << 10) : geom_type); } } @@ -7108,7 +7122,10 @@ bool Table_map_log_event::init_primary_key_field() for (uint i= 0; i < pk->user_defined_key_parts; i++) { KEY_PART_INFO *key_part= pk->key_part+i; - store_compressed_length(buf, key_part->fieldnr-1); + store_compressed_length( + buf, + DBUG_EVALUATE_IF("corrupt_table_map_simple_pk_length", 1, 0) ? + (1 << 10) : key_part->fieldnr-1); } return write_tlv_field(m_metadata_buf, SIMPLE_PRIMARY_KEY, buf); } @@ -7120,7 +7137,10 @@ bool Table_map_log_event::init_primary_key_field() KEY_PART_INFO *key_part= pk->key_part+i; size_t prefix= 0; - store_compressed_length(buf, key_part->fieldnr-1); + store_compressed_length( + buf, + DBUG_EVALUATE_IF("corrupt_table_map_pk_prefix_length", 1, 0) ? + (1 << 10) : key_part->fieldnr-1); // Store character length but not octet length if (key_part->length != m_table->field[key_part->fieldnr-1]->key_length())