diff --git a/mysql-test/r/mysqlbinlog.result b/mysql-test/r/mysqlbinlog.result index d16a4c39a11..9f001c293de 100644 --- a/mysql-test/r/mysqlbinlog.result +++ b/mysql-test/r/mysqlbinlog.result @@ -325,4 +325,5 @@ flush logs; drop table t1; 1 drop table t1; +shell> mysqlbinlog std_data/corrupt-relay-bin.000624 > var/tmp/bug31793.sql End of 5.0 tests diff --git a/mysql-test/std_data/corrupt-relay-bin.000624 b/mysql-test/std_data/corrupt-relay-bin.000624 new file mode 100644 index 00000000000..21b4901211c Binary files /dev/null and b/mysql-test/std_data/corrupt-relay-bin.000624 differ diff --git a/mysql-test/t/mysqlbinlog.test b/mysql-test/t/mysqlbinlog.test index 451eef17108..c83fe94f2eb 100644 --- a/mysql-test/t/mysqlbinlog.test +++ b/mysql-test/t/mysqlbinlog.test @@ -237,4 +237,8 @@ let $c= `select $a=$b`; --echo $c drop table t1; +echo shell> mysqlbinlog std_data/corrupt-relay-bin.000624 > var/tmp/bug31793.sql; +error 1; +exec $MYSQL_BINLOG $MYSQL_TEST_DIR/std_data/corrupt-relay-bin.000624 > $MYSQLTEST_VARDIR/tmp/bug31793.sql; + --echo End of 5.0 tests diff --git a/sql/log_event.cc b/sql/log_event.cc index 3899e772bf8..0e257bf7f6c 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -1400,17 +1400,46 @@ Query_log_event::Query_log_event(THD* thd_arg, const char* query_arg, /* 2 utility functions for the next method */ -/* - Get the pointer for a string (src) that contains the length in - the first byte. Set the output string (dst) to the string value - and place the length of the string in the byte after the string. +/** + Read a string with length from memory. + + This function reads the string-with-length stored at + src and extract the length into *len and + a pointer to the start of the string into *dst. The + string can then be copied using memcpy() with the + number of bytes given in *len. + + @param src Pointer to variable holding a pointer to the memory to + read the string from. + @param dst Pointer to variable holding a pointer where the actual + string starts. Starting from this position, the string + can be copied using @c memcpy(). + @param len Pointer to variable where the length will be stored. + @param end One-past-the-end of the memory where the string is + stored. + + @return Zero if the entire string can be copied successfully, + @c UINT_MAX if the length could not be read from memory + (that is, if *src >= end), otherwise the + number of bytes that are missing to read the full + string, which happends *dst + *len >= end. */ -static void get_str_len_and_pointer(const Log_event::Byte **src, - const char **dst, - uint *len) +static int +get_str_len_and_pointer(const Log_event::Byte **src, + const char **dst, + uint *len, + const Log_event::Byte *end) { - if ((*len= **src)) - *dst= (char *)*src + 1; // Will be copied later + if (*src >= end) + return -1; // Will be UINT_MAX in two-complement arithmetics + uint length= **src; + if (length > 0) + { + if (*src + length >= end) + return *src + length - end; // Number of bytes missing + *dst= (char *)*src + 1; // Will be copied later + } + *len= length; (*src)+= *len + 1; } @@ -1424,6 +1453,23 @@ static void copy_str_and_move(const char **src, *(*dst)++= 0; } + +/** + Macro to check that there is enough space to read from memory. + + @param PTR Pointer to memory + @param END End of memory + @param CNT Number of bytes that should be read. + */ +#define CHECK_SPACE(PTR,END,CNT) \ + do { \ + DBUG_ASSERT((PTR) + (CNT) <= (END)); \ + if ((PTR) + (CNT) > (END)) { \ + query= 0; \ + DBUG_VOID_RETURN; \ + } \ + } while (0) + /* Query_log_event::Query_log_event() This is used by the SQL slave thread to prepare the event before execution. @@ -1475,6 +1521,17 @@ Query_log_event::Query_log_event(const char* buf, uint event_len, if (tmp) { status_vars_len= uint2korr(buf + Q_STATUS_VARS_LEN_OFFSET); + /* + Check if status variable length is corrupt and will lead to very + wrong data. We could be even more strict and require data_len to + be even bigger, but this will suffice to catch most corruption + errors that can lead to a crash. + */ + if (status_vars_len >= min(data_len + 1, MAX_SIZE_LOG_EVENT_STATUS)) + { + query= 0; + DBUG_VOID_RETURN; + } data_len-= status_vars_len; DBUG_PRINT("info", ("Query_log_event has status_vars_len: %u", (uint) status_vars_len)); @@ -1494,6 +1551,7 @@ Query_log_event::Query_log_event(const char* buf, uint event_len, { switch (*pos++) { case Q_FLAGS2_CODE: + CHECK_SPACE(pos, end, 4); flags2_inited= 1; flags2= uint4korr(pos); DBUG_PRINT("info",("In Query_log_event, read flags2: %lu", (ulong) flags2)); @@ -1504,6 +1562,7 @@ Query_log_event::Query_log_event(const char* buf, uint event_len, #ifndef DBUG_OFF char buff[22]; #endif + CHECK_SPACE(pos, end, 8); sql_mode_inited= 1; sql_mode= (ulong) uint8korr(pos); // QQ: Fix when sql_mode is ulonglong DBUG_PRINT("info",("In Query_log_event, read sql_mode: %s", @@ -1512,15 +1571,21 @@ Query_log_event::Query_log_event(const char* buf, uint event_len, break; } case Q_CATALOG_NZ_CODE: - get_str_len_and_pointer(&pos, &catalog, &catalog_len); + if (get_str_len_and_pointer(&pos, &catalog, &catalog_len, end)) + { + query= 0; + DBUG_VOID_RETURN; + } break; case Q_AUTO_INCREMENT: + CHECK_SPACE(pos, end, 4); auto_increment_increment= uint2korr(pos); auto_increment_offset= uint2korr(pos+2); pos+= 4; break; case Q_CHARSET_CODE: { + CHECK_SPACE(pos, end, 6); charset_inited= 1; memcpy(charset, pos, 6); pos+= 6; @@ -1528,20 +1593,28 @@ Query_log_event::Query_log_event(const char* buf, uint event_len, } case Q_TIME_ZONE_CODE: { - get_str_len_and_pointer(&pos, &time_zone_str, &time_zone_len); + if (get_str_len_and_pointer(&pos, &time_zone_str, &time_zone_len, end)) + { + query= 0; + DBUG_VOID_RETURN; + } break; } case Q_CATALOG_CODE: /* for 5.0.x where 0<=x<=3 masters */ + CHECK_SPACE(pos, end, 1); if ((catalog_len= *pos)) catalog= (char*) pos+1; // Will be copied later + CHECK_SPACE(pos, end, catalog_len + 2); pos+= catalog_len+2; // leap over end 0 catalog_nz= 0; // catalog has end 0 in event break; case Q_LC_TIME_NAMES_CODE: + CHECK_SPACE(pos, end, 2); lc_time_names_number= uint2korr(pos); pos+= 2; break; case Q_CHARSET_DATABASE_CODE: + CHECK_SPACE(pos, end, 2); charset_database_number= uint2korr(pos); pos+= 2; break;