Commit 0d0c59ff authored by Sujatha Sivakumar's avatar Sujatha Sivakumar

Bug#19145698: READ OUT OF BOUNDS ISSUE

Problem:
========
In a master slave replication if a slave receives a
Start_log_event_v3 the payload is expected to be of fixed
size. If a payload which is smaller than the fixed size is
received it causes a read out of bounds issue.

Analysis:
========
According to documentation the fixed data part of
Start_log_event_v3 looks as shown below.

2 bytes: The binary log format version
50 bytes: The MySQL server's version
4 bytes: Timestamp in seconds when this event was created

Since the payload is expected to be of fixed size, therefore
ST_SERVER_VER_LEN (50) bytes are memcpy'ed into
server_version. But if a malicious master sends a shorter
payload it causes a read out of bounds issue.

Fix:
===
In Start_log_event_v3 event's constructor a check has been
added which expects the minimum payload length to be of size
common_header_len + ST_COMMON_HEADER_LEN_OFFSET bytes. If a
malicious packet of lesser length is received it will be
considered as an invalid event.
parent 0fc7b50c
...@@ -1307,7 +1307,7 @@ Log_event* Log_event::read_log_event(const char* buf, uint event_len, ...@@ -1307,7 +1307,7 @@ Log_event* Log_event::read_log_event(const char* buf, uint event_len,
ev = new Execute_load_log_event(buf, event_len, description_event); ev = new Execute_load_log_event(buf, event_len, description_event);
break; break;
case START_EVENT_V3: /* this is sent only by MySQL <=4.x */ case START_EVENT_V3: /* this is sent only by MySQL <=4.x */
ev = new Start_log_event_v3(buf, description_event); ev = new Start_log_event_v3(buf, event_len, description_event);
break; break;
case STOP_EVENT: case STOP_EVENT:
ev = new Stop_log_event(buf, description_event); ev = new Stop_log_event(buf, description_event);
...@@ -3788,11 +3788,17 @@ void Start_log_event_v3::print(FILE* file, PRINT_EVENT_INFO* print_event_info) ...@@ -3788,11 +3788,17 @@ void Start_log_event_v3::print(FILE* file, PRINT_EVENT_INFO* print_event_info)
Start_log_event_v3::Start_log_event_v3() Start_log_event_v3::Start_log_event_v3()
*/ */
Start_log_event_v3::Start_log_event_v3(const char* buf, Start_log_event_v3::Start_log_event_v3(const char* buf, uint event_len,
const Format_description_log_event const Format_description_log_event
*description_event) *description_event)
:Log_event(buf, description_event) :Log_event(buf, description_event), binlog_version(BINLOG_VERSION)
{ {
if (event_len < (uint)description_event->common_header_len +
ST_COMMON_HEADER_LEN_OFFSET)
{
server_version[0]= 0;
return;
}
buf+= description_event->common_header_len; buf+= description_event->common_header_len;
binlog_version= uint2korr(buf+ST_BINLOG_VER_OFFSET); binlog_version= uint2korr(buf+ST_BINLOG_VER_OFFSET);
memcpy(server_version, buf+ST_SERVER_VER_OFFSET, memcpy(server_version, buf+ST_SERVER_VER_OFFSET,
...@@ -4082,16 +4088,15 @@ Format_description_log_event(const char* buf, ...@@ -4082,16 +4088,15 @@ Format_description_log_event(const char* buf,
const const
Format_description_log_event* Format_description_log_event*
description_event) description_event)
:Start_log_event_v3(buf, description_event), event_type_permutation(0) :Start_log_event_v3(buf, event_len, description_event),
common_header_len(0), post_header_len(NULL), event_type_permutation(0)
{ {
DBUG_ENTER("Format_description_log_event::Format_description_log_event(char*,...)"); DBUG_ENTER("Format_description_log_event::Format_description_log_event(char*,...)");
if (!Start_log_event_v3::is_valid())
DBUG_VOID_RETURN; /* sanity check */
buf+= LOG_EVENT_MINIMAL_HEADER_LEN; buf+= LOG_EVENT_MINIMAL_HEADER_LEN;
if ((common_header_len=buf[ST_COMMON_HEADER_LEN_OFFSET]) < OLD_HEADER_LEN) if ((common_header_len=buf[ST_COMMON_HEADER_LEN_OFFSET]) < OLD_HEADER_LEN)
{
/* this makes is_valid() return false. */
post_header_len= NULL;
DBUG_VOID_RETURN; /* sanity check */ DBUG_VOID_RETURN; /* sanity check */
}
number_of_event_types= 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_COMMON_HEADER_LEN_OFFSET+1);
DBUG_PRINT("info", ("common_header_len=%d number_of_event_types=%d", DBUG_PRINT("info", ("common_header_len=%d number_of_event_types=%d",
......
...@@ -2276,14 +2276,14 @@ public: ...@@ -2276,14 +2276,14 @@ public:
void print(FILE* file, PRINT_EVENT_INFO* print_event_info); void print(FILE* file, PRINT_EVENT_INFO* print_event_info);
#endif #endif
Start_log_event_v3(const char* buf, Start_log_event_v3(const char* buf, uint event_len,
const Format_description_log_event* description_event); const Format_description_log_event* description_event);
~Start_log_event_v3() {} ~Start_log_event_v3() {}
Log_event_type get_type_code() { return START_EVENT_V3;} Log_event_type get_type_code() { return START_EVENT_V3;}
#ifdef MYSQL_SERVER #ifdef MYSQL_SERVER
bool write(IO_CACHE* file); bool write(IO_CACHE* file);
#endif #endif
bool is_valid() const { return 1; } bool is_valid() const { return server_version[0] != 0; }
int get_data_size() int get_data_size()
{ {
return START_V3_HEADER_LEN; //no variable-sized part return START_V3_HEADER_LEN; //no variable-sized part
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment