diff --git a/mysql-test/main/binlog_invalid_row_v2_tag.test b/mysql-test/main/binlog_invalid_row_v2_tag.test new file mode 100644 index 0000000000000..283e27ff66a14 --- /dev/null +++ b/mysql-test/main/binlog_invalid_row_v2_tag.test @@ -0,0 +1,14 @@ +# MDEV-39485 Heap-buffer-overflow upon read in `Rows_log_event` constructor +# +# This binlog file contains a normal Format Description +# Event followed by 3 malformed v2 Write Rows Events: +# 1. A tag followed by no data +# 2. An undersized tagged data followed by an unrecognized tag +# 3. A tag followed by a length longer than the entire event + +--source include/not_embedded.inc + +--let SEARCH_PATTERN= [Uu]nknown [Ee]vent +--let SEARCH_FILE= $MYSQLTEST_VARDIR/tmp/invalid_row_v2_tag.sql +--exec $MYSQL_BINLOG --force-read --verbose std_data/binlog_invalid_row_v2_tag.001 > $SEARCH_FILE +--source include/search_pattern_in_file.inc diff --git a/mysql-test/main/invalid_row_v2_tag.result b/mysql-test/main/invalid_row_v2_tag.result new file mode 100644 index 0000000000000..63f13f71399a1 --- /dev/null +++ b/mysql-test/main/invalid_row_v2_tag.result @@ -0,0 +1 @@ +FOUND 3 /[Uu]nknown [Ee]vent/ in invalid_row_v2_tag.sql diff --git a/mysql-test/std_data/binlog_invalid_row_v2_tag.001 b/mysql-test/std_data/binlog_invalid_row_v2_tag.001 new file mode 100644 index 0000000000000..4b558372ea550 Binary files /dev/null and b/mysql-test/std_data/binlog_invalid_row_v2_tag.001 differ diff --git a/mysql-test/suite/binlog/r/fdle_overflow.result b/mysql-test/suite/binlog/r/fdle_overflow.result new file mode 100644 index 0000000000000..cb2e2f293ccb0 --- /dev/null +++ b/mysql-test/suite/binlog/r/fdle_overflow.result @@ -0,0 +1,11 @@ +SET @saved_dbug= @@GLOBAL.debug_dbug; +SET @@GLOBAL.debug_dbug= '+d,truncate_fde_at_post_header_len'; +FLUSH BINARY LOGS; +SHOW BINLOG EVENTS IN 'master-bin.000002'; +ERROR HY000: Error when executing command SHOW BINLOG EVENTS: Wrong offset or I/O error +SET @@GLOBAL.debug_dbug= '+d,truncate_fde_common_header_len'; +FLUSH BINARY LOGS; +SHOW BINLOG EVENTS IN 'master-bin.000003'; +ERROR HY000: Error when executing command SHOW BINLOG EVENTS: Wrong offset or I/O error +SET @@GLOBAL.debug_dbug= @saved_dbug; +RESET MASTER; diff --git a/mysql-test/suite/binlog/t/fdle_overflow.test b/mysql-test/suite/binlog/t/fdle_overflow.test new file mode 100644 index 0000000000000..b4a997abe1a88 --- /dev/null +++ b/mysql-test/suite/binlog/t/fdle_overflow.test @@ -0,0 +1,33 @@ +--source include/have_debug.inc +--source include/have_binlog_format_mixed.inc # format-agnostic + +# OOB read on malformed `Format_description_log_event` +# +# Verify that Format Description Events with truncated contents fails gracefully +# rather than underflow the post-header count and lead to buffer over-read. + +SET @saved_dbug= @@GLOBAL.debug_dbug; + + +# MDEV-40366: `used_checksum_alg` + +SET @@GLOBAL.debug_dbug= '+d,truncate_fde_at_post_header_len'; +FLUSH BINARY LOGS; + +--let $binlog_file= query_get_value(SHOW BINLOG STATUS, File, 1) +--error ER_ERROR_WHEN_EXECUTING_COMMAND +--eval SHOW BINLOG EVENTS IN '$binlog_file' + + +# MDEV-40365: `common_header_len` & `post_header_len` + +SET @@GLOBAL.debug_dbug= '+d,truncate_fde_common_header_len'; +FLUSH BINARY LOGS; + +--let $binlog_file= query_get_value(SHOW BINLOG STATUS, File, 1) +--error ER_ERROR_WHEN_EXECUTING_COMMAND +--eval SHOW BINLOG EVENTS IN '$binlog_file' + +# Clean-up +SET @@GLOBAL.debug_dbug= @saved_dbug; +RESET MASTER; diff --git a/sql/log_event.cc b/sql/log_event.cc index 62f6c2ccf4514..aa4956d85c4d6 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -982,7 +982,7 @@ Log_event* Log_event::read_log_event(const uchar *buf, uint event_len, my_bool crc_check, my_bool print_errors) { - Log_event* ev; + Log_event* ev= nullptr; enum_binlog_checksum_alg alg; DBUG_ENTER("Log_event::read_log_event(char*,...)"); DBUG_ASSERT(fdle != 0); @@ -1001,15 +1001,22 @@ Log_event* Log_event::read_log_event(const uchar *buf, uint event_len, } uint event_type= buf[EVENT_TYPE_OFFSET]; + switch (event_type) { + case FORMAT_DESCRIPTION_EVENT: + // If event is FD the descriptor is in it. + if (unlikely(get_checksum_alg(buf, event_len, &alg))) + goto exit; + break; + case START_EVENT_V3: // all following START events in the current file are without checksum - if (event_type == START_EVENT_V3) (const_cast< Format_description_log_event *>(fdle))->used_checksum_alg= BINLOG_CHECKSUM_ALG_OFF; + // fall-through + default: /* CRC verification by SQL and Show-Binlog-Events master side. The caller has to provide @fdle->checksum_alg to be the last seen FD's (A) descriptor. - If event is FD the descriptor is in it. Notice, FD of the binlog can be only in one instance and therefore Show-Binlog-Events executing master side thread needs just to know the only FD's (A) value - whereas RL can contain more. @@ -1024,11 +1031,9 @@ Log_event* Log_event::read_log_event(const uchar *buf, uint event_len, Notice, a pre-checksum FD version forces alg := BINLOG_CHECKSUM_ALG_UNDEF. */ - alg= (event_type != FORMAT_DESCRIPTION_EVENT) ? - fdle->used_checksum_alg : get_checksum_alg(buf, event_len); + alg= fdle->used_checksum_alg; // Emulate the corruption during reading an event DBUG_EXECUTE_IF("corrupt_read_log_event_char", - if (event_type != FORMAT_DESCRIPTION_EVENT) { uchar *debug_event_buf_c= const_cast(buf); int debug_cor_pos= rand() % (event_len - BINLOG_CHECKSUM_LEN); @@ -1037,6 +1042,7 @@ Log_event* Log_event::read_log_event(const uchar *buf, uint event_len, DBUG_SET("-d,corrupt_read_log_event_char"); } ); + } if (crc_check && event_checksum_test(const_cast(buf), event_len, alg)) { #ifdef MYSQL_CLIENT @@ -2156,7 +2162,8 @@ Format_description_log_event(const uchar *buf, uint event_len, { DBUG_ENTER("Format_description_log_event::Format_description_log_event(char*,...)"); used_checksum_alg= BINLOG_CHECKSUM_ALG_UNDEF; - if (event_len < LOG_EVENT_MINIMAL_HEADER_LEN + ST_COMMON_HEADER_LEN_OFFSET) + if (unlikely( + event_len < LOG_EVENT_MINIMAL_HEADER_LEN + ST_POST_HEADER_LEN_OFFSET)) { server_version[0]= 0; DBUG_VOID_RETURN; @@ -2169,32 +2176,36 @@ Format_description_log_event(const uchar *buf, uint event_len, created= uint4korr(buf+ST_CREATED_OFFSET); dont_set_created= 1; - if (server_version[0] == 0) + if (unlikely(server_version[0] == 0)) DBUG_VOID_RETURN; /* sanity check */ - if ((common_header_len=buf[ST_COMMON_HEADER_LEN_OFFSET]) < LOG_EVENT_MINIMAL_HEADER_LEN) + common_header_len= buf[ST_COMMON_HEADER_LEN_OFFSET]; + if (unlikely(common_header_len < LOG_EVENT_MINIMAL_HEADER_LEN)) DBUG_VOID_RETURN; /* sanity check */ number_of_event_types= - event_len - (LOG_EVENT_MINIMAL_HEADER_LEN + ST_COMMON_HEADER_LEN_OFFSET + 1); + event_len - (LOG_EVENT_MINIMAL_HEADER_LEN + ST_POST_HEADER_LEN_OFFSET); DBUG_PRINT("info", ("common_header_len=%d number_of_event_types=%d", common_header_len, number_of_event_types)); /* If alloc fails, we'll detect it in is_valid() */ - post_header_len= (uint8*) my_memdup(PSI_INSTRUMENT_ME, - buf+ST_COMMON_HEADER_LEN_OFFSET+1, - number_of_event_types* - sizeof(*post_header_len), - MYF(0)); calc_server_version_split(); + buf+= ST_POST_HEADER_LEN_OFFSET; if (!is_version_before_checksum(&server_version_split)) { - /* the last bytes are the checksum alg desc and value (or value's room) */ + /* the last bytes are the checksum alg desc */ + if (unlikely(number_of_event_types < BINLOG_CHECKSUM_ALG_DESC_LEN)) + DBUG_VOID_RETURN; /* sanity check: But there are no last bytes. */ number_of_event_types -= BINLOG_CHECKSUM_ALG_DESC_LEN; - used_checksum_alg= (enum_binlog_checksum_alg)post_header_len[number_of_event_types]; + used_checksum_alg= (enum_binlog_checksum_alg)buf[number_of_event_types]; } else { used_checksum_alg= BINLOG_CHECKSUM_ALG_OFF; } + post_header_len= (uint8*) my_memdup(PSI_INSTRUMENT_ME, + buf, + number_of_event_types* + sizeof(*post_header_len), + MYF(0)); deduct_options_written_to_bin_log(); reset_crypto(); @@ -2308,35 +2319,50 @@ Format_description_log_event::is_version_before_checksum(const master_version_sp } /** - @param buf buffer holding serialized FD event - @param len netto (possible checksum is stripped off) length of the event buf - - @return the version-safe checksum alg descriptor where zero + @param buf buffer holding serialized FD event including the 4-byte checksum + @param len length of the event buf + @param alg output the version-safe checksum alg descriptor where zero designates no checksum, 255 - the orginator is checksum-unaware (effectively no checksum) and the actuall [1-254] range alg descriptor. + @return whether this is an invalid FD event */ -enum_binlog_checksum_alg get_checksum_alg(const uchar *buf, ulong len) +bool get_checksum_alg(const uchar *buf, ulong len, + enum_binlog_checksum_alg *alg) { - enum_binlog_checksum_alg ret; + constexpr ptrdiff_t POST_HEADER_LEN_OFFSET= + LOG_EVENT_MINIMAL_HEADER_LEN + ST_POST_HEADER_LEN_OFFSET; char version[ST_SERVER_VER_LEN]; DBUG_ENTER("get_checksum_alg"); DBUG_ASSERT(buf[EVENT_TYPE_OFFSET] == FORMAT_DESCRIPTION_EVENT); + if (unlikely(len < POST_HEADER_LEN_OFFSET)) + DBUG_RETURN(true); memcpy(version, buf + LOG_EVENT_MINIMAL_HEADER_LEN + ST_SERVER_VER_OFFSET, ST_SERVER_VER_LEN); version[ST_SERVER_VER_LEN - 1]= 0; Format_description_log_event::master_version_split version_split(version); - ret= Format_description_log_event::is_version_before_checksum(&version_split) - ? BINLOG_CHECKSUM_ALG_UNDEF - : (enum_binlog_checksum_alg)buf[len - BINLOG_CHECKSUM_LEN - BINLOG_CHECKSUM_ALG_DESC_LEN]; - DBUG_ASSERT(ret == BINLOG_CHECKSUM_ALG_OFF || - ret == BINLOG_CHECKSUM_ALG_UNDEF || - ret == BINLOG_CHECKSUM_ALG_CRC32); - DBUG_RETURN(ret); + if (Format_description_log_event::is_version_before_checksum(&version_split)) + *alg= BINLOG_CHECKSUM_ALG_UNDEF; + else + { + /* + len >= POST_HEADER_LEN_OFFSET > + BINLOG_CHECKSUM_LEN + BINLOG_CHECKSUM_ALG_DESC_LEN + */ + ulong checksum_alg_offset= + len - BINLOG_CHECKSUM_LEN - BINLOG_CHECKSUM_ALG_DESC_LEN; + if (unlikely(checksum_alg_offset < POST_HEADER_LEN_OFFSET)) + DBUG_RETURN(true); + *alg= static_cast(buf[checksum_alg_offset]); + } + DBUG_ASSERT(*alg == BINLOG_CHECKSUM_ALG_OFF || + *alg == BINLOG_CHECKSUM_ALG_UNDEF || + *alg == BINLOG_CHECKSUM_ALG_CRC32); + DBUG_RETURN(false); } Start_encryption_log_event:: @@ -3120,9 +3146,17 @@ Rows_log_event::Rows_log_event(const uchar *buf, uint event_len, case RW_V_EXTRAINFO_TAG: { /* Have an 'extra info' section, read it in */ - assert((end - pos) >= EXTRA_ROW_INFO_HDR_BYTES); + if (unlikely((end - pos) <= EXTRA_ROW_INFO_LEN_OFFSET)) + { + m_cols.bitmap= 0; + DBUG_VOID_RETURN; + } uint8 infoLen= pos[EXTRA_ROW_INFO_LEN_OFFSET]; - assert((end - pos) >= infoLen); + if (unlikely(infoLen < EXTRA_ROW_INFO_HDR_BYTES || (end-pos) < infoLen)) + { + m_cols.bitmap= 0; + DBUG_VOID_RETURN; + } /* Just store/use the first tag of this type, skip others */ if (likely(!m_extra_row_data)) { diff --git a/sql/log_event.h b/sql/log_event.h index abd5cd54aa714..31863f6d769cf 100644 --- a/sql/log_event.h +++ b/sql/log_event.h @@ -204,7 +204,7 @@ class String; #define STOP_HEADER_LEN 0 #define LOAD_HEADER_LEN (4 + 4 + 4 + 1 +1 + 4) #define SLAVE_HEADER_LEN 0 -#define START_V3_HEADER_LEN (2 + ST_SERVER_VER_LEN + 4) +#define START_V3_HEADER_LEN ST_COMMON_HEADER_LEN_OFFSET #define ROTATE_HEADER_LEN 8 // this is FROZEN (the Rotate post-header is frozen) #define INTVAR_HEADER_LEN 0 #define CREATE_FILE_HEADER_LEN 4 @@ -285,6 +285,7 @@ class String; #define ST_SERVER_VER_OFFSET 2 #define ST_CREATED_OFFSET (ST_SERVER_VER_OFFSET + ST_SERVER_VER_LEN) #define ST_COMMON_HEADER_LEN_OFFSET (ST_CREATED_OFFSET + 4) +#define ST_POST_HEADER_LEN_OFFSET (ST_COMMON_HEADER_LEN_OFFSET + 1) /* slave event post-header (this event is never written) */ @@ -5504,7 +5505,8 @@ bool slave_execute_deferred_events(THD *thd); bool event_that_should_be_ignored(const uchar *buf); bool event_checksum_test(uchar *buf, ulong event_len, enum_binlog_checksum_alg alg); -enum_binlog_checksum_alg get_checksum_alg(const uchar *buf, ulong len); +bool get_checksum_alg(const uchar *buf, ulong len, + enum_binlog_checksum_alg *alg); extern TYPELIB binlog_checksum_typelib; #ifdef WITH_WSREP enum Log_event_type wsrep_peak_event(rpl_group_info *rgi, ulonglong* event_size); diff --git a/sql/log_event_server.cc b/sql/log_event_server.cc index 85139fece910f..218bc9498a74f 100644 --- a/sql/log_event_server.cc +++ b/sql/log_event_server.cc @@ -2417,9 +2417,12 @@ bool Format_description_log_event::write(Log_event_writer *writer) We don't call Start_log_event_v::write() because this would make 2 my_b_safe_write(). */ - uchar buff[START_V3_HEADER_LEN+1]; - size_t rec_size= sizeof(buff) + BINLOG_CHECKSUM_ALG_DESC_LEN + - number_of_event_types; + uchar buff[ST_POST_HEADER_LEN_OFFSET]; + const size_t buff_size= DBUG_IF("truncate_fde_common_header_len") ? + ST_COMMON_HEADER_LEN_OFFSET : sizeof(buff); + size_t rec_size= buff_size; + if (!DBUG_IF("truncate_fde_at_post_header_len")) + rec_size += BINLOG_CHECKSUM_ALG_DESC_LEN + number_of_event_types; int2store(buff + ST_BINLOG_VER_OFFSET,binlog_version); memcpy((char*) buff + ST_SERVER_VER_OFFSET,server_version,ST_SERVER_VER_LEN); if (!dont_set_created) @@ -2454,10 +2457,12 @@ bool Format_description_log_event::write(Log_event_writer *writer) uint orig_checksum_len= writer->checksum_len; writer->checksum_len= BINLOG_CHECKSUM_LEN; ret= write_header(writer, rec_size) || - write_data(writer, buff, sizeof(buff)) || + write_data(writer, buff, buff_size) || + (!DBUG_IF("truncate_fde_at_post_header_len") && ( write_data(writer, post_header_len, number_of_event_types) || write_data(writer, &checksum_byte, sizeof(checksum_byte)) || - write_footer(writer); + write_footer(writer) + )); writer->checksum_len= orig_checksum_len; return ret; } diff --git a/sql/slave.cc b/sql/slave.cc index 74efaff5734b6..8abb01e4863d8 100644 --- a/sql/slave.cc +++ b/sql/slave.cc @@ -6328,7 +6328,12 @@ static int queue_event(Master_info* mi, const uchar *buf, ulong event_len) */ if (buf[EVENT_TYPE_OFFSET] == FORMAT_DESCRIPTION_EVENT) { - checksum_alg= get_checksum_alg(buf, event_len); + if (unlikely(get_checksum_alg(buf, event_len, &checksum_alg))) + { + error= ER_SLAVE_RELAY_LOG_WRITE_FAILURE; + unlock_data_lock= FALSE; + goto err; + } } else if (buf[EVENT_TYPE_OFFSET] == START_EVENT_V3) { diff --git a/sql/sql_repl.cc b/sql/sql_repl.cc index 613a24aa47d26..19ff5d9e6383f 100644 --- a/sql/sql_repl.cc +++ b/sql/sql_repl.cc @@ -1706,10 +1706,10 @@ gtid_state_from_pos(const char *name, uint32 offset, goto end; } - current_checksum_alg= get_checksum_alg((uchar*) packet.ptr(), - packet.length()); found_format_description_event= true; - if (unlikely(!(tmp= new Format_description_log_event((uchar*) packet.ptr(), + if (unlikely(get_checksum_alg((uchar*) packet.ptr(), packet.length(), + ¤t_checksum_alg) || + !(tmp= new Format_description_log_event((uchar*)packet.ptr(), packet.length(), fdev)))) { @@ -2483,6 +2483,8 @@ static int init_binlog_sender(binlog_send_info *info, static int send_format_descriptor_event(binlog_send_info *info, IO_CACHE *log, LOG_INFO *linfo, my_off_t start_pos) { + static constexpr const char* CORRUPT_FDE= + "Corrupt Format_description event found or out-of-memory"; int error; ulong ev_offset; THD *thd= info->thd; @@ -2549,9 +2551,14 @@ static int send_format_descriptor_event(binlog_send_info *info, IO_CACHE *log, DBUG_RETURN(1); } - info->current_checksum_alg= get_checksum_alg((uchar*) packet->ptr() + - ev_offset, - packet->length() - ev_offset); + if (unlikely(get_checksum_alg((uchar*) packet->ptr() + ev_offset, + packet->length() - ev_offset, + &(info->current_checksum_alg)))) + { + info->error= ER_MASTER_FATAL_ERROR_READING_BINLOG; + info->errmsg= CORRUPT_FDE; + DBUG_RETURN(1); + } DBUG_ASSERT(info->current_checksum_alg == BINLOG_CHECKSUM_ALG_OFF || info->current_checksum_alg == BINLOG_CHECKSUM_ALG_UNDEF || @@ -2579,8 +2586,7 @@ static int send_format_descriptor_event(binlog_send_info *info, IO_CACHE *log, ev_len, info->fdev))) { info->error= ER_MASTER_FATAL_ERROR_READING_BINLOG; - info->errmsg= "Corrupt Format_description event found " - "or out-of-memory"; + info->errmsg= CORRUPT_FDE; DBUG_RETURN(1); } delete info->fdev;