Bug#31752: check strmake() bounds

strmake() calls are easy to get wrong. Add checks in extra
debug mode to identify possible exploits.

Remove some dead code.

Remove some off-by-one errors identified with new checks.
parent 39f6cbc2
...@@ -966,7 +966,7 @@ void MYSQL_LOG::make_log_name(char* buf, const char* log_ident) ...@@ -966,7 +966,7 @@ void MYSQL_LOG::make_log_name(char* buf, const char* log_ident)
if (dir_len > FN_REFLEN) if (dir_len > FN_REFLEN)
dir_len=FN_REFLEN-1; dir_len=FN_REFLEN-1;
strnmov(buf, log_file_name, dir_len); strnmov(buf, log_file_name, dir_len);
strmake(buf+dir_len, log_ident, FN_REFLEN - dir_len); strmake(buf+dir_len, log_ident, FN_REFLEN - dir_len -1);
} }
......
...@@ -926,7 +926,7 @@ int load_master_data(THD* thd) ...@@ -926,7 +926,7 @@ int load_master_data(THD* thd)
0, (SLAVE_IO | SLAVE_SQL))) 0, (SLAVE_IO | SLAVE_SQL)))
send_error(thd, ER_MASTER_INFO); send_error(thd, ER_MASTER_INFO);
strmake(active_mi->master_log_name, row[0], strmake(active_mi->master_log_name, row[0],
sizeof(active_mi->master_log_name)); sizeof(active_mi->master_log_name) -1);
active_mi->master_log_pos= my_strtoll10(row[1], (char**) 0, &error); active_mi->master_log_pos= my_strtoll10(row[1], (char**) 0, &error);
/* at least in recent versions, the condition below should be false */ /* at least in recent versions, the condition below should be false */
if (active_mi->master_log_pos < BIN_LOG_HEADER_SIZE) if (active_mi->master_log_pos < BIN_LOG_HEADER_SIZE)
......
...@@ -1573,7 +1573,7 @@ bool sys_var::check_set(THD *thd, set_var *var, TYPELIB *enum_names) ...@@ -1573,7 +1573,7 @@ bool sys_var::check_set(THD *thd, set_var *var, TYPELIB *enum_names)
&not_used)); &not_used));
if (error_len) if (error_len)
{ {
strmake(buff, error, min(sizeof(buff), error_len)); strmake(buff, error, min(sizeof(buff) - 1, error_len));
goto err; goto err;
} }
} }
......
...@@ -136,7 +136,7 @@ int mysqld_show_tables(THD *thd,const char *db,const char *wild) ...@@ -136,7 +136,7 @@ int mysqld_show_tables(THD *thd,const char *db,const char *wild)
{ {
Item_string *field=new Item_string("",0,thd->charset()); Item_string *field=new Item_string("",0,thd->charset());
List<Item> field_list; List<Item> field_list;
char path[FN_LEN],*end; char path[FN_REFLEN],*end;
List<char> files; List<char> files;
char *file_name; char *file_name;
Protocol *protocol= thd->protocol; Protocol *protocol= thd->protocol;
...@@ -457,7 +457,7 @@ int mysqld_extend_show_tables(THD *thd,const char *db,const char *wild) ...@@ -457,7 +457,7 @@ int mysqld_extend_show_tables(THD *thd,const char *db,const char *wild)
Item *item; Item *item;
List<char> files; List<char> files;
List<Item> field_list; List<Item> field_list;
char path[FN_LEN]; char path[FN_REFLEN];
char *file_name; char *file_name;
TABLE *table; TABLE *table;
Protocol *protocol= thd->protocol; Protocol *protocol= thd->protocol;
......
...@@ -140,6 +140,9 @@ bool mysql_create_frm(THD *thd, my_string file_name, ...@@ -140,6 +140,9 @@ bool mysql_create_frm(THD *thd, my_string file_name,
strmake((char*) forminfo+47,create_info->comment ? create_info->comment : "", strmake((char*) forminfo+47,create_info->comment ? create_info->comment : "",
60); 60);
forminfo[46]=(uchar) strlen((char*)forminfo+47); // Length of comment forminfo[46]=(uchar) strlen((char*)forminfo+47); // Length of comment
#ifdef EXTRA_DEBUG
memset((char*) forminfo+47 + forminfo[46], 0, 61 - forminfo[46]);
#endif
if (my_pwrite(file,(byte*) fileinfo,64,0L,MYF_RW) || if (my_pwrite(file,(byte*) fileinfo,64,0L,MYF_RW) ||
my_pwrite(file,(byte*) keybuff,key_info_length, my_pwrite(file,(byte*) keybuff,key_info_length,
......
...@@ -28,23 +28,25 @@ ...@@ -28,23 +28,25 @@
#include <my_global.h> #include <my_global.h>
#include "m_string.h" #include "m_string.h"
#ifdef BAD_STRING_COMPILER char *strmake(register char *dst, register const char *src, uint length)
char *strmake(char *dst,const char *src,uint length)
{ {
reg1 char *res; #ifdef EXTRA_DEBUG
/*
if ((res=memccpy(dst,src,0,length))) 'length' is the maximum length of the string; the buffer needs
return res-1; to be one character larger to accomodate the terminating '\0'.
dst[length]=0; This is easy to get wrong, so we make sure we write to the
return dst+length; entire length of the buffer to identify incorrect buffer-sizes.
} We only initialise the "unused" part of the buffer here, a) for
efficiency, and b) because dst==src is allowed, so initialising
#define strmake strmake_overlapp /* Use orginal for overlapping str */ the entire buffer would overwrite the source-string. Also, we
write a character rather than '\0' as this makes spotting these
problems in the results easier.
*/
uint n= strlen(src) + 1;
if (n <= length)
memset(dst + n, (int) 'Z', length - n + 1);
#endif #endif
char *strmake(register char *dst, register const char *src, uint length)
{
while (length--) while (length--)
if (! (*dst++ = *src++)) if (! (*dst++ = *src++))
return dst-1; return dst-1;
......
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