Fix memory leaks (#9)

frozer · web-flow · commit 7b9f5d99f5af · 2024-12-28T23:06:52.000-05:00
* Fix hex2str memory leak

* Fix uninitialized value usage

* Fix socket-server memory leak

* Fix device info uncertain behavior

* Fix event processing memory leak

* Fix uninitialized field issue

* Fix make dist issues
diff --git a/Makefile.am b/Makefile.am
@@ -3,10 +3,5 @@ SUBDIRS = liblogger libdozor dozord tools
 DISTDIR = $(top_builddir)/dist
 ACLOCAL_AMFLAGS = -I m4
 
-# Override the 'distdir' target
-#dist-hook:
-#	@mkdir -p $(DISTDIR)
-#	@mv $(top_builddir)/$(PACKAGE)-$(VERSION).tar.gz $(DISTDIR)/
-  
 clean-local:
   @echo: This will be run by "make clean"
diff --git a/configure.ac b/configure.ac
@@ -9,8 +9,6 @@ AC_PROG_CC
 
 AC_PROG_MAKE_SET
 
-
-
 # Checks for libraries.
 AX_PTHREAD
 
diff --git a/dozord/Makefile.am b/dozord/Makefile.am
@@ -2,9 +2,9 @@ AUTOMAKE_OPTS = gnu
 bin_PROGRAMS = dozord
 
 dozord_SOURCES = main.c nightshift-mqtt.c socket-server.c command.c logger.c app-config.c command.h nightshift-mqtt.h logger.h socket-server.h app-config.h
-dozord_LDADD = ../liblogger/liblogger.la ../libdozor/libdozor.la -lpthread -lmosquitto
+dozord_LDADD = $(top_builddir)/liblogger/liblogger.la $(top_builddir)/libdozor/libdozor.la -lpthread -lmosquitto
 dozord_CFLAGS = -pthread
-AM_CPPFLAGS = -I../libdozor
+dozord_CPPFLAGS = -I$(top_srcdir)/liblogger -I$(top_srcdir)/libdozor
 
 man_MANS = dozord.8
 
diff --git a/dozord/logger.c b/dozord/logger.c
@@ -19,7 +19,7 @@
 #include <string.h>
 #include <time.h>
 #include <sys/time.h>
-#include "../liblogger/liblogger.h"
+#include "liblogger.h"
 
 void getISODateTime(char* buffer, size_t bufferSize) {
     // Get current time
diff --git a/dozord/logger.h b/dozord/logger.h
@@ -17,7 +17,7 @@
 #ifndef LOGGER_H
 #define LOGGER_H
 
-#include "../liblogger/liblogger.h"
+#include "liblogger.h"
 
 void prettyLogger(LogLevel level, const char* source, const char* message);
 
diff --git a/dozord/main.c b/dozord/main.c
@@ -26,7 +26,7 @@
 #include <stdbool.h>
 #include <mosquitto.h>
 #include <signal.h>
-#include <dozor.h>
+#include "dozor.h"
 #include "command.h"
 #include "nightshift-mqtt.h"
 #include "logger.h"
diff --git a/dozord/socket-server.c b/dozord/socket-server.c
@@ -78,6 +78,8 @@ void * connectionCb(void * payload) {
     return -1;
   }
 
+  responsePayload->responseLength = 0;
+  
   // @todo handle return value, -1 - error
   connInfo->on_message(responsePayload, data, &connInfo->clientIp);
 
@@ -108,14 +110,14 @@ void * connectionCb(void * payload) {
   free(responsePayload);
   close(connInfo->sockfd);
 
-  free(connInfo);
-
   pthread_mutex_lock(&GlobaSocketLock);
   connectionWorkers[connInfo->workerId] = 0;
   pthread_mutex_unlock(&GlobaSocketLock);
-
+  
   snprintf(logMessage, sizeof(logMessage), "%s closed", connInfo->clientIp);
   prettyLogger(LOG_LEVEL_DEBUG, "TCP", logMessage);   
+  
+  free(connInfo);
 
   pthread_exit(NULL);
 }
diff --git a/libdozor/Makefile.am b/libdozor/Makefile.am
@@ -2,7 +2,8 @@ AUTOMAKE_OPTIONS = gnu
 lib_LTLIBRARIES = libdozor.la
 
 libdozor_la_SOURCES = libdozor.c device-event.c dozor-crypto.c rc4.c utils.c event.c dozor-crypto.h event.h device-event.h rc4.h session.h utils.h
-libdozor_la_LIBADD = ../liblogger/liblogger.la
+libdozor_la_LIBADD = $(top_builddir)/liblogger/liblogger.la
+libdozor_la_CPPFLAGS = -I$(top_srcdir)/liblogger
 include_HEADERS = dozor.h
 
 libdozor_la_LDFLAGS = -version-info 0:0:0
diff --git a/libdozor/device-event.c b/libdozor/device-event.c
@@ -20,7 +20,7 @@
 #include <string.h>
 #include <inttypes.h>
 #include <errno.h>
-#include "../liblogger/liblogger.h"
+#include "liblogger.h"
 #include "device-event.h"
 
 const unsigned char MSGDATASIZE[68] = { 
@@ -55,6 +55,8 @@ unsigned short int getDeviceEvents(const uint8_t * raw, long int bufSize, Device
     return 64;
   }
 
+  memset(deviceEvent, 0, sizeof(DeviceEvent));
+  
   if (bufSize == 0)
   {
     logger(LOG_LEVEL_DEBUG, "device-event(getDeviceEvents)", "No events\n");
diff --git a/libdozor/dozor-crypto.c b/libdozor/dozor-crypto.c
@@ -17,7 +17,7 @@
 
 #include <stdio.h>
 #include <inttypes.h>
-#include "../liblogger/liblogger.h"
+#include "liblogger.h"
 #include "dozor-crypto.h"
 #include "utils.h"
 #include "rc4.h"
diff --git a/libdozor/event.c b/libdozor/event.c
@@ -18,7 +18,7 @@
 #include <string.h>
 #include <stdio.h>
 #include "event.h"
-#include "../liblogger/liblogger.h"
+#include "liblogger.h"
 
 const char events[EVENT_COUNT][MAX_EVENT_NAME_LENGTH] = {
 // 0x0
@@ -195,6 +195,9 @@ void getKeepAliveEvent(EventInfo* eventInfo, uint8_t site, DeviceInfo* info)
   eventInfo->eventType = ENUM_EVENT_TYPE_KEEPALIVE;
   sprintf(eventInfo->event, "%s", res);
   eventInfo->siteId = site;
+  
+  free(temp);
+
   free(res);
 }
 
@@ -210,8 +213,11 @@ void convertDeviceEventToCommon(EventInfo* eventInfo, uint8_t site, DeviceEvent*
   char * res = malloc(sizeof(char) * 1024);
   unsigned short int index;
   char * temp = malloc(sizeof(char) * 3);
+  
   uint8_t * ptr = (uint8_t*) deviceEvent->data;
 
+  char * parsedEvent;
+
   memcpy(timestamp, getDateTime(deviceEvent->time), sizeof(char) * 25);
   timestamp[24] = 0x0;
 
@@ -235,9 +241,14 @@ void convertDeviceEventToCommon(EventInfo* eventInfo, uint8_t site, DeviceEvent*
     case 0xd:
     case 0xf:
       logger(LOG_LEVEL_DEBUG, "event.c", "handle zone event");
-      strcat(res, getZoneEventData(deviceEvent->type, deviceEvent->data, deviceEvent->dataLength));
+      parsedEvent = getZoneEventData(deviceEvent->type, deviceEvent->data, deviceEvent->dataLength);
+
+      strcat(res, parsedEvent);
       sprintf(eventInfo->sourceId, "%s", getData(deviceEvent->data, DEFAULT_DATA_POSITION, deviceEvent->dataLength));
       eventInfo->eventType = ENUM_EVENT_TYPE_ZONEINFO;
+      
+      free(parsedEvent);
+
       break;
 
     // SectionEvent
@@ -248,57 +259,92 @@ void convertDeviceEventToCommon(EventInfo* eventInfo, uint8_t site, DeviceEvent*
     case 0x35:
     case 0x37:
       logger(LOG_LEVEL_DEBUG, "event.c", "handling section event\n");
-      strcat(res, getSectionEventData(deviceEvent->type, deviceEvent->data, deviceEvent->dataLength));
+      parsedEvent = getSectionEventData(deviceEvent->type, deviceEvent->data, deviceEvent->dataLength);
+
+      strcat(res, parsedEvent);
       sprintf(eventInfo->sourceId, "%s", getData(deviceEvent->data, DEFAULT_DATA_POSITION, deviceEvent->dataLength));
       eventInfo->eventType = ENUM_EVENT_TYPE_SECTIONINFO;
+
+      free(parsedEvent);
+
       break;
 
     // AuthenticationEvent
     case 0x1b:
       logger(LOG_LEVEL_DEBUG, "event.c", "handling authentication event");
-      strcat(res, getAuthEventData(deviceEvent->type, deviceEvent->data, deviceEvent->dataLength));
+      parsedEvent = getAuthEventData(deviceEvent->type, deviceEvent->data, deviceEvent->dataLength);
+      strcat(res, parsedEvent);
       sprintf(eventInfo->sourceId, "%s", getData(deviceEvent->data, DEFAULT_DATA_POSITION, deviceEvent->dataLength));
+
+      free(parsedEvent);
+
       break;
 
     // Arm / Disarm by user
     case 0x39:
     case 0x3a:
       logger(LOG_LEVEL_DEBUG, "event.c", "handling arm/disarm event");
-      strcat(res, getSecurityEventData(deviceEvent->type, deviceEvent->data, deviceEvent->dataLength));
+      parsedEvent = getSecurityEventData(deviceEvent->type, deviceEvent->data, deviceEvent->dataLength);
+      strcat(res, parsedEvent);
       sprintf(eventInfo->sourceId, "%s", getData(deviceEvent->data, USER_DATA_POSITION, deviceEvent->dataLength));
       eventInfo->eventType = ENUM_EVENT_TYPE_ARM_DISARM;
+      
+      free(parsedEvent);
+
       break;
 
     // SecurityEvent
     case 0x19:    
     case 0x29:    
     case 0x3b:
       logger(LOG_LEVEL_DEBUG, "event.c", "handling security event");
-      strcat(res, getCommonEventData(deviceEvent->type, deviceEvent->data, deviceEvent->dataLength, SECURITY_EVENT_SCOPE));
+      parsedEvent = getCommonEventData(deviceEvent->type, deviceEvent->data, deviceEvent->dataLength, SECURITY_EVENT_SCOPE);
+      strcat(res, parsedEvent);
+
+      free(parsedEvent);
+      
       break;
 
     // ReportEvent
     case 0x25:
       logger(LOG_LEVEL_DEBUG, "event.c", "handling report event");
-      strcat(res, getReportEventData(deviceEvent->type, deviceEvent->data, deviceEvent->dataLength));
+      parsedEvent = getReportEventData(deviceEvent->type, deviceEvent->data, deviceEvent->dataLength);
+      strcat(res, parsedEvent);
+      
       eventInfo->eventType = ENUM_EVENT_TYPE_REPORT;
+      
+      free(parsedEvent);
+
       break;
 
     // Remote Command Executed
     case 0x3f:
       logger(LOG_LEVEL_DEBUG, "event.c", "handling command result event (%s...)", res);
-      strcat(res, getCommandEventData(deviceEvent->type, deviceEvent->data, deviceEvent->dataLength));
+      parsedEvent = getCommandEventData(deviceEvent->type, deviceEvent->data, deviceEvent->dataLength);
+      strcat(res, parsedEvent);
+      
       eventInfo->eventType = ENUM_EVENT_TYPE_COMMAND_RESPONSE;
+      
+      free(parsedEvent);
+
       break;
 
     case 0x1:
       logger(LOG_LEVEL_DEBUG, "event.c", "handling firmware version event");
-      strcat(res, getFirmwareVersionEventData(deviceEvent->type, deviceEvent->data, deviceEvent->dataLength));
+      parsedEvent = getFirmwareVersionEventData(deviceEvent->type, deviceEvent->data, deviceEvent->dataLength);
+      strcat(res, parsedEvent);
+
+      free(parsedEvent);
+
       break;
 
     default: 
       logger(LOG_LEVEL_DEBUG, "event.c", "handling non-specific event");
-      strcat(res, getCommonEventData(deviceEvent->type, deviceEvent->data, deviceEvent->dataLength, COMMON_EVENT_SCOPE));
+      parsedEvent = getCommonEventData(deviceEvent->type, deviceEvent->data, deviceEvent->dataLength, COMMON_EVENT_SCOPE);
+      strcat(res, parsedEvent);
+
+      free(parsedEvent);
+
       break;
   }
 
diff --git a/libdozor/libdozor.c b/libdozor/libdozor.c
@@ -22,7 +22,7 @@
 #include "utils.h"
 #include "event.h"
 #include "dozor.h"
-#include "../liblogger/liblogger.h"
+#include "liblogger.h"
 
 union rawMessage {
   struct {
@@ -75,7 +75,17 @@ Events * dozor_unpackV2(CryptoSession * crypto, uint8_t * raw, char * pinCode)
   
   ptr = (uint8_t*) packet.raw;
 
-  logger(LOG_LEVEL_DEBUG, "libdozor", "Incoming length - %d, %s", msgLength - 4, blobToHexStr(ptr, msgLength));
+  // Allocate memory for the hex string (2 characters per byte + 1 for null terminator)
+  char *hexStr = (char *)malloc(msgLength * 2 + 1);
+  if (!hexStr) {
+      return NULL;  // Return NULL if memory allocation fails
+  }
+
+  blobToHexStr(hexStr, ptr, msgLength);
+
+  logger(LOG_LEVEL_DEBUG, "libdozor", "Incoming length - %d, message length - %d, %s", msgLength, msgLength - 4, hexStr);
+
+  free(hexStr);
 
   deviceReport = malloc(sizeof(DozorReport));
   if (deviceReport == NULL)
@@ -84,6 +94,8 @@ Events * dozor_unpackV2(CryptoSession * crypto, uint8_t * raw, char * pinCode)
     events->errorCode = HANDLER_UNABLE_TO_ALLOCATE_MEMORY_REPORT;
     return events;
   }
+  
+  memset(deviceReport, 0, sizeof(DozorReport));
 
   result = getReport(deviceReport, crypto, (const unsigned char *) packet.raw, msgLength);
   if ( result < 0)
@@ -104,6 +116,9 @@ Events * dozor_unpackV2(CryptoSession * crypto, uint8_t * raw, char * pinCode)
       fprintf(stderr, "Unable to allocate memory for event info structure: %s\n", strerror(errno));
       return HANDLER_UNABLE_TO_ALLOCATE_MEMORY_REPORT;
     }
+
+    memset(eventInfo, 0, sizeof(EventInfo));
+
     getKeepAliveEvent(eventInfo, deviceReport->site, &(deviceReport->info));
     
     logger(LOG_LEVEL_DEBUG, "libdozor", "[%d] %s", eventInfo->eventType, eventInfo->event);
@@ -128,6 +143,8 @@ Events * dozor_unpackV2(CryptoSession * crypto, uint8_t * raw, char * pinCode)
       return HANDLER_UNABLE_TO_ALLOCATE_MEMORY_REPORT;
     }
 
+    memset(eventInfo, 0, sizeof(EventInfo));
+    
     convertDeviceEventToCommon(eventInfo, deviceReport->site, &(deviceReport->events[index]));
     
     logger(LOG_LEVEL_DEBUG, "libdozor", "[%d] %s", eventInfo->eventType, eventInfo->event);
diff --git a/libdozor/rc4.c b/libdozor/rc4.c
@@ -20,7 +20,7 @@
 #include <string.h>
 #include <inttypes.h>
 #include "rc4.h"
-#include "../liblogger/liblogger.h"
+#include "liblogger.h"
 
 #define POOL_SIZE 256
 
diff --git a/liblogger/Makefile.am b/liblogger/Makefile.am
@@ -5,5 +5,7 @@ lib_LTLIBRARIES = liblogger.la
 liblogger_la_SOURCES = liblogger.c liblogger.h
 liblogger_la_HEADERS = liblogger.h
 
+include_HEADERS = liblogger.h
+
 # Specify any additional flags or libraries needed
 AM_CFLAGS = -Wall -Wextra
diff --git a/liblogger/liblogger.c b/liblogger/liblogger.c
@@ -38,20 +38,12 @@ void logger(LogLevel level, const char* module, const char* format, ...) {
     printf("[%s] %s: %s\n", logLevel2Str(level), module, logMessage);
 }
 
-char * blobToHexStr(uint8_t *data, int data_length) {
-    // Allocate memory for the hex string (2 characters per byte + 1 for null terminator)
-    char *hexStr = (char *)malloc(data_length * 2 + 1);
-    if (!hexStr) {
-        return NULL;  // Return NULL if memory allocation fails
-    }
-
+void blobToHexStr(char *res, uint8_t *data, int data_length) {
     int offset = 0;
     for (int i = 0; i < data_length; i++) {
-        offset += snprintf(hexStr + offset, 3, "%02x", data[i]);
+        offset += snprintf(res + offset, 3, "%02x", data[i]);
     }
 
     // Ensure the string is null-terminated
-    hexStr[offset] = '\0';
-
-    return hexStr;
+    res[offset] = '\0';
 }
diff --git a/liblogger/liblogger.h b/liblogger/liblogger.h
@@ -17,5 +17,5 @@ void set_log_level(LogLevel level);
 LogLevel get_log_level();
 char * logLevel2Str(LogLevel level);
 void logger(LogLevel level, const char* module, const char* format, ...);
-char * blobToHexStr(uint8_t *data, int data_length);
+void blobToHexStr(char *res, uint8_t *data, int data_length);
 #endif // LIBLOGGER_H
diff --git a/tools/Makefile.am b/tools/Makefile.am
@@ -2,6 +2,5 @@ AUTOMAKE_OPTS = gnu
 bin_PROGRAMS = parser
 
 parser_SOURCES = parser.c
-parser_LDADD = ../liblogger/liblogger.la ../libdozor/libdozor.la
-
-AM_CPPFLAGS = -I../liblogger -I../libdozor
+parser_LDADD = $(top_builddir)/liblogger/liblogger.la $(top_builddir)/libdozor/libdozor.la
+parser_CPPFLAGS = -I$(top_srcdir)/liblogger -I$(top_srcdir)/libdozor
diff --git a/tools/parser.c b/tools/parser.c
@@ -20,8 +20,8 @@
 #include <string.h>
 #include <time.h>
 #include <errno.h>
-#include <dozor.h>
-#include "../liblogger/liblogger.h"
+#include "dozor.h"
+#include "liblogger.h"
 
 #define BUFFERSIZE 1024
 
