diff --git a/mysql-test/suite/binlog/r/binlog_old_versions.result b/mysql-test/suite/binlog/r/binlog_old_versions.result index 30b64535eb43e..de8e270a3b99d 100644 --- a/mysql-test/suite/binlog/r/binlog_old_versions.result +++ b/mysql-test/suite/binlog/r/binlog_old_versions.result @@ -13,6 +13,7 @@ SELECT COUNT(*) FROM t3; COUNT(*) 17920 DROP TABLE t1, t2, t3; +FOUND 3 /[Uu]nknown [Ee]vent/ in invalid_row_v2_tag.sql ==== Read modern binlog (version 5.1.23) ==== SELECT * FROM t1 ORDER BY a; a b 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/std_data/invalid_row_v2_tag.001 b/mysql-test/suite/binlog/std_data/invalid_row_v2_tag.001 new file mode 100644 index 0000000000000..4b558372ea550 Binary files /dev/null and b/mysql-test/suite/binlog/std_data/invalid_row_v2_tag.001 differ diff --git a/mysql-test/suite/binlog/t/binlog_old_versions.test b/mysql-test/suite/binlog/t/binlog_old_versions.test index 130101541e39b..2517c72e78f1e 100644 --- a/mysql-test/suite/binlog/t/binlog_old_versions.test +++ b/mysql-test/suite/binlog/t/binlog_old_versions.test @@ -35,6 +35,18 @@ SELECT COUNT(*) FROM t3; # Reset. DROP TABLE t1, t2, t3; +# 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 +--let SEARCH_PATTERN= [Uu]nknown [Ee]vent +--let SEARCH_FILE= $MYSQLTEST_VARDIR/tmp/invalid_row_v2_tag.sql +--exec $MYSQL_BINLOG --force-read --verbose suite/binlog/std_data/invalid_row_v2_tag.001 > $SEARCH_FILE +--source include/search_pattern_in_file.inc + --echo ==== Read modern binlog (version 5.1.23) ==== 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 f72423718db59..55e6b7d8a41ab 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -1022,7 +1022,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 enum_binlog_checksum_alg alg; DBUG_ENTER("Log_event::read_log_event(char*,...)"); DBUG_ASSERT(fdle != 0); @@ -1041,14 +1041,21 @@ 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))->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. @@ -1063,11 +1070,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->checksum_alg : get_checksum_alg(buf, event_len); + alg= fdle->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); @@ -1076,6 +1081,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 @@ -2269,33 +2275,39 @@ Format_description_log_event(const uchar *buf, uint event_len, common_header_len(0), post_header_len(NULL), event_type_permutation(0) { DBUG_ENTER("Format_description_log_event::Format_description_log_event(char*,...)"); - if (!Start_log_event_v3::is_valid()) + if (unlikely( + event_len < LOG_EVENT_MINIMAL_HEADER_LEN + ST_POST_HEADER_LEN_OFFSET || + !Start_log_event_v3::is_valid())) DBUG_VOID_RETURN; /* sanity check */ buf+= LOG_EVENT_MINIMAL_HEADER_LEN; - if ((common_header_len=buf[ST_COMMON_HEADER_LEN_OFFSET]) < OLD_HEADER_LEN) + if (unlikely( + (common_header_len=buf[ST_COMMON_HEADER_LEN_OFFSET]) < OLD_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; - checksum_alg= (enum_binlog_checksum_alg)post_header_len[number_of_event_types]; + checksum_alg= (enum_binlog_checksum_alg)buf[number_of_event_types]; } else { checksum_alg= BINLOG_CHECKSUM_ALG_UNDEF; } + 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(); @@ -2408,35 +2420,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 enum_binlog_checksum_alg get_checksum_alg(const uchar *buf, ulong len) +bool get_checksum_alg(const uchar *buf, ulong len, + enum enum_binlog_checksum_alg *alg) { - enum 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:: @@ -3396,9 +3423,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 de93450669c2a..ec90bb806ce7e 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 @@ -280,6 +280,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) */ @@ -5868,7 +5869,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 enum_binlog_checksum_alg get_checksum_alg(const uchar *buf, ulong len); +bool get_checksum_alg(const uchar *buf, ulong len, + enum 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 00a393a03955a..ec36fc5692682 100644 --- a/sql/log_event_server.cc +++ b/sql/log_event_server.cc @@ -2372,9 +2372,12 @@ bool Format_description_log_event::write() We don't call Start_log_event_v3::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_EVALUATE_IF("truncate_fde_common_header_len", + ST_COMMON_HEADER_LEN_OFFSET, sizeof(buff)); + size_t rec_size= buff_size + + DBUG_EVALUATE_IF("truncate_fde_at_post_header_len", 0, + 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) @@ -2413,10 +2416,11 @@ bool Format_description_log_event::write() checksum_alg= BINLOG_CHECKSUM_ALG_CRC32; // Forcing (V) room to fill anyway } ret= write_header(rec_size) || - write_data(buff, sizeof(buff)) || + write_data(buff, buff_size) || + DBUG_EVALUATE_IF("truncate_fde_at_post_header_len", false, write_data(post_header_len, number_of_event_types) || write_data(&checksum_byte, sizeof(checksum_byte)) || - write_footer(); + write_footer()); if (no_checksum) checksum_alg= BINLOG_CHECKSUM_ALG_OFF; return ret; diff --git a/sql/slave.cc b/sql/slave.cc index 9ca95dee93851..bbffb3db216e1 100644 --- a/sql/slave.cc +++ b/sql/slave.cc @@ -6520,7 +6520,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 25d5c9e0a585f..3a7987f55d926 100644 --- a/sql/sql_repl.cc +++ b/sql/sql_repl.cc @@ -1511,10 +1511,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)))) { @@ -2248,6 +2248,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; @@ -2314,9 +2316,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 || @@ -2344,8 +2351,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;