From 1cdb56278cfc142733a2130cdd5ccef56d9620d7 Mon Sep 17 00:00:00 2001 From: ParadoxV5 Date: Thu, 16 Jul 2026 18:11:14 -0600 Subject: [PATCH 1/3] MDEV-40365 OOB read on malformed `Format_description_log_event` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The binary-parsing Format Description Event constructor did not validate content length beyond the superclass `is_valid()` call. When parsing a malformed FDE, to load the content fields added in FDE v4 that are missing in this not-FDE, the parser constructor would read from erroneous memory locations beyond the buffer. If this did not outright crash the program, this would corrupt the FDE. With the Format Description playing a critical role in determining how to parse the events to follow, a corrupted FDE would also corrupt (or trigger a crash in) the parsing of subsequent non-FDE events as well. This commit fills in the validation with a FDE-specific guard. It adds the FDE constant `ST_POST_HEADER_LEN_OFFSET` to assist with comparing to the correct minimum size in the future. (Both of these points are designed to merge with the superclass’s guard as part of the MDEV-30128 merger). --- .../suite/binlog/r/fdle_overflow.result | 7 ++++++ mysql-test/suite/binlog/t/fdle_overflow.test | 22 +++++++++++++++++++ sql/log_event.cc | 11 ++++++---- sql/log_event.h | 3 ++- sql/log_event_server.cc | 14 +++++++----- 5 files changed, 47 insertions(+), 10 deletions(-) create mode 100644 mysql-test/suite/binlog/r/fdle_overflow.result create mode 100644 mysql-test/suite/binlog/t/fdle_overflow.test 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..28fc630f29091 --- /dev/null +++ b/mysql-test/suite/binlog/r/fdle_overflow.result @@ -0,0 +1,7 @@ +SET @saved_dbug= @@GLOBAL.debug_dbug; +SET @@GLOBAL.debug_dbug= '+d,truncate_fde_at_common_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= @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..e8ffd84538293 --- /dev/null +++ b/mysql-test/suite/binlog/t/fdle_overflow.test @@ -0,0 +1,22 @@ +--source include/have_debug.inc +--source include/have_binlog_format_mixed.inc # format-agnostic + +# MDEV-40365 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; + + +SET @@GLOBAL.debug_dbug= '+d,truncate_fde_at_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..ac9fcbcda4d20 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -2269,19 +2269,22 @@ 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, + buf + ST_POST_HEADER_LEN_OFFSET, number_of_event_types* sizeof(*post_header_len), MYF(0)); diff --git a/sql/log_event.h b/sql/log_event.h index de93450669c2a..35a690bbf5345 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) */ diff --git a/sql/log_event_server.cc b/sql/log_event_server.cc index 00a393a03955a..6730c027ff5fe 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_at_common_header_len", + ST_COMMON_HEADER_LEN_OFFSET, sizeof(buff)); + size_t rec_size= buff_size + + DBUG_EVALUATE_IF("truncate_fde_at_common_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_common_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; From ea38444e91c629df4975b12d107669bdeda7da15 Mon Sep 17 00:00:00 2001 From: ParadoxV5 Date: Sun, 26 Jul 2026 16:11:28 -0600 Subject: [PATCH 2/3] MDEV-40366 OOB read on malformed `Format_description_log_event` Neither the binary-parsing Format Description Event constructor nor the constructor-bypassing `get_checksum_alg()` function validated the content length of the passed event buffer. If they receive an FDE with undersized contents, they would obtain corrupt results from an erronous memory location, if not outright crash the program with that memory error. This commit fixes both sites by adding content length checks. Because the goal is not to solve the existence of two binary parsers, `get_checksum_alg()` receives a check duplicated from the Format Description constructor and is no longer a function that never errors. --- .../suite/binlog/r/fdle_overflow.result | 6 +- mysql-test/suite/binlog/t/fdle_overflow.test | 15 +++- sql/log_event.cc | 76 ++++++++++++------- sql/log_event.h | 3 +- sql/log_event_server.cc | 6 +- sql/slave.cc | 7 +- sql/sql_repl.cc | 22 ++++-- 7 files changed, 93 insertions(+), 42 deletions(-) diff --git a/mysql-test/suite/binlog/r/fdle_overflow.result b/mysql-test/suite/binlog/r/fdle_overflow.result index 28fc630f29091..cb2e2f293ccb0 100644 --- a/mysql-test/suite/binlog/r/fdle_overflow.result +++ b/mysql-test/suite/binlog/r/fdle_overflow.result @@ -1,7 +1,11 @@ SET @saved_dbug= @@GLOBAL.debug_dbug; -SET @@GLOBAL.debug_dbug= '+d,truncate_fde_at_common_header_len'; +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 index e8ffd84538293..b4a997abe1a88 100644 --- a/mysql-test/suite/binlog/t/fdle_overflow.test +++ b/mysql-test/suite/binlog/t/fdle_overflow.test @@ -1,7 +1,7 @@ --source include/have_debug.inc --source include/have_binlog_format_mixed.inc # format-agnostic -# MDEV-40365 OOB read on malformed `Format_description_log_event` +# 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. @@ -9,7 +9,9 @@ SET @saved_dbug= @@GLOBAL.debug_dbug; -SET @@GLOBAL.debug_dbug= '+d,truncate_fde_at_common_header_len'; +# 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) @@ -17,6 +19,15 @@ FLUSH BINARY LOGS; --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 ac9fcbcda4d20..c079e4f79f79c 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 @@ -2283,22 +2289,25 @@ Format_description_log_event(const uchar *buf, uint event_len, 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_POST_HEADER_LEN_OFFSET, - 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(); @@ -2411,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:: diff --git a/sql/log_event.h b/sql/log_event.h index 35a690bbf5345..ec90bb806ce7e 100644 --- a/sql/log_event.h +++ b/sql/log_event.h @@ -5869,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 6730c027ff5fe..ec36fc5692682 100644 --- a/sql/log_event_server.cc +++ b/sql/log_event_server.cc @@ -2373,10 +2373,10 @@ bool Format_description_log_event::write() my_b_safe_write(). */ uchar buff[ST_POST_HEADER_LEN_OFFSET]; - const size_t buff_size= DBUG_EVALUATE_IF("truncate_fde_at_common_header_len", + 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_common_header_len", 0, + 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); @@ -2417,7 +2417,7 @@ bool Format_description_log_event::write() } ret= write_header(rec_size) || write_data(buff, buff_size) || - DBUG_EVALUATE_IF("truncate_fde_at_common_header_len", false, + 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()); 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; From 6e4df4353bdb0a4bee34ad8f39748efcae188100 Mon Sep 17 00:00:00 2001 From: ParadoxV5 Date: Tue, 21 Jul 2026 00:16:15 -0600 Subject: [PATCH 3/3] MDEV-39485 Heap-buffer-overflow upon read in `Rows_log_event` constructor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MariaDB recognizes Version 2 Rows Events from MySQL, including the format of the “extra data” field added in this version. (MDEV-5115) When parsing this extra data according to the format, whether this data has sufficient length was only checked by assertions in the `Rows_log_event` constructor and the `mariadb-binlog --verbose` printer. When parsing an event with malformed extra data, these assertions * would straight up terminate the program in debug builds. * were stripped in non-debug (release) builds. This would render the parser defenseless to reading from erroneous memory locations outside of the containing event, which will either crash the program or, for `mariadb-binlog --verbose`, snapshot the running memory to be exposed when outputting the event. This commit replaces those assertions with an actual validity check. Since MariaDB does not generate v2 Rows Events, the included test uses a handcrafted binlog file. --- .../suite/binlog/r/binlog_old_versions.result | 1 + .../suite/binlog/std_data/invalid_row_v2_tag.001 | Bin 0 -> 353 bytes .../suite/binlog/t/binlog_old_versions.test | 12 ++++++++++++ sql/log_event.cc | 12 ++++++++++-- 4 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 mysql-test/suite/binlog/std_data/invalid_row_v2_tag.001 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/std_data/invalid_row_v2_tag.001 b/mysql-test/suite/binlog/std_data/invalid_row_v2_tag.001 new file mode 100644 index 0000000000000000000000000000000000000000..4b558372ea550f6aace9cefa6992b199efbb243a GIT binary patch literal 353 zcmeyDl$m!YJvNJy*KV8@?!YmBDKefEmPMkB3Mr0@)5I(yR;&jQ>#;B%&(-$^!sP>lvQ_ literal 0 HcmV?d00001 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/sql/log_event.cc b/sql/log_event.cc index c079e4f79f79c..55e6b7d8a41ab 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -3423,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)) {