Skip to content

Commit 9de9f10

Browse files
ChaloffChaloff
authored
Use memory safe snprintf() in Connect Engine and elsewhere (#2210)
Continue with similar changes as done in 19af189 to replace sprintf(buf, ...) with snprintf(buf, sizeof(buf), ...), specifically in the "easy" cases where buf is allocated with a size known at compile time. All new code of the whole pull request, including one or several files that are either new files or modified ones, are contributed under the BSD-new license. I am contributing on behalf of my employer Amazon Web Services, Inc.
1 parent b2cfcf1 commit 9de9f10

File tree

13 files changed

+70
-61
lines changed

13 files changed

+70
-61
lines changed

‎client/mysqlshow.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,7 @@ list_dbs(MYSQL *mysql,const char *wild)
449449
MYSQL_RES *tresult = mysql_list_tables(mysql,(char*)NULL);
450450
if (mysql_affected_rows(mysql) > 0)
451451
{
452-
sprintf(tables,"%6lu",(ulong) mysql_affected_rows(mysql));
452+
snprintf(tables, sizeof(tables), "%6lu",(ulong) mysql_affected_rows(mysql));
453453
rowcount = 0;
454454
if (opt_verbose > 1)
455455
{
@@ -470,13 +470,13 @@ list_dbs(MYSQL *mysql,const char *wild)
470470
}
471471
}
472472
}
473-
sprintf(rows,"%12lu",rowcount);
473+
snprintf(rows, sizeof(rows), "%12lu", rowcount);
474474
}
475475
}
476476
else
477477
{
478-
sprintf(tables,"%6d",0);
479-
sprintf(rows,"%12d",0);
478+
snprintf(tables, sizeof(tables), "%6d" ,0);
479+
snprintf(rows, sizeof(rows), "%12d", 0);
480480
}
481481
mysql_free_result(tresult);
482482
}
@@ -594,7 +594,7 @@ list_tables(MYSQL *mysql,const char *db,const char *table)
594594
}
595595
else
596596
{
597-
sprintf(fields,"%8u",(uint) mysql_num_fields(rresult));
597+
snprintf(fields, sizeof(fields), "%8u", (uint) mysql_num_fields(rresult));
598598
mysql_free_result(rresult);
599599

600600
if (opt_verbose > 1)
@@ -610,10 +610,10 @@ list_tables(MYSQL *mysql,const char *db,const char *table)
610610
rowcount += (unsigned long) strtoull(rrow[0], (char**) 0, 10);
611611
mysql_free_result(rresult);
612612
}
613-
sprintf(rows,"%10lu",rowcount);
613+
snprintf(rows, sizeof(rows), "%10lu", rowcount);
614614
}
615615
else
616-
sprintf(rows,"%10d",0);
616+
snprintf(rows, sizeof(rows), "%10d", 0);
617617
}
618618
}
619619
}

‎sql/sql_analyse.cc

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -412,7 +412,7 @@ void field_real::add()
412412

413413
if ((decs = decimals()) >= FLOATING_POINT_DECIMALS)
414414
{
415-
length= sprintf(buff, "%g", num);
415+
length= snprintf(buff, sizeof(buff), "%g", num);
416416
if (rint(num) != num)
417417
max_notzero_dec_len = 1;
418418
}
@@ -423,7 +423,7 @@ void field_real::add()
423423
snprintf(buff, sizeof(buff)-1, "%-.*f", (int) decs, num);
424424
length = (uint) strlen(buff);
425425
#else
426-
length= sprintf(buff, "%-.*f", (int) decs, num);
426+
length= snprintf(buff, sizeof(buff), "%-.*f", (int) decs, num);
427427
#endif
428428

429429
// We never need to check further than this
@@ -810,32 +810,32 @@ void field_str::get_opt_type(String *answer, ha_rows total_rows)
810810
if (can_be_still_num)
811811
{
812812
if (num_info.is_float)
813-
sprintf(buff, "DOUBLE"); // number was like 1e+50... TODO:
813+
snprintf(buff, sizeof(buff), "DOUBLE"); // number was like 1e+50... TODO:
814814
else if (num_info.decimals) // DOUBLE(%d,%d) sometime
815815
{
816816
if (num_info.dval > -FLT_MAX && num_info.dval < FLT_MAX)
817-
sprintf(buff, "FLOAT(%d,%d)", (num_info.integers + num_info.decimals), num_info.decimals);
817+
snprintf(buff, sizeof(buff), "FLOAT(%d,%d)", (num_info.integers + num_info.decimals), num_info.decimals);
818818
else
819-
sprintf(buff, "DOUBLE(%d,%d)", (num_info.integers + num_info.decimals), num_info.decimals);
819+
snprintf(buff, sizeof(buff), "DOUBLE(%d,%d)", (num_info.integers + num_info.decimals), num_info.decimals);
820820
}
821821
else if (ev_num_info.llval >= -128 &&
822822
ev_num_info.ullval <=
823823
(ulonglong) (ev_num_info.llval >= 0 ? 255 : 127))
824-
sprintf(buff, "TINYINT(%d)", num_info.integers);
824+
snprintf(buff, sizeof(buff), "TINYINT(%d)", num_info.integers);
825825
else if (ev_num_info.llval >= INT_MIN16 &&
826826
ev_num_info.ullval <= (ulonglong) (ev_num_info.llval >= 0 ?
827827
UINT_MAX16 : INT_MAX16))
828-
sprintf(buff, "SMALLINT(%d)", num_info.integers);
828+
snprintf(buff, sizeof(buff), "SMALLINT(%d)", num_info.integers);
829829
else if (ev_num_info.llval >= INT_MIN24 &&
830830
ev_num_info.ullval <= (ulonglong) (ev_num_info.llval >= 0 ?
831831
UINT_MAX24 : INT_MAX24))
832-
sprintf(buff, "MEDIUMINT(%d)", num_info.integers);
832+
snprintf(buff, sizeof(buff), "MEDIUMINT(%d)", num_info.integers);
833833
else if (ev_num_info.llval >= INT_MIN32 &&
834834
ev_num_info.ullval <= (ulonglong) (ev_num_info.llval >= 0 ?
835835
UINT_MAX32 : INT_MAX32))
836-
sprintf(buff, "INT(%d)", num_info.integers);
836+
snprintf(buff, sizeof(buff), "INT(%d)", num_info.integers);
837837
else
838-
sprintf(buff, "BIGINT(%d)", num_info.integers);
838+
snprintf(buff, sizeof(buff), "BIGINT(%d)", num_info.integers);
839839
answer->append(buff, (uint) strlen(buff));
840840
if (ev_num_info.llval >= 0 && ev_num_info.min_dval >= 0)
841841
answer->append(STRING_WITH_LEN(" UNSIGNED"));
@@ -853,12 +853,12 @@ void field_str::get_opt_type(String *answer, ha_rows total_rows)
853853
}
854854
else if ((max_length * (total_rows - nulls)) < (sum + total_rows))
855855
{
856-
sprintf(buff, "CHAR(%d)", (int) max_length);
856+
snprintf(buff, sizeof(buff), "CHAR(%d)", (int) max_length);
857857
answer->append(buff, (uint) strlen(buff));
858858
}
859859
else
860860
{
861-
sprintf(buff, "VARCHAR(%d)", (int) max_length);
861+
snprintf(buff, sizeof(buff), "VARCHAR(%d)", (int) max_length);
862862
answer->append(buff, (uint) strlen(buff));
863863
}
864864
}
@@ -897,18 +897,18 @@ void field_real::get_opt_type(String *answer,
897897
0 : (item->decimals + 1));
898898

899899
if (min_arg >= -128 && max_arg <= (min_arg >= 0 ? 255 : 127))
900-
sprintf(buff, "TINYINT(%d)", len);
900+
snprintf(buff, sizeof(buff), "TINYINT(%d)", len);
901901
else if (min_arg >= INT_MIN16 && max_arg <= (min_arg >= 0 ?
902902
UINT_MAX16 : INT_MAX16))
903-
sprintf(buff, "SMALLINT(%d)", len);
903+
snprintf(buff, sizeof(buff), "SMALLINT(%d)", len);
904904
else if (min_arg >= INT_MIN24 && max_arg <= (min_arg >= 0 ?
905905
UINT_MAX24 : INT_MAX24))
906-
sprintf(buff, "MEDIUMINT(%d)", len);
906+
snprintf(buff, sizeof(buff), "MEDIUMINT(%d)", len);
907907
else if (min_arg >= INT_MIN32 && max_arg <= (min_arg >= 0 ?
908908
UINT_MAX32 : INT_MAX32))
909-
sprintf(buff, "INT(%d)", len);
909+
snprintf(buff, sizeof(buff), "INT(%d)", len);
910910
else
911-
sprintf(buff, "BIGINT(%d)", len);
911+
snprintf(buff, sizeof(buff), "BIGINT(%d)", len);
912912
answer->append(buff, (uint) strlen(buff));
913913
if (min_arg >= 0)
914914
answer->append(STRING_WITH_LEN(" UNSIGNED"));
@@ -923,10 +923,10 @@ void field_real::get_opt_type(String *answer,
923923
else
924924
{
925925
if (min_arg >= -FLT_MAX && max_arg <= FLT_MAX)
926-
sprintf(buff, "FLOAT(%d,%d)", (int) max_length - (item->decimals + 1) + max_notzero_dec_len,
926+
snprintf(buff, sizeof(buff), "FLOAT(%d,%d)", (int) max_length - (item->decimals + 1) + max_notzero_dec_len,
927927
max_notzero_dec_len);
928928
else
929-
sprintf(buff, "DOUBLE(%d,%d)", (int) max_length - (item->decimals + 1) + max_notzero_dec_len,
929+
snprintf(buff, sizeof(buff), "DOUBLE(%d,%d)", (int) max_length - (item->decimals + 1) + max_notzero_dec_len,
930930
max_notzero_dec_len);
931931
answer->append(buff, (uint) strlen(buff));
932932
}
@@ -945,18 +945,18 @@ void field_longlong::get_opt_type(String *answer,
945945
char buff[MAX_FIELD_WIDTH];
946946

947947
if (min_arg >= -128 && max_arg <= (min_arg >= 0 ? 255 : 127))
948-
sprintf(buff, "TINYINT(%d)", (int) max_length);
948+
snprintf(buff, sizeof(buff), "TINYINT(%d)", (int) max_length);
949949
else if (min_arg >= INT_MIN16 && max_arg <= (min_arg >= 0 ?
950950
UINT_MAX16 : INT_MAX16))
951-
sprintf(buff, "SMALLINT(%d)", (int) max_length);
951+
snprintf(buff, sizeof(buff), "SMALLINT(%d)", (int) max_length);
952952
else if (min_arg >= INT_MIN24 && max_arg <= (min_arg >= 0 ?
953953
UINT_MAX24 : INT_MAX24))
954-
sprintf(buff, "MEDIUMINT(%d)", (int) max_length);
954+
snprintf(buff, sizeof(buff), "MEDIUMINT(%d)", (int) max_length);
955955
else if (min_arg >= INT_MIN32 && max_arg <= (min_arg >= 0 ?
956956
UINT_MAX32 : INT_MAX32))
957-
sprintf(buff, "INT(%d)", (int) max_length);
957+
snprintf(buff, sizeof(buff), "INT(%d)", (int) max_length);
958958
else
959-
sprintf(buff, "BIGINT(%d)", (int) max_length);
959+
snprintf(buff, sizeof(buff), "BIGINT(%d)", (int) max_length);
960960
answer->append(buff, (uint) strlen(buff));
961961
if (min_arg >= 0)
962962
answer->append(STRING_WITH_LEN(" UNSIGNED"));
@@ -976,15 +976,15 @@ void field_ulonglong::get_opt_type(String *answer,
976976
char buff[MAX_FIELD_WIDTH];
977977

978978
if (max_arg < 256)
979-
sprintf(buff, "TINYINT(%d) UNSIGNED", (int) max_length);
979+
snprintf(buff, sizeof(buff), "TINYINT(%d) UNSIGNED", (int) max_length);
980980
else if (max_arg <= ((2 * INT_MAX16) + 1))
981-
sprintf(buff, "SMALLINT(%d) UNSIGNED", (int) max_length);
981+
snprintf(buff, sizeof(buff), "SMALLINT(%d) UNSIGNED", (int) max_length);
982982
else if (max_arg <= ((2 * INT_MAX24) + 1))
983-
sprintf(buff, "MEDIUMINT(%d) UNSIGNED", (int) max_length);
983+
snprintf(buff, sizeof(buff), "MEDIUMINT(%d) UNSIGNED", (int) max_length);
984984
else if (max_arg < (((ulonglong) 1) << 32))
985-
sprintf(buff, "INT(%d) UNSIGNED", (int) max_length);
985+
snprintf(buff, sizeof(buff), "INT(%d) UNSIGNED", (int) max_length);
986986
else
987-
sprintf(buff, "BIGINT(%d) UNSIGNED", (int) max_length);
987+
snprintf(buff, sizeof(buff), "BIGINT(%d) UNSIGNED", (int) max_length);
988988
// if item is FIELD_ITEM, it _must_be_ Field_num in this class
989989
answer->append(buff, (uint) strlen(buff));
990990
if (item->type() == Item::FIELD_ITEM &&
@@ -1005,7 +1005,7 @@ void field_decimal::get_opt_type(String *answer,
10051005
my_decimal_set_zero(&zero);
10061006
my_bool is_unsigned= (my_decimal_cmp(&zero, &min_arg) >= 0);
10071007

1008-
length= sprintf(buff, "DECIMAL(%d, %d)",
1008+
length= snprintf(buff, sizeof(buff), "DECIMAL(%d, %d)",
10091009
(int) (max_length - (item->decimals ? 1 : 0)),
10101010
item->decimals);
10111011
if (is_unsigned)

‎sql/sql_repl.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4032,7 +4032,7 @@ bool mysql_show_binlog_events(THD* thd)
40324032
binlog_size= s.st_size;
40334033
if (lex_mi->pos > binlog_size)
40344034
{
4035-
sprintf(errmsg_buf, "Invalid pos specified. Requested from pos:%llu is "
4035+
snprintf(errmsg_buf, sizeof(errmsg_buf), "Invalid pos specified. Requested from pos:%llu is "
40364036
"greater than actual file size:%lu\n", lex_mi->pos,
40374037
(ulong)s.st_size);
40384038
errmsg= errmsg_buf;

‎storage/connect/bsonudf.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1144,7 +1144,7 @@ my_bool BJNX::LocateArray(PGLOBAL g, PBVAL jarp)
11441144

11451145
for (int i = 0; i < n && !Found; i++) {
11461146
Jp->N = m;
1147-
sprintf(s, "[%d]", i + B);
1147+
snprintf(s, sizeof(s), "[%d]", i + B);
11481148

11491149
if (Jp->WriteStr(s))
11501150
return true;
@@ -1438,7 +1438,7 @@ my_bool BJNX::AddPath(void)
14381438

14391439
for (int i = 0; i <= I; i++) {
14401440
if (Jpnp[i].Type == TYPE_JAR) {
1441-
sprintf(s, "[%d]", Jpnp[i].N + B);
1441+
snprintf(s, sizeof(s), "[%d]", Jpnp[i].N + B);
14421442

14431443
if (Jp->WriteStr(s))
14441444
return true;

‎storage/connect/global.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
#include <time.h> /* time_t type declaration */
1515
#include <setjmp.h> /* Long jump declarations */
1616

17+
#define ROUNDUP_TO_8(num) (((num + 7) / 8) * 8)
18+
1719
#if defined(_WIN32) && !defined(NOEX)
1820
#define DllExport __declspec( dllexport )
1921
#else // !_WIN32

‎storage/connect/jdbconn.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -451,8 +451,14 @@ PQRYRES JDBCSrcCols(PGLOBAL g, PCSZ src, PJPARM sjp)
451451

452452
if (strstr(src, "%s")) {
453453
// Place holder for an eventual where clause
454-
sqry = (char*)PlugSubAlloc(g, NULL, strlen(src) + 2);
455-
sprintf(sqry, src, "1=1"); // dummy where clause
454+
size_t sqry_size = strlen(src) + 2;
455+
sqry = (char*)PlugSubAlloc(g, NULL, sqry_size);
456+
// Function PlugSubAlloc(...) recalculate string size
457+
// while allocate memory - it rounds size up size to multiple of 8
458+
// we need to know the real allocated size
459+
// to use it in sprintf(...)
460+
const int sqry_real_allocated_size = ROUNDUP_TO_8(sqry_size);
461+
snprintf(sqry, sqry_real_allocated_size, src, "1=1"); // dummy where clause
456462
} else
457463
sqry = (char*)src;
458464

‎storage/connect/json.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1023,13 +1023,13 @@ bool JDOC::SerializeValue(PJVAL jvp)
10231023
case TYPE_DTM:
10241024
return js->Escape(jvp->Strp);
10251025
case TYPE_INTG:
1026-
sprintf(buf, "%d", jvp->N);
1026+
snprintf(buf, sizeof(buf), "%d", jvp->N);
10271027
return js->WriteStr(buf);
10281028
case TYPE_BINT:
1029-
sprintf(buf, "%lld", jvp->LLn);
1029+
snprintf(buf, sizeof(buf), "%lld", jvp->LLn);
10301030
return js->WriteStr(buf);
10311031
case TYPE_DBL: // dfp to limit to the default number of decimals
1032-
sprintf(buf, "%.*f", MY_MIN(jvp->Nd, dfp), jvp->F);
1032+
snprintf(buf, sizeof(buf), "%.*f", MY_MIN(jvp->Nd, dfp), jvp->F);
10331033
return js->WriteStr(buf);
10341034
case TYPE_NULL:
10351035
return js->WriteStr("null");

‎storage/connect/jsonudf.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -908,7 +908,7 @@ my_bool JSNX::LocateArray(PGLOBAL g, PJAR jarp)
908908

909909
for (int i = 0; i < jarp->size() && !Found; i++) {
910910
Jp->N = m;
911-
sprintf(s, "[%d]", i + B);
911+
snprintf(s, sizeof(s), "[%d]", i + B);
912912

913913
if (Jp->WriteStr(s))
914914
return true;
@@ -1189,7 +1189,7 @@ my_bool JSNX::AddPath(void) {
11891189

11901190
for (int i = 0; i <= I; i++) {
11911191
if (Jpnp[i].Type == TYPE_JAR) {
1192-
sprintf(s, "[%d]", Jpnp[i].N + B);
1192+
snprintf(s, sizeof(s), "[%d]", Jpnp[i].N + B);
11931193

11941194
if (Jp->WriteStr(s))
11951195
return true;

‎storage/connect/plugutil.cpp

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -371,13 +371,13 @@ char *PlugReadMessage(PGLOBAL g, int mid, char *m)
371371
PlugSetPath(msgfile, NULL, buff, msg_path);
372372

373373
if (!(mfile = fopen(msgfile, "rt"))) {
374-
sprintf(stmsg, "Fail to open message file %s", msgfile);
374+
snprintf(stmsg, sizeof(stmsg), "Fail to open message file %s", msgfile);
375375
goto err;
376376
} // endif mfile
377377

378378
for (;;)
379379
if (!fgets(buff, 256, mfile)) {
380-
sprintf(stmsg, "Cannot get message %d %s", mid, SVP(m));
380+
snprintf(stmsg, sizeof(stmsg), "Cannot get message %d %s", mid, SVP(m));
381381
goto fin;
382382
} else
383383
if (atoi(buff) == mid)
@@ -386,7 +386,7 @@ char *PlugReadMessage(PGLOBAL g, int mid, char *m)
386386
if (sscanf(buff, " %*d %.31s \"%.255[^\"]", msgid, stmsg) < 2) {
387387
// Old message file
388388
if (!sscanf(buff, " %*d \"%.255[^\"]", stmsg)) {
389-
sprintf(stmsg, "Bad message file for %d %s", mid, SVP(m));
389+
snprintf(stmsg, sizeof(stmsg), "Bad message file for %d %s", mid, SVP(m));
390390
goto fin;
391391
} else
392392
m = NULL;
@@ -425,17 +425,18 @@ char *PlugGetMessage(PGLOBAL g, int mid)
425425

426426
if (n == 0) {
427427
DWORD rc = GetLastError();
428-
msg = (char*)PlugSubAlloc(g, NULL, 512); // Extend buf allocation
429-
n = sprintf(msg, "Message %d, rc=%d: ", mid, rc);
428+
const int BUF_SIZE= 512;
429+
msg = (char*)PlugSubAlloc(g, NULL, BUF_SIZE); // Extend buf allocation
430+
n = snprintf(msg, BUF_SIZE, "Message %d, rc=%d: ", mid, rc);
430431
FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM |
431432
FORMAT_MESSAGE_IGNORE_INSERTS, NULL, rc, 0,
432-
(LPTSTR)(msg + n), 512 - n, NULL);
433+
(LPTSTR)(msg + n), BUF_SIZE - n, NULL);
433434
return msg;
434435
} // endif n
435436

436437
#else // ALL
437438
if (!GetRcString(mid, stmsg, 200))
438-
sprintf(stmsg, "Message %d not found", mid);
439+
snprintf(stmsg, sizeof(stmsg) "Message %d not found", mid);
439440
#endif // ALL
440441

441442
if (g) {
@@ -564,7 +565,7 @@ void *PlugSubAlloc(PGLOBAL g, void *memp, size_t size)
564565
/*******************************************************************/
565566
memp = g->Sarea;
566567

567-
size = ((size + 7) / 8) * 8; /* Round up size to multiple of 8 */
568+
size = ROUNDUP_TO_8(size); /* Round up size to multiple of 8 */
568569
pph = (PPOOLHEADER)memp;
569570

570571
if (trace(16))

‎storage/connect/tabbson.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -477,7 +477,7 @@ bool BSONDISC::Find(PGLOBAL g, PBVAL jvp, PCSZ key, int j)
477477
n = sizeof(fmt) - (strlen(fmt) + 1);
478478

479479
if (!tdp->Xcol || stricmp(tdp->Xcol, key)) {
480-
sprintf(buf, "%d", k);
480+
snprintf(buf, sizeof(buf), "%d", k);
481481

482482
if (tdp->Uri) {
483483
strncat(strncat(fmt, sep, n), buf, n - strlen(sep));
@@ -798,7 +798,7 @@ void BCUTIL::SetJsonValue(PGLOBAL g, PVAL vp, PBVAL jvp)
798798

799799
break;
800800
default:
801-
sprintf(G->Message, "Unsupported column type %d", vp->GetType());
801+
snprintf(G->Message, sizeof(G->Message), "Unsupported column type %d", vp->GetType());
802802
throw 888;
803803
} // endswitch Type
804804

0 commit comments

Comments
 (0)