Skip to content

Commit cb29741

Browse files
author
Shishir Jaiswal
committed
Bug#21977380 - POSSIBLE BUFFER OVERFLOW ISSUES
DESCRIPTION =========== Buffer overflow is reported in a lot of code sections spanning across server, client programs, Regex libraries etc. If not handled appropriately, they can cause abnormal behaviour. ANALYSIS ======== The reported casea are the ones which are likely to result in SEGFAULT, MEMORY LEAK etc. FIX === - sprintf() has been replaced by my_snprintf() to avoid buffer overflow. - my_free() is done after checking if the pointer isn't NULL already and setting it to NULL thereafter at few places. - Buffer is ensured to be large enough to hold the data. - 'unsigned int' (aka 'uint') is replaced with 'size_t' to avoid wraparound. - Memory is freed (if not done so) after its alloced and used. - Inserted assert() for size check in InnoDb memcached code (from 5.6 onwards) - Other minor changes
1 parent df7ecf6 commit cb29741

File tree

5 files changed

+78
-57
lines changed

5 files changed

+78
-57
lines changed

‎client/mysqlcheck.c

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -213,13 +213,13 @@ static int process_selected_tables(char *db, char **table_names, int tables);
213213
static int process_all_tables_in_db(char *database);
214214
static int process_one_db(char *database);
215215
static int use_db(char *database);
216-
static int handle_request_for_tables(char *tables, uint length);
216+
static int handle_request_for_tables(char *tables, size_t length);
217217
static int dbConnect(char *host, char *user,char *passwd);
218218
static void dbDisconnect(char *host);
219219
static void DBerror(MYSQL *mysql, const char *when);
220220
static void safe_exit(int error);
221221
static void print_result();
222-
static uint fixed_name_length(const char *name);
222+
static size_t fixed_name_length(const char *name);
223223
static char *fix_table_name(char *dest, char *src);
224224
int what_to_do = 0;
225225

@@ -486,7 +486,7 @@ static int process_selected_tables(char *db, char **table_names, int tables)
486486
*end++= ',';
487487
}
488488
*--end = 0;
489-
handle_request_for_tables(table_names_comma_sep + 1, (uint) (tot_length - 1));
489+
handle_request_for_tables(table_names_comma_sep + 1, tot_length - 1);
490490
my_free(table_names_comma_sep);
491491
}
492492
else
@@ -496,10 +496,10 @@ static int process_selected_tables(char *db, char **table_names, int tables)
496496
} /* process_selected_tables */
497497

498498

499-
static uint fixed_name_length(const char *name)
499+
static size_t fixed_name_length(const char *name)
500500
{
501501
const char *p;
502-
uint extra_length= 2; /* count the first/last backticks */
502+
size_t extra_length= 2; /* count the first/last backticks */
503503

504504
for (p= name; *p; p++)
505505
{
@@ -508,7 +508,7 @@ static uint fixed_name_length(const char *name)
508508
else if (*p == '.')
509509
extra_length+= 2;
510510
}
511-
return (uint) ((p - name) + extra_length);
511+
return (size_t) ((p - name) + extra_length);
512512
}
513513

514514

@@ -564,7 +564,7 @@ static int process_all_tables_in_db(char *database)
564564
*/
565565

566566
char *tables, *end;
567-
uint tot_length = 0;
567+
size_t tot_length = 0;
568568

569569
while ((row = mysql_fetch_row(res)))
570570
tot_length+= fixed_name_length(row[0]) + 2;
@@ -622,7 +622,9 @@ static int fix_table_storage_name(const char *name)
622622
int rc= 0;
623623
if (strncmp(name, "#mysql50#", 9))
624624
return 1;
625-
sprintf(qbuf, "RENAME TABLE `%s` TO `%s`", name, name + 9);
625+
my_snprintf(qbuf, sizeof(qbuf), "RENAME TABLE `%s` TO `%s`",
626+
name, name + 9);
627+
626628
rc= run_query(qbuf);
627629
if (verbose)
628630
printf("%-50s %s\n", name, rc ? "FAILED" : "OK");
@@ -635,7 +637,8 @@ static int fix_database_storage_name(const char *name)
635637
int rc= 0;
636638
if (strncmp(name, "#mysql50#", 9))
637639
return 1;
638-
sprintf(qbuf, "ALTER DATABASE `%s` UPGRADE DATA DIRECTORY NAME", name);
640+
my_snprintf(qbuf, sizeof(qbuf), "ALTER DATABASE `%s` UPGRADE DATA DIRECTORY "
641+
"NAME", name);
639642
rc= run_query(qbuf);
640643
if (verbose)
641644
printf("%-50s %s\n", name, rc ? "FAILED" : "OK");
@@ -653,7 +656,7 @@ static int rebuild_table(char *name)
653656
ptr= strmov(query, "ALTER TABLE ");
654657
ptr= fix_table_name(ptr, name);
655658
ptr= strxmov(ptr, " FORCE", NullS);
656-
if (mysql_real_query(sock, query, (uint)(ptr - query)))
659+
if (mysql_real_query(sock, query, (ulong)(ptr - query)))
657660
{
658661
fprintf(stderr, "Failed to %s\n", query);
659662
fprintf(stderr, "Error: %s\n", mysql_error(sock));
@@ -702,10 +705,10 @@ static int disable_binlog()
702705
return run_query(stmt);
703706
}
704707

705-
static int handle_request_for_tables(char *tables, uint length)
708+
static int handle_request_for_tables(char *tables, size_t length)
706709
{
707710
char *query, *end, options[100], message[100];
708-
uint query_length= 0;
711+
size_t query_length= 0, query_size= sizeof(char)*(length+110);
709712
const char *op = 0;
710713

711714
options[0] = 0;
@@ -736,10 +739,14 @@ static int handle_request_for_tables(char *tables, uint length)
736739
return fix_table_storage_name(tables);
737740
}
738741

739-
if (!(query =(char *) my_malloc((sizeof(char)*(length+110)), MYF(MY_WME))))
742+
if (!(query =(char *) my_malloc(query_size, MYF(MY_WME))))
743+
{
740744
return 1;
745+
}
741746
if (opt_all_in_1)
742747
{
748+
DBUG_ASSERT(strlen(op)+strlen(tables)+strlen(options)+8+1 <= query_size);
749+
743750
/* No backticks here as we added them before */
744751
query_length= sprintf(query, "%s TABLE %s %s", op, tables, options);
745752
}
@@ -750,7 +757,7 @@ static int handle_request_for_tables(char *tables, uint length)
750757
ptr= strmov(strmov(query, op), " TABLE ");
751758
ptr= fix_table_name(ptr, tables);
752759
ptr= strxmov(ptr, " ", options, NullS);
753-
query_length= (uint) (ptr - query);
760+
query_length= (size_t) (ptr - query);
754761
}
755762
if (mysql_real_query(sock, query, query_length))
756763
{
@@ -834,7 +841,10 @@ static void print_result()
834841
prev_alter[0]= 0;
835842
}
836843
else
837-
strcpy(prev_alter, alter_txt);
844+
{
845+
strncpy(prev_alter, alter_txt, MAX_ALTER_STR_SIZE-1);
846+
prev_alter[MAX_ALTER_STR_SIZE-1]= 0;
847+
}
838848
}
839849
}
840850
}
@@ -978,7 +988,7 @@ int main(int argc, char **argv)
978988
process_databases(argv);
979989
if (opt_auto_repair)
980990
{
981-
uint i;
991+
size_t i;
982992

983993
if (!opt_silent && (tables4repair.elements || tables4rebuild.elements))
984994
puts("\nRepairing tables");

‎client/mysqldump.c

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@
8686

8787
static void add_load_option(DYNAMIC_STRING *str, const char *option,
8888
const char *option_value);
89-
static ulong find_set(TYPELIB *lib, const char *x, uint length,
89+
static ulong find_set(TYPELIB *lib, const char *x, size_t length,
9090
char **err_pos, uint *err_len);
9191
static char *alloc_query_str(ulong size);
9292

@@ -852,7 +852,7 @@ get_one_option(int optid, const struct my_option *opt __attribute__((unused)),
852852
opt_set_charset= 0;
853853
opt_compatible_mode_str= argument;
854854
opt_compatible_mode= find_set(&compatible_mode_typelib,
855-
argument, (uint) strlen(argument),
855+
argument, strlen(argument),
856856
&err_ptr, &err_len);
857857
if (err_len)
858858
{
@@ -862,7 +862,7 @@ get_one_option(int optid, const struct my_option *opt __attribute__((unused)),
862862
}
863863
#if !defined(DBUG_OFF)
864864
{
865-
uint size_for_sql_mode= 0;
865+
size_t size_for_sql_mode= 0;
866866
const char **ptr;
867867
for (ptr= compatible_mode_names; *ptr; ptr++)
868868
size_for_sql_mode+= strlen(*ptr);
@@ -1138,8 +1138,8 @@ static int fetch_db_collation(const char *db_name,
11381138
break;
11391139
}
11401140

1141-
strncpy(db_cl_name, db_cl_row[0], db_cl_size);
1142-
db_cl_name[db_cl_size - 1]= 0; /* just in case. */
1141+
strncpy(db_cl_name, db_cl_row[0], db_cl_size-1);
1142+
db_cl_name[db_cl_size - 1]= 0;
11431143

11441144
} while (FALSE);
11451145

@@ -1150,7 +1150,7 @@ static int fetch_db_collation(const char *db_name,
11501150

11511151

11521152
static char *my_case_str(const char *str,
1153-
uint str_len,
1153+
size_t str_len,
11541154
const char *token,
11551155
uint token_len)
11561156
{
@@ -1366,7 +1366,7 @@ static int switch_character_set_results(MYSQL *mysql, const char *cs_name)
13661366
*/
13671367

13681368
static char *cover_definer_clause(const char *stmt_str,
1369-
uint stmt_length,
1369+
size_t stmt_length,
13701370
const char *definer_version_str,
13711371
uint definer_version_length,
13721372
const char *stmt_version_str,
@@ -1548,14 +1548,14 @@ static void dbDisconnect(char *host)
15481548
} /* dbDisconnect */
15491549

15501550

1551-
static void unescape(FILE *file,char *pos,uint length)
1551+
static void unescape(FILE *file,char *pos, size_t length)
15521552
{
15531553
char *tmp;
15541554
DBUG_ENTER("unescape");
15551555
if (!(tmp=(char*) my_malloc(length*2+1, MYF(MY_WME))))
15561556
die(EX_MYSQLERR, "Couldn't allocate memory");
15571557

1558-
mysql_real_escape_string(&mysql_connection, tmp, pos, length);
1558+
mysql_real_escape_string(&mysql_connection, tmp, pos, (ulong)length);
15591559
fputc('\'', file);
15601560
fputs(tmp, file);
15611561
fputc('\'', file);
@@ -1669,7 +1669,7 @@ static char *quote_for_like(const char *name, char *buff)
16691669
Quote '<' '>' '&' '\"' chars and print a string to the xml_file.
16701670
*/
16711671

1672-
static void print_quoted_xml(FILE *xml_file, const char *str, ulong len,
1672+
static void print_quoted_xml(FILE *xml_file, const char *str, size_t len,
16731673
my_bool is_attribute_name)
16741674
{
16751675
const char *end;
@@ -1928,7 +1928,7 @@ static void print_xml_row(FILE *xml_file, const char *row_name,
19281928
squeezed to a single hyphen.
19291929
*/
19301930

1931-
static void print_xml_comment(FILE *xml_file, ulong len,
1931+
static void print_xml_comment(FILE *xml_file, size_t len,
19321932
const char *comment_string)
19331933
{
19341934
const char* end;
@@ -2045,7 +2045,7 @@ static uint dump_events_for_db(char *db)
20452045
DBUG_ENTER("dump_events_for_db");
20462046
DBUG_PRINT("enter", ("db: '%s'", db));
20472047

2048-
mysql_real_escape_string(mysql, db_name_buff, db, strlen(db));
2048+
mysql_real_escape_string(mysql, db_name_buff, db, (ulong)strlen(db));
20492049

20502050
/* nice comments */
20512051
print_comment(sql_file, 0,
@@ -2164,6 +2164,11 @@ static uint dump_events_for_db(char *db)
21642164
(const char *) (query_str != NULL ? query_str : row[3]),
21652165
(const char *) delimiter);
21662166

2167+
if(query_str)
2168+
{
2169+
my_free(query_str);
2170+
query_str= NULL;
2171+
}
21672172
restore_time_zone(sql_file, delimiter);
21682173
restore_sql_mode(sql_file, delimiter);
21692174

@@ -2257,7 +2262,7 @@ static uint dump_routines_for_db(char *db)
22572262
DBUG_ENTER("dump_routines_for_db");
22582263
DBUG_PRINT("enter", ("db: '%s'", db));
22592264

2260-
mysql_real_escape_string(mysql, db_name_buff, db, strlen(db));
2265+
mysql_real_escape_string(mysql, db_name_buff, db, (ulong)strlen(db));
22612266

22622267
/* nice comments */
22632268
print_comment(sql_file, 0,
@@ -2311,9 +2316,9 @@ static uint dump_routines_for_db(char *db)
23112316
if the user has EXECUTE privilege he see routine names, but NOT the
23122317
routine body of other routines that are not the creator of!
23132318
*/
2314-
DBUG_PRINT("info",("length of body for %s row[2] '%s' is %d",
2319+
DBUG_PRINT("info",("length of body for %s row[2] '%s' is %zu",
23152320
routine_name, row[2] ? row[2] : "(null)",
2316-
row[2] ? (int) strlen(row[2]) : 0));
2321+
row[2] ? strlen(row[2]) : 0));
23172322
if (row[2] == NULL)
23182323
{
23192324
print_comment(sql_file, 1, "\n-- insufficient privileges to %s\n",
@@ -3873,7 +3878,7 @@ static int dump_tablespaces_for_tables(char *db, char **table_names, int tables)
38733878
int i;
38743879
char name_buff[NAME_LEN*2+3];
38753880

3876-
mysql_real_escape_string(mysql, name_buff, db, strlen(db));
3881+
mysql_real_escape_string(mysql, name_buff, db, (ulong)strlen(db));
38773882

38783883
init_dynamic_string_checked(&where, " AND TABLESPACE_NAME IN ("
38793884
"SELECT DISTINCT TABLESPACE_NAME FROM"
@@ -3886,7 +3891,7 @@ static int dump_tablespaces_for_tables(char *db, char **table_names, int tables)
38863891
for (i=0 ; i<tables ; i++)
38873892
{
38883893
mysql_real_escape_string(mysql, name_buff,
3889-
table_names[i], strlen(table_names[i]));
3894+
table_names[i], (ulong)strlen(table_names[i]));
38903895

38913896
dynstr_append_checked(&where, "'");
38923897
dynstr_append_checked(&where, name_buff);
@@ -3917,7 +3922,7 @@ static int dump_tablespaces_for_databases(char** databases)
39173922
{
39183923
char db_name_buff[NAME_LEN*2+3];
39193924
mysql_real_escape_string(mysql, db_name_buff,
3920-
databases[i], strlen(databases[i]));
3925+
databases[i], (ulong)strlen(databases[i]));
39213926
dynstr_append_checked(&where, "'");
39223927
dynstr_append_checked(&where, db_name_buff);
39233928
dynstr_append_checked(&where, "',");
@@ -4927,7 +4932,7 @@ static int start_transaction(MYSQL *mysql_con)
49274932
}
49284933

49294934

4930-
static ulong find_set(TYPELIB *lib, const char *x, uint length,
4935+
static ulong find_set(TYPELIB *lib, const char *x, size_t length,
49314936
char **err_pos, uint *err_len)
49324937
{
49334938
const char *end= x + length;
@@ -4985,7 +4990,7 @@ static void print_value(FILE *file, MYSQL_RES *result, MYSQL_ROW row,
49854990
fputc(' ',file);
49864991
fputs(prefix, file);
49874992
if (string_value)
4988-
unescape(file,row[0],(uint) strlen(row[0]));
4993+
unescape(file,row[0], strlen(row[0]));
49894994
else
49904995
fputs(row[0], file);
49914996
check_io(file);
@@ -5238,8 +5243,8 @@ static my_bool get_view_structure(char *table, char* db)
52385243
verbose_msg("-- Retrieving view structure for table %s...\n", table);
52395244

52405245
#ifdef NOT_REALLY_USED_YET
5241-
sprintf(insert_pat, "SET SQL_QUOTE_SHOW_CREATE=%d",
5242-
(opt_quoted || opt_keywords));
5246+
dynstr_append_checked(&insert_pat, "SET SQL_QUOTE_SHOW_CREATE=");
5247+
dynstr_append_checked(&insert_pat, (opt_quoted || opt_keywords)? "1":"0");
52435248
#endif
52445249

52455250
result_table= quote_name(table, table_buff, 1);

0 commit comments

Comments
 (0)