appended null termination to version string

adding closure of socket in syslog_thread and read_socket

fixed missing free of filepath in compressed file transfer

fixed missing closedir in added in send_one error case

fixed missing fclose of src_file in ferror case

dlt-daemon.c fixed missing fclose in ferror case

dlt_common.c using snprintf for filename instead sprintf in dlt_file_open

modified some magix numbers - replaced by limits.h values. added strncpy instead strcpy to make cp more safe. split up a function to avoid coverity complaint when computing size of a write buffer length

dlt_daemon_common: added strncpy instead strcpy to make cp more safe.

Fixes some more Coverity issues in dlt-daemon.c dlt_user.c dlt_common.c and dlt_offline_trace.c.

Conflicts:

	src/daemon/dlt-daemon.c
	src/lib/dlt_user.c

Signed-off-by: Alexander Wenzel <Alexander.AW.Wenzel@bmw.de>

4 files changed, 62 insertions, 56 deletions

diff --git a/src/daemon/dlt-daemon.c b/src/daemon/dlt-daemon.c
index 7e9b6ba..059afe8 100644
--- a/src/daemon/dlt-daemon.c
+++ b/src/daemon/dlt-daemon.c
@@ -159,7 +159,7 @@ int option_handling(DltDaemonLocal *daemon_local,int argc, char* argv[])
         }
         case 'c':
         {
-            strncpy(daemon_local->flags.cvalue,optarg,sizeof(daemon_local->flags.cvalue));
+            strncpy(daemon_local->flags.cvalue,optarg,NAME_MAX);
             break;
         }
         case 'h':
@@ -203,9 +203,10 @@ int option_handling(DltDaemonLocal *daemon_local,int argc, char* argv[])
 int option_file_parser(DltDaemonLocal *daemon_local)
 {
 	FILE * pFile;
-	char line[1024];
-	char token[1024];
-	char value[1024];
+        int value_length = 1024;
+        char line[value_length-1];
+        char token[value_length];
+        char value[value_length];
     char *pch;
     const char *filename;
 
@@ -234,7 +235,7 @@ int option_file_parser(DltDaemonLocal *daemon_local)
 		while(1)
 		{
 			/* fetch line from configuration file */
-			if ( fgets (line , 1024 , pFile) != NULL )
+                        if ( fgets (line , value_length - 1 , pFile) != NULL )
 			{
 				  pch = strtok (line," =\r\n");
 				  token[0]=0;
@@ -247,11 +248,11 @@ int option_file_parser(DltDaemonLocal *daemon_local)
 
 					if(token[0]==0)
 					{
-						strncpy(token,pch,sizeof(token));
+                                                strncpy(token,pch,sizeof(token) - 1);
 					}
 					else
 					{
-						strncpy(value,pch,sizeof(value));
+                                                strncpy(value,pch,sizeof(value) - 1);
 						break;
 					}
 
@@ -308,22 +309,22 @@ int option_file_parser(DltDaemonLocal *daemon_local)
 						}
 						else if(strcmp(token,"RS232DeviceName")==0)
 						{
-							strncpy(daemon_local->flags.yvalue,value,sizeof(daemon_local->flags.yvalue));
+                                                        strncpy(daemon_local->flags.yvalue,value,NAME_MAX);
 							//printf("Option: %s=%s\n",token,value);
 						}
 						else if(strcmp(token,"RS232Baudrate")==0)
 						{
-							strncpy(daemon_local->flags.bvalue,value,sizeof(daemon_local->flags.bvalue));
+                                                        strncpy(daemon_local->flags.bvalue,value,NAME_MAX);
 							//printf("Option: %s=%s\n",token,value);
 						}
 						else if(strcmp(token,"ECUId")==0)
 						{
-							strncpy(daemon_local->flags.evalue,value,sizeof(daemon_local->flags.evalue));
+                                                        strncpy(daemon_local->flags.evalue,value,NAME_MAX);
 							//printf("Option: %s=%s\n",token,value);
 						}
 						else if(strcmp(token,"PersistanceStoragePath")==0)
 						{
-							strncpy(daemon_local->flags.ivalue,value,sizeof(daemon_local->flags.ivalue));
+                                                        strncpy(daemon_local->flags.ivalue,value,NAME_MAX);
 							//printf("Option: %s=%s\n",token,value);
 						}
 						else if(strcmp(token,"LoggingMode")==0)
@@ -338,7 +339,7 @@ int option_file_parser(DltDaemonLocal *daemon_local)
 						}
 						else if(strcmp(token,"LoggingFilename")==0)
 						{
-							strncpy(daemon_local->flags.loggingFilename,value,sizeof(daemon_local->flags.loggingFilename));
+                                                        strncpy(daemon_local->flags.loggingFilename,value,sizeof(daemon_local->flags.loggingFilename) - 1);
 							//printf("Option: %s=%s\n",token,value);
 						}
 						else if(strcmp(token,"SharedMemorySize")==0)
@@ -348,7 +349,7 @@ int option_file_parser(DltDaemonLocal *daemon_local)
 						}
 						else if(strcmp(token,"OfflineTraceDirectory")==0)
 						{
-							strncpy(daemon_local->flags.offlineTraceDirectory,value,sizeof(daemon_local->flags.offlineTraceDirectory));
+                                                        strncpy(daemon_local->flags.offlineTraceDirectory,value,sizeof(daemon_local->flags.offlineTraceDirectory) - 1);
 							//printf("Option: %s=%s\n",token,value);
 						}
 						else if(strcmp(token,"OfflineTraceFileSize")==0)
@@ -368,7 +369,7 @@ int option_file_parser(DltDaemonLocal *daemon_local)
 						}
 						else if(strcmp(token,"PathToECUSoftwareVersion")==0)
 						{
-							strncpy(daemon_local->flags.pathToECUSoftwareVersion,value,sizeof(daemon_local->flags.pathToECUSoftwareVersion));
+                                                        strncpy(daemon_local->flags.pathToECUSoftwareVersion,value,sizeof(daemon_local->flags.pathToECUSoftwareVersion) - 1);
 							//printf("Option: %s=%s\n",token,value);
 						}
 
@@ -995,25 +996,13 @@ void dlt_daemon_daemonize(int verbose)
     /* Open standard descriptors stdin, stdout, stderr */
     i=open("/dev/null",O_RDWR); /* open stdin */
     ret=dup(i); /* stdout */
-    if (0 > ret){
-            dlt_log(LOG_CRIT, "can't open standard descriptor stdout\n");
-            exit(-1); /* can not open */
-    }
     ret=dup(i); /* stderr */
-    if (0 > ret){
-            dlt_log(LOG_CRIT, "can't open standard descriptor stderr");
-            exit(-1); /* can not open */
-    }
 
     /* Set umask */
     umask(DLT_DAEMON_UMASK);
 
     /* Change to known directory */
     ret=chdir(DLT_USER_DIR);
-    if (0 > ret){
-            dlt_log(LOG_CRIT, "Can't change to known directory");
-            exit(-1); /* Can't change to known directory */
-    }
 
     /* Ensure single copy of daemon;
        run only one instance at a time */
@@ -1032,9 +1021,6 @@ void dlt_daemon_daemonize(int verbose)
 
     sprintf(str,"%d\n",getpid());
     bytes_written=write(lfp,str,strlen(str)); /* record pid to lockfile */
-    if (0 > bytes_written){
-            dlt_log(LOG_CRIT, "write pid to lockfile failed:");
-    }
 
     /* Catch signals */
     signal(SIGCHLD,SIG_IGN); /* ignore child */
@@ -1932,21 +1918,24 @@ int dlt_daemon_process_user_message_log(DltDaemon *daemon, DltDaemonLocal *daemo
                         if (daemon_local->flags.lflag)
                         {
                             ret=write(j,dltSerialHeader,sizeof(dltSerialHeader));
-                            if (0 > ret)
+					        if (0 > ret)
                             {
-                              dlt_log(LOG_ERR,"write(j,daemon_local->msg.headerbuffer failed\n");
+                    	          dlt_log(LOG_ERR,"write(j,dltSerialHeader failed\n");
                             }
                         }
-
-                        ret=write(j,daemon_local->msg.headerbuffer+sizeof(DltStorageHeader),daemon_local->msg.headersize-sizeof(DltStorageHeader));
-                        if (0 > ret)
-                        {
-                          dlt_log(LOG_ERR,"write(j,dltSerialHeader failed\n");
-                        }
-                        ret=write(j,daemon_local->msg.databuffer,daemon_local->msg.datasize);
-                        if (0 > ret)
-                        {
-                          dlt_log(LOG_ERR,"write(j,daemon_local->msg.databuffer failed\n");
+                        int32_t diff = daemon_local->msg.headersize-sizeof(DltStorageHeader);
+                        //extra calculation for coverity
+                        if ( 0 <= diff ){
+                                ret=write(j,daemon_local->msg.headerbuffer+sizeof(DltStorageHeader),diff);
+								if (0 > ret)
+		                        {
+        		                	dlt_log(LOG_ERR,"write(j,daemon_local->msg.headerbuffer failed\n");
+        		                }
+                                ret=write(j,daemon_local->msg.databuffer,daemon_local->msg.datasize);
+								if (0 > ret)
+		                        {
+									dlt_log(LOG_ERR,"write(j,daemon_local->msg.databuffer failed\n");
+        		                }
                         }
 
                         DLT_DAEMON_SEM_FREE();
@@ -2443,14 +2432,18 @@ void dlt_daemon_ecu_version_thread(void *ptr)
 
 		while(!feof(f))
 		{
-			char buf[DLT_DAEMON_TEXTBUFSIZE];
-			read = fread(buf, 1, DLT_DAEMON_TEXTBUFSIZE, f);
+                        char buf[DLT_DAEMON_TEXTBUFSIZE];
+                        read = fread(buf, 1, DLT_DAEMON_TEXTBUFSIZE - 1, f);
+                        buf [read] = '\0';//append null termination at end of version string. Read is definitely max: DLT_DAEMON_TEXTBUFSIZE - 1
+                        read++;//Include the appended null termination position
+
 			if(ferror(f))
 			{
 				dlt_log(LOG_ERR, "Failed to read ECU Software version file.\n");
+                fclose(f);
 				return;
 			}
-			if(bufpos + read > DLT_DAEMON_TEXTBUFSIZE)
+            if(bufpos + read > ( DLT_DAEMON_TEXTBUFSIZE - 1))
 			{
 				dlt_log(LOG_ERR, "Too long file for ecu version info.\n");
 				fclose(f);
@@ -2459,6 +2452,7 @@ void dlt_daemon_ecu_version_thread(void *ptr)
 			strncpy(version+bufpos, buf, read);
 			bufpos += read;
 		}
+		version[bufpos] = '\0';//append null termination at end of version string
 		fclose(f);
 	}
 
diff --git a/src/daemon/dlt-daemon.h b/src/daemon/dlt-daemon.h
index f595e8d..aa8c372 100755
--- a/src/daemon/dlt-daemon.h
+++ b/src/daemon/dlt-daemon.h
@@ -65,12 +65,16 @@
 #ifndef DLT_DAEMON_H

 #define DLT_DAEMON_H

 

+#include <limits.h> /* for NAME_MAX */

+

 #include "dlt_daemon_common.h"

 #include "dlt_user_shared.h"

 #include "dlt_user_shared_cfg.h"

 

+

 #include <dlt_offline_trace.h>

 

+

 /**

  * The flags of a dlt daemon.

  */

@@ -85,11 +89,11 @@ typedef struct
     int rflag;      /**< (Boolean) Send automatic get log info response during context registration */

     int mflag;      /**< (Boolean) Sync to serial header on serial connection */

     int nflag;      /**< (Boolean) Sync to serial header on all TCP connections */

-    char evalue[256];   /**< (String: ECU ID) Set ECU ID (Default: ECU1) */

-    char bvalue[256];   /**< (String: Baudrate) Serial device baudrate (Default: 115200) */

-    char yvalue[256];   /**< (String: Devicename) Additional support for serial device */

-    char ivalue[256];   /**< (String: Directory) Directory where to store the persistant configuration (Default: /tmp) */

-    char cvalue[256];   /**< (String: Directory) Filename of DLT configuration file (Default: /etc/dlt.conf) */

+    char evalue[NAME_MAX + 1];   /**< (String: ECU ID) Set ECU ID (Default: ECU1) */

+    char bvalue[NAME_MAX + 1];   /**< (String: Baudrate) Serial device baudrate (Default: 115200) */

+    char yvalue[NAME_MAX + 1];   /**< (String: Devicename) Additional support for serial device */

+    char ivalue[NAME_MAX + 1];   /**< (String: Directory) Directory where to store the persistant configuration (Default: /tmp) */

+    char cvalue[NAME_MAX + 1];   /**< (String: Directory) Filename of DLT configuration file (Default: /etc/dlt.conf) */

     int  sharedMemorySize;	   /**< (int) Size of shared memory (Default: 100000) */

     int  sendMessageTime;	   /**< (Boolean) Send periodic Message Time if client is connected (Default: 0) */

     char offlineTraceDirectory[256]; /**< (String: Directory) Store DLT messages to local directory (Default: /etc/dlt.conf) */

diff --git a/src/daemon/dlt_daemon_common.c b/src/daemon/dlt_daemon_common.c
index 7bbce7b..2f0e73a 100644
--- a/src/daemon/dlt_daemon_common.c
+++ b/src/daemon/dlt_daemon_common.c
@@ -125,6 +125,7 @@ int dlt_daemon_init(DltDaemon *daemon,const char *runtime_directory, int verbose
         return -1;
     }
 
+    int append_length = 0;
     daemon->num_contexts = 0;
     daemon->contexts = 0;
 
@@ -141,18 +142,24 @@ int dlt_daemon_init(DltDaemon *daemon,const char *runtime_directory, int verbose
     daemon->mode = DLT_USER_MODE_EXTERNAL;
 
     /* prepare filenames for configuration */
+
+    append_length = PATH_MAX - sizeof(DLT_RUNTIME_APPLICATION_CFG);
     if(runtime_directory[0])
-    	strcpy(daemon->runtime_application_cfg,runtime_directory);
+        strncpy(daemon->runtime_application_cfg,runtime_directory,append_length);
     else
     	strcpy(daemon->runtime_application_cfg,DLT_RUNTIME_DEFAULT_DIRECTORY);
     strcat(daemon->runtime_application_cfg,DLT_RUNTIME_APPLICATION_CFG);
+
+    append_length = PATH_MAX - sizeof(DLT_RUNTIME_CONTEXT_CFG);
     if(runtime_directory[0])
-    	strcpy(daemon->runtime_context_cfg,runtime_directory);
+        strncpy(daemon->runtime_context_cfg,runtime_directory,append_length);
     else
     	strcpy(daemon->runtime_context_cfg,DLT_RUNTIME_DEFAULT_DIRECTORY);
     strcat(daemon->runtime_context_cfg,DLT_RUNTIME_CONTEXT_CFG);
-    if(runtime_directory[0])
-    	strcpy(daemon->runtime_configuration,runtime_directory);
+
+    append_length = PATH_MAX - sizeof(DLT_RUNTIME_CONFIGURATION);
+    if(runtime_directory[0])    
+        strncpy(daemon->runtime_configuration,runtime_directory,append_length);
     else
     	strcpy(daemon->runtime_configuration,DLT_RUNTIME_DEFAULT_DIRECTORY);
     strcat(daemon->runtime_configuration,DLT_RUNTIME_CONFIGURATION);
diff --git a/src/daemon/dlt_daemon_common.h b/src/daemon/dlt_daemon_common.h
index b3dfbc6..8f5a36f 100644
--- a/src/daemon/dlt_daemon_common.h
+++ b/src/daemon/dlt_daemon_common.h
@@ -73,6 +73,7 @@
   \{

 */

 

+#include <limits.h>

 #include <semaphore.h>

 #include "dlt_common.h"

 #include "dlt_user.h"

@@ -137,9 +138,9 @@ typedef struct
 	int sendserialheader;          /**< 1: send serial header; 0 don't send serial header */

 	int timingpackets;              /**< 1: send continous timing packets; 0 don't send continous timing packets */

 	DltBuffer client_ringbuffer; /**< Ring-buffer for storing received logs while no client connection is available */

-	char runtime_application_cfg[256]; /**< Path and filename of persistent application configuration */

-	char runtime_context_cfg[256]; /**< Path and filename of persistent context configuration */

-	char runtime_configuration[256]; /**< Path and filename of persistent configuration */

+        char runtime_application_cfg[PATH_MAX + 1]; /**< Path and filename of persistent application configuration. Set to path max, as it specifies a full path*/

+        char runtime_context_cfg[PATH_MAX + 1]; /**< Path and filename of persistent context configuration */

+        char runtime_configuration[PATH_MAX + 1]; /**< Path and filename of persistent configuration */

     DltUserLogMode mode;	/**< Mode used for tracing: off, external, internal, both */

     char state;				/**< state for tracing: 0 = no client connected, 1 = client connected */

 } DltDaemon;