Commit f8a8ac0e authored by Nuno Carvalho's avatar Nuno Carvalho

BUG#14629727: USER_VAR_EVENT IS MISSING RANGE CHECKS

This bug had two problems:
 P1) Reads out of bounds;
 P2) Writes out of bounds.

PROBLEM P1
----------
User_var_log_event unmarshalling from binlog was not performing range
checks when using name_len and val_len variables to walk on event
buffer.

Added range checks to User_var_log_event unmarshalling to prevent
unmarshalling errors.

PROBLEM P2
----------
User_var_log_event value was allocated on thread stack, what caused
stack frame errors when User_var_log_event value was bigger than thread
stack size.

Currently value is allocated on heap memory.
parent f255d63c
......@@ -1286,7 +1286,7 @@ Log_event* Log_event::read_log_event(const char* buf, uint event_len,
ev = new Rand_log_event(buf, description_event);
break;
case USER_VAR_EVENT:
ev = new User_var_log_event(buf, description_event);
ev = new User_var_log_event(buf, event_len, description_event);
break;
case FORMAT_DESCRIPTION_EVENT:
ev = new Format_description_log_event(buf, event_len, description_event);
......@@ -5685,18 +5685,34 @@ void User_var_log_event::pack_info(Protocol* protocol)
User_var_log_event::
User_var_log_event(const char* buf,
User_var_log_event(const char* buf, uint event_len,
const Format_description_log_event* description_event)
:Log_event(buf, description_event)
#ifndef MYSQL_CLIENT
, deferred(false)
#endif
{
bool error= false;
const char* buf_start= buf;
/* The Post-Header is empty. The Variable Data part begins immediately. */
buf+= description_event->common_header_len +
description_event->post_header_len[USER_VAR_EVENT-1];
name_len= uint4korr(buf);
name= (char *) buf + UV_NAME_LEN_SIZE;
/*
We don't know yet is_null value, so we must assume that name_len
may have the bigger value possible, is_null= True and there is no
payload for val.
*/
if (0 == name_len ||
!valid_buffer_range<uint>(name_len, buf_start, name,
event_len - UV_VAL_IS_NULL))
{
error= true;
goto err;
}
buf+= UV_NAME_LEN_SIZE + name_len;
is_null= (bool) *buf;
if (is_null)
......@@ -5708,13 +5724,31 @@ User_var_log_event(const char* buf,
}
else
{
if (!valid_buffer_range<uint>(UV_VAL_IS_NULL + UV_VAL_TYPE_SIZE
+ UV_CHARSET_NUMBER_SIZE + UV_VAL_LEN_SIZE,
buf_start, buf, event_len))
{
error= true;
goto err;
}
type= (Item_result) buf[UV_VAL_IS_NULL];
charset_number= uint4korr(buf + UV_VAL_IS_NULL + UV_VAL_TYPE_SIZE);
val_len= uint4korr(buf + UV_VAL_IS_NULL + UV_VAL_TYPE_SIZE +
UV_CHARSET_NUMBER_SIZE);
val= (char *) (buf + UV_VAL_IS_NULL + UV_VAL_TYPE_SIZE +
UV_CHARSET_NUMBER_SIZE + UV_VAL_LEN_SIZE);
if (!valid_buffer_range<uint>(val_len, buf_start, val, event_len))
{
error= true;
goto err;
}
}
err:
if (error)
name= 0;
}
......@@ -5860,8 +5894,9 @@ void User_var_log_event::print(FILE* file, PRINT_EVENT_INFO* print_event_info)
char *hex_str;
CHARSET_INFO *cs;
if (!(hex_str= (char *)my_alloca(2*val_len+1+2))) // 2 hex digits / byte
break; // no error, as we are 'void'
hex_str= (char *)my_malloc(2*val_len+1+2,MYF(MY_WME)); // 2 hex digits / byte
if (!hex_str)
return;
str_to_hex(hex_str, val, val_len);
/*
For proper behaviour when mysqlbinlog|mysql, we need to explicitely
......@@ -5879,7 +5914,7 @@ void User_var_log_event::print(FILE* file, PRINT_EVENT_INFO* print_event_info)
my_b_printf(&cache, ":=_%s %s COLLATE `%s`%s\n",
cs->csname, hex_str, cs->name,
print_event_info->delimiter);
my_afree(hex_str);
my_free(hex_str, MYF(MY_WME));
}
break;
case ROW_RESULT:
......
......@@ -2496,7 +2496,7 @@ class User_var_log_event: public Log_event
void print(FILE* file, PRINT_EVENT_INFO* print_event_info);
#endif
User_var_log_event(const char* buf,
User_var_log_event(const char* buf, uint event_len,
const Format_description_log_event *description_event);
~User_var_log_event() {}
Log_event_type get_type_code() { return USER_VAR_EVENT;}
......@@ -2510,7 +2510,7 @@ class User_var_log_event: public Log_event
bool is_deferred() { return deferred; }
void set_deferred() { deferred= true; }
#endif
bool is_valid() const { return 1; }
bool is_valid() const { return name != 0; }
private:
#if !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION)
......
......@@ -501,6 +501,50 @@ class Default_object_creation_ctx : public Object_creation_ctx
*/
#define MAX_TIME_ZONE_NAME_LENGTH (NAME_LEN + 1)
/*
Check how many bytes are available on buffer.
@param buf_start Pointer to buffer start.
@param buf_current Pointer to the current position on buffer.
@param buf_len Buffer length.
@return Number of bytes available on event buffer.
*/
template <class T> T available_buffer(const char* buf_start,
const char* buf_current,
T buf_len)
{
return buf_len - (buf_current - buf_start);
}
/* Explicit instantion to unsigned int. */
template unsigned int available_buffer<unsigned int>(const char*,
const char*,
unsigned int);
/*
Check if jump value is within buffer limits.
@param jump Number of positions we want to advance.
@param buf_start Pointer to buffer start
@param buf_current Pointer to the current position on buffer.
@param buf_len Buffer length.
@return True If jump value is within buffer limits.
False Otherwise.
*/
template <class T> bool valid_buffer_range(T jump,
const char* buf_start,
const char* buf_current,
T buf_len)
{
return (jump <= available_buffer(buf_start, buf_current, buf_len));
}
/* Explicit instantion to unsigned int. */
template bool valid_buffer_range<unsigned int>(unsigned int,
const char*,
const char*,
unsigned int);
/* The rest of the file is included in the server only */
#ifndef MYSQL_CLIENT
......
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