Explorar el Código

Ensure that strings are null terminated after strncpy().

From the strcpy(3) man page, the following warning is given:
  The strncpy() function is similar, except that at most n bytes of src
  are  copied.  Warning: If there is no null byte among the first n bytes
  of src, the string placed in dest will not be null-terminated.

The current corosync code base does not take this warning into account
when using strncpy, potentially resulting in non-null terminated strings.

Signed-off-by: Russell Bryant <russell@russellbryant.net>
Reviewed-by: Steven Dake <sdake@redhat.com>
(backported from commit a609f79f1f8d23f8e57fe2afb383bd62621545f6)
Jan Friesse hace 14 años
padre
commit
53959750a7
Se han modificado 5 ficheros con 22 adiciones y 9 borrados
  1. 5 2
      exec/logsys.c
  2. 2 1
      exec/util.c
  3. 3 0
      test/sa_error.c
  4. 6 3
      tools/corosync-notifyd.c
  5. 6 3
      tools/corosync-objctl.c

+ 5 - 2
exec/logsys.c

@@ -922,7 +922,9 @@ static void logsys_subsys_init (
 			LOGSYS_LOGGER_INIT_DONE;
 			LOGSYS_LOGGER_INIT_DONE;
 	}
 	}
 	strncpy (logsys_loggers[subsysid].subsys, subsys,
 	strncpy (logsys_loggers[subsysid].subsys, subsys,
-		LOGSYS_MAX_SUBSYS_NAMELEN);
+		sizeof (logsys_loggers[subsysid].subsys));
+	logsys_loggers[subsysid].subsys[
+		sizeof (logsys_loggers[subsysid].subsys) - 1] = '\0';
 }
 }
 
 
 /*
 /*
@@ -978,7 +980,8 @@ int _logsys_system_setup(
 			(logsys_loggers[i].init_status ==
 			(logsys_loggers[i].init_status ==
 			 LOGSYS_LOGGER_NEEDS_INIT)) {
 			 LOGSYS_LOGGER_NEEDS_INIT)) {
 				strncpy (tempsubsys, logsys_loggers[i].subsys,
 				strncpy (tempsubsys, logsys_loggers[i].subsys,
-					LOGSYS_MAX_SUBSYS_NAMELEN);
+					sizeof (tempsubsys));
+				tempsubsys[sizeof (tempsubsys) - 1] = '\0';
 				logsys_subsys_init(tempsubsys, i);
 				logsys_subsys_init(tempsubsys, i);
 		}
 		}
 	}
 	}

+ 2 - 1
exec/util.c

@@ -160,7 +160,8 @@ char *getcs_name_t (cs_name_t *name)
 }
 }
 
 
 void setcs_name_t (cs_name_t *name, char *str) {
 void setcs_name_t (cs_name_t *name, char *str) {
-	strncpy ((char *)name->value, str, CS_MAX_NAME_LENGTH);
+	strncpy ((char *)name->value, str, sizeof (name->value));
+	((char *)name->value)[sizeof (name->value) - 1] = '\0';
 	if (strlen ((char *)name->value) > CS_MAX_NAME_LENGTH) {
 	if (strlen ((char *)name->value) > CS_MAX_NAME_LENGTH) {
 		name->length = CS_MAX_NAME_LENGTH;
 		name->length = CS_MAX_NAME_LENGTH;
 	} else {
 	} else {

+ 3 - 0
test/sa_error.c

@@ -46,6 +46,9 @@ int get_sa_error(cs_error_t error, char *str, int len)
 		return -1;
 		return -1;
 	}
 	}
 	strncpy(str, sa_error_list[error], len);
 	strncpy(str, sa_error_list[error], len);
+	if (len > 0) {
+		str[len - 1] = '\0';
+	}
 	return 0;
 	return 0;
 }
 }
 
 

+ 6 - 3
tools/corosync-notifyd.c

@@ -330,7 +330,8 @@ _cs_confdb_find_object (confdb_handle_t handle,
 	char tmp_name[CS_MAX_NAME_LENGTH];
 	char tmp_name[CS_MAX_NAME_LENGTH];
 	cs_error_t res = CS_OK;
 	cs_error_t res = CS_OK;
 
 
-	strncpy (tmp_name, name_pt, CS_MAX_NAME_LENGTH);
+	strncpy (tmp_name, name_pt, sizeof (tmp_name));
+	tmp_name[sizeof (tmp_name) - 1] = '\0';
 	obj_name_pt = strtok_r(tmp_name, SEPERATOR_STR, &save_pt);
 	obj_name_pt = strtok_r(tmp_name, SEPERATOR_STR, &save_pt);
 
 
 	while (obj_name_pt != NULL) {
 	while (obj_name_pt != NULL) {
@@ -838,7 +839,8 @@ _cs_local_node_info_get(char **nodename, uint32_t *nodeid)
 		corosync_cfg_finalize(cfg_handle);
 		corosync_cfg_finalize(cfg_handle);
 		if (rc != CS_OK) {
 		if (rc != CS_OK) {
 			local_nodeid = 0;
 			local_nodeid = 0;
-			strncpy(local_nodename, "localhost", CS_MAX_NAME_LENGTH);
+			strncpy(local_nodename, "localhost", sizeof (local_nodename));
+			local_nodename[sizeof (local_nodename) - 1] = '\0';
 		} else {
 		} else {
 			gethostname(local_nodename, CS_MAX_NAME_LENGTH);
 			gethostname(local_nodename, CS_MAX_NAME_LENGTH);
 		}
 		}
@@ -1007,7 +1009,8 @@ main(int argc, char *argv[])
 				break;
 				break;
 			case 'm':
 			case 'm':
 				conf[CS_NTF_SNMP] = 1;
 				conf[CS_NTF_SNMP] = 1;
-				strncpy(snmp_manager_buf, optarg, CS_MAX_NAME_LENGTH);
+				strncpy(snmp_manager_buf, optarg, sizeof (snmp_manager_buf));
+				snmp_manager_buf[sizeof (snmp_manager_buf) - 1] = '\0';
 				snmp_manager = snmp_manager_buf;
 				snmp_manager = snmp_manager_buf;
 				break;
 				break;
 			case 'o':
 			case 'o':

+ 6 - 3
tools/corosync-objctl.c

@@ -432,7 +432,8 @@ static cs_error_t find_object (confdb_handle_t handle,
 	char tmp_name[OBJ_NAME_SIZE];
 	char tmp_name[OBJ_NAME_SIZE];
 	cs_error_t res = CS_OK;
 	cs_error_t res = CS_OK;
 
 
-	strncpy (tmp_name, name_pt, OBJ_NAME_SIZE);
+	strncpy (tmp_name, name_pt, sizeof (tmp_name));
+	tmp_name[sizeof (tmp_name) - 1] = '\0';
 	obj_name_pt = strtok_r(tmp_name, SEPERATOR_STR, &save_pt);
 	obj_name_pt = strtok_r(tmp_name, SEPERATOR_STR, &save_pt);
 
 
 	while (obj_name_pt != NULL) {
 	while (obj_name_pt != NULL) {
@@ -542,7 +543,8 @@ static void create_object(confdb_handle_t handle, char * name_pt)
 	char tmp_name[OBJ_NAME_SIZE];
 	char tmp_name[OBJ_NAME_SIZE];
 	cs_error_t res;
 	cs_error_t res;
 
 
-	strncpy (tmp_name, name_pt, OBJ_NAME_SIZE);
+	strncpy (tmp_name, name_pt, sizeof (tmp_name));
+	tmp_name[sizeof (tmp_name) - 1] = '\0';
 	obj_name_pt = strtok_r(tmp_name, SEPERATOR_STR, &save_pt);
 	obj_name_pt = strtok_r(tmp_name, SEPERATOR_STR, &save_pt);
 
 
 	while (obj_name_pt != NULL) {
 	while (obj_name_pt != NULL) {
@@ -595,7 +597,8 @@ static void create_object_key(confdb_handle_t handle, char *name_pt)
 	get_parent_name(name_pt, parent_name);
 	get_parent_name(name_pt, parent_name);
 	get_key(name_pt, key_name, key_value);
 	get_key(name_pt, key_name, key_value);
 
 
-	strncpy (tmp_name, parent_name, OBJ_NAME_SIZE);
+	strncpy (tmp_name, parent_name, sizeof (tmp_name));
+	tmp_name[sizeof (tmp_name) - 1] = '\0';
 	obj_name_pt = strtok_r(tmp_name, SEPERATOR_STR, &save_pt);
 	obj_name_pt = strtok_r(tmp_name, SEPERATOR_STR, &save_pt);
 
 
 	/*
 	/*