Skip to content

Commit cff07b7

Browse files
committed
Fixes warnings for possible string truncation with strncpy()
All of these fixes handles strncpy(), and the compiler warning have been silenced by checking the length before copying the string, we check with '>=' which means that the string must be one smaller and therefore have room for at least one NULL terminating character, if this fails the function provides a error message and/or return. Some of the buffer lengths had a '+ 1' appended, some of these have been removed. I'm also using sizeof() operator, in case we later want to change the buffer size. Most of the previous solutions also checked for NULL terminated strings but the compiler wasn't satisfied with it happening *after* the command to strncpy().
1 parent fb4d6a1 commit cff07b7

7 files changed

Lines changed: 45 additions & 39 deletions

File tree

src/emc/rs274ngc/interp_o_word.cc

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -541,7 +541,7 @@ int Interp::control_back_to( block_pointer block, // pointer to block
541541
setup_pointer settings) // pointer to machine settings
542542
{
543543
static char name[] = "control_back_to";
544-
char newFileName[PATH_MAX+1];
544+
char newFileName[PATH_MAX];
545545
FILE *newFP;
546546
offset_map_iterator it;
547547
offset_pointer op;
@@ -561,12 +561,12 @@ int Interp::control_back_to( block_pointer block, // pointer to block
561561
newFP = fopen(op->filename, "r");
562562
// set the line number
563563
settings->sequence_number = 0;
564-
strncpy(settings->filename, op->filename, sizeof(settings->filename));
565-
if (settings->filename[sizeof(settings->filename)-1] != '\0') {
564+
if (strlen(op->filename) >= sizeof(settings->filename)) {
566565
fclose(settings->file_pointer);
567566
logOword("filename too long: %s", op->filename);
568567
ERS(NCE_UNABLE_TO_OPEN_FILE, op->filename);
569568
}
569+
strncpy(settings->filename, op->filename, sizeof(settings->filename));
570570

571571
if (newFP) {
572572
// close the old file...
@@ -597,11 +597,11 @@ int Interp::control_back_to( block_pointer block, // pointer to block
597597
if (settings->file_pointer)
598598
fclose(settings->file_pointer);
599599
settings->file_pointer = newFP;
600-
strncpy(settings->filename, newFileName, sizeof(settings->filename));
601-
if (settings->filename[sizeof(settings->filename)-1] != '\0') {
600+
if (strlen(newFileName) >= sizeof(settings->filename)) {
602601
logOword("new filename '%s' is too long (max len %zu)\n", newFileName, sizeof(settings->filename)-1);
603-
settings->filename[sizeof(settings->filename)-1] = '\0'; // oh well, truncate the filename
602+
ERS(NCE_UNABLE_TO_OPEN_FILE, newFileName);
604603
}
604+
strncpy(settings->filename, newFileName, sizeof(settings->filename));
605605
} else {
606606
char *dirname = getcwd(NULL, 0);
607607
logOword("fopen: |%s| failed CWD:|%s|", newFileName,

src/emc/rs274ngc/interp_read.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1595,8 +1595,10 @@ int Interp::read_o( /* ARGUMENTS */
15951595

15961596
// Subroutine name not provided in Fanuc syntax, so pull from
15971597
// context
1598+
if (strlen(_setup.sub_context[_setup.call_level].subName) >= sizeof(oNameBuf))
1599+
ERS(NCE_UNABLE_TO_OPEN_FILE, _setup.sub_context[_setup.call_level].subName);
15981600
strncpy(oNameBuf, _setup.sub_context[_setup.call_level].subName,
1599-
LINELEN+1);
1601+
sizeof(oNameBuf));
16001602
} else
16011603
// any other m-code should have been handled by read_m()
16021604
OERR(_("%d: Bug: Non-m98/m99 M-code passed to read_o(): '%s'"),

src/emc/rs274ngc/rs274ngc_pre.cc

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2492,12 +2492,12 @@ int Interp::ini_load(const char *filename)
24922492

24932493
char parameter_file_name[LINELEN]={};
24942494
if (NULL != (inistring = inifile.Find("PARAMETER_FILE", "RS274NGC"))) {
2495-
strncpy(parameter_file_name, inistring, LINELEN);
2496-
2497-
if (parameter_file_name[LINELEN-1] != '\0') {
2498-
logDebug("%s:[RS274NGC]PARAMETER_FILE is too long (max len %d)", filename, LINELEN-1);
2495+
if (strlen(inistring) >= sizeof(parameter_file_name)) {
2496+
logDebug("%s:[RS274NGC]PARAMETER_FILE is too long (max len %zu)",
2497+
filename, sizeof(parameter_file_name)-1);
24992498
} else {
2500-
logDebug("found PARAMETER_FILE:%s:", parameter_file_name);
2499+
strncpy(parameter_file_name, inistring, sizeof(parameter_file_name));
2500+
logDebug("found PARAMETER_FILE:%s:", parameter_file_name);
25012501
}
25022502
} else {
25032503
// not found, leave RS274NGC_PARAMETER_FILE alone

src/emc/task/emctask.cc

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ static void user_defined_add_m_code(int num, double arg1, double arg2)
112112

113113
int emcTaskInit()
114114
{
115-
char mdir[MAX_M_DIRS][PATH_MAX+1];
115+
char mdir[MAX_M_DIRS][PATH_MAX];
116116
int num,dct,dmax;
117117
char path[EMC_SYSTEM_CMD_LEN];
118118
struct stat buf;
@@ -123,18 +123,14 @@ int emcTaskInit()
123123

124124
// Identify user_defined_function directories
125125
if (NULL != (inistring = inifile.Find("PROGRAM_PREFIX", "DISPLAY"))) {
126-
strncpy(mdir[0],inistring, sizeof(mdir[0]));
127-
if (mdir[0][sizeof(mdir[0])-1] != '\0') {
126+
if (strlen(inistring) >= sizeof(mdir[0])) {
128127
rcs_print("[DISPLAY]PROGRAM_PREFIX too long (max len %zu)\n", sizeof(mdir[0]));
129128
return -1;
130129
}
130+
strncpy(mdir[0], inistring, sizeof(mdir[0]));
131131
} else {
132132
// default dir if no PROGRAM_PREFIX
133-
strncpy(mdir[0],"nc_files", sizeof(mdir[0]));
134-
if (mdir[0][sizeof(mdir[0])-1] != '\0') {
135-
rcs_print("default nc_files too long (max len %zu)\n", sizeof(mdir[0]));
136-
return -1;
137-
}
133+
strncpy(mdir[0], "nc_files", sizeof(mdir[0]));
138134
}
139135
dmax = 1; //one directory mdir[0], USER_M_PATH specifies additional dirs
140136

@@ -146,21 +142,22 @@ int emcTaskInit()
146142

147143
for (dct=1; dct < MAX_M_DIRS; dct++) mdir[dct][0] = 0;
148144

149-
strncpy(tmpdirs,inistring, sizeof(tmpdirs));
150-
if (tmpdirs[sizeof(tmpdirs)-1] != '\0') {
145+
if (strlen(inistring) >= sizeof(tmpdirs)) {
151146
rcs_print("[RS274NGC]USER_M_PATH too long (max len %zu)\n", sizeof(tmpdirs));
152147
return -1;
153148
}
149+
strncpy(tmpdirs, inistring, sizeof(tmpdirs));
154150

155151
nextdir = strtok(tmpdirs,":"); // first token
156152
dct = 1;
157153
while (dct < MAX_M_DIRS) {
158154
if (nextdir == NULL) break; // no more tokens
159-
strncpy(mdir[dct],nextdir, sizeof(mdir[dct]));
160-
if (mdir[dct][sizeof(mdir[dct])-1] != '\0') {
161-
rcs_print("[RS274NGC]USER_M_PATH component (%s) too long (max len %zu)\n", nextdir, sizeof(mdir[dct]));
155+
if (strlen(nextdir) >= sizeof(mdir[dct])) {
156+
rcs_print("[RS274NGC]USER_M_PATH component (%s) too long (max len %zu)\n",
157+
nextdir, sizeof(mdir[dct]));
162158
return -1;
163159
}
160+
strncpy(mdir[dct], nextdir, sizeof(mdir[dct]));
164161
nextdir = strtok(NULL,":");
165162
dct++;
166163
}

src/emc/usr_intf/emcrsh.cc

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -609,18 +609,12 @@ static int commandHello(connectionRecType *context)
609609
if (strcmp(pch, pwd) != 0) return -1;
610610

611611
pch = strtok(NULL, delims);
612-
if (pch == NULL) return -1;
612+
if (pch == NULL || strlen(pch) >= sizeof(context->hostName)) return -1;
613613
strncpy(context->hostName, pch, sizeof(context->hostName));
614-
if (context->hostName[sizeof(context->hostName)-1] != '\0') {
615-
return -1;
616-
}
617614

618615
pch = strtok(NULL, delims);
619-
if (pch == NULL) return -1;
616+
if (pch == NULL|| strlen(pch) >= sizeof(context->version)) return -1;
620617
strncpy(context->version, pch, sizeof(context->version));
621-
if (context->version[sizeof(context->version)-1] != '\0') {
622-
return -1;
623-
}
624618

625619
context->linked = true;
626620
printf("Connected to %s\n", context->hostName);
@@ -1169,12 +1163,12 @@ static cmdResponseType setOpen(char *s, connectionRecType *context)
11691163

11701164
pch = strtok(NULL, "\n\r\0");
11711165
if (pch == NULL) return rtStandardError;
1172-
1173-
strncpy(context->progName, pch, sizeof(context->progName));
1174-
if (context->progName[sizeof(context->progName) - 1] != '\0') {
1175-
fprintf(stderr, "linuxcncrsh: 'set open' filename too long for context (got %lu bytes, max %lu)", (unsigned long)strlen(pch), (unsigned long)sizeof(context->progName));
1166+
if (strlen(pch) >= sizeof(context->progName)) {
1167+
fprintf(stderr, "linuxcncrsh: 'set open' filename too long for context (got %lu bytes, max %lu)",
1168+
(unsigned long)strlen(pch), (unsigned long)sizeof(context->progName));
11761169
return rtStandardError;
11771170
}
1171+
strncpy(context->progName, pch, sizeof(context->progName));
11781172

11791173
if (sendProgramOpen(context->progName) != 0) return rtStandardError;
11801174
return rtNoError;

src/hal/user_comps/mb2hal/mb2hal_init.c

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -696,7 +696,11 @@ retCode init_mb_links()
696696
this_mb_link->lp_link_type = this_mb_tx->cfg_link_type;
697697

698698
if (this_mb_link->lp_link_type == linkRTU) { //serial
699-
strncpy(this_mb_link->lp_serial_device, this_mb_tx->cfg_serial_device, MB2HAL_MAX_DEVICE_LENGTH-1);
699+
if (strlen(this_mb_tx->cfg_serial_device) >= MB2HAL_MAX_DEVICE_LENGTH) {
700+
ERR(gbl.init_dbg, "serial_device name to long [%s]", this_mb_tx->cfg_serial_device);
701+
return retERR;
702+
}
703+
strncpy(this_mb_link->lp_serial_device, this_mb_tx->cfg_serial_device, MB2HAL_MAX_DEVICE_LENGTH);
700704
this_mb_link->lp_serial_baud=this_mb_tx->cfg_serial_baud;
701705

702706
if (strcasecmp(this_mb_tx->cfg_serial_parity, "even") == 0) {
@@ -723,7 +727,11 @@ retCode init_mb_links()
723727
}
724728
}
725729
else { //tcp
726-
strncpy(this_mb_link->lp_tcp_ip, this_mb_tx->cfg_tcp_ip, sizeof(this_mb_tx->cfg_tcp_ip)-1);
730+
if (strlen(this_mb_tx->cfg_tcp_ip) >= sizeof(this_mb_link->lp_tcp_ip)) {
731+
ERR(gbl.init_dbg, "tcp_ip too long [%s]", this_mb_tx->cfg_tcp_ip);
732+
return retERR;
733+
}
734+
strncpy(this_mb_link->lp_tcp_ip, this_mb_tx->cfg_tcp_ip, sizeof(this_mb_tx->cfg_tcp_ip));
727735
this_mb_link->lp_tcp_port=this_mb_tx->cfg_tcp_port;
728736

729737
this_mb_link->modbus = modbus_new_tcp(this_mb_link->lp_tcp_ip, this_mb_link->lp_tcp_port);

src/libnml/cms/cms_cfg.cc

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,11 @@ int load_nml_config_file(const char *file)
110110

111111
CONFIG_FILE_INFO *info = new CONFIG_FILE_INFO();
112112
info->lines_list = new LinkedList();
113+
if (strlen(file) >= 80) {
114+
rcs_print_error("cms_config: file name too long\n");
115+
loading_config_file = 0;
116+
return -1;
117+
}
113118
strncpy(info->file_name, file, 80);
114119
FILE *fp;
115120
fp = fopen(file, "r");

0 commit comments

Comments
 (0)