RDKEMW-25735: Validate dynamic logging requests

src/include/rdk_dynamic_logger_parser.h

@@ -0,0 +1,45 @@
+#ifndef RDK_DYNAMIC_LOGGER_PARSER_H
+#define RDK_DYNAMIC_LOGGER_PARSER_H
+
+#include <stddef.h>
+#include <string.h>
+
+static int rdk_dyn_log_parse_request(const unsigned char *buf, size_t length, const char *program, char *component, size_t component_capacity, unsigned char *log_level)
+{
+    const size_t signature_length = 4;
+    const size_t app_offset = 7;
+    size_t program_length;
+    size_t app_length;
+    size_t component_length_offset;
+    size_t component_length;
+    size_t component_offset;
+
+    if (buf == NULL || program == NULL || component == NULL || log_level == NULL || component_capacity == 0 || length < app_offset)
+        return 0;
+    if ((size_t)buf[4] + signature_length + 1 != length)
+        return 0;
+    if (memcmp(buf, "COMC", signature_length) != 0)
+        return 0;
+
+    program_length = strlen(program);
+    app_length = buf[6];
+    if (app_length != program_length || app_length > length - app_offset)
+        return 0;
+    if (memcmp(buf + app_offset, program, app_length) != 0)
+        return 0;
+
+    component_length_offset = app_offset + app_length;
+    if (component_length_offset >= length)
+        return 0;
+    component_length = buf[component_length_offset];
+    component_offset = component_length_offset + 1;
+    if (component_length >= component_capacity || component_length > length - component_offset || component_offset + component_length != length)
+        return 0;
+
+    memcpy(component, buf + component_offset, component_length);
+    component[component_length] = '\0';
+    *log_level = buf[5];
+    return 1;
+}
+
+#endif

src/rdk_dynamic_logger.c

@@ -30,6 +30,7 @@
 #include <sys/time.h>
 
 #include "rdk_dynamic_logger.h"
+#include "rdk_dynamic_logger_parser.h"
 #include "rdk_debug_priv.h"
 
 #define DL_PORT 12035
@@ -54,67 +55,47 @@ static char * rdk_dyn_log_logLevelToString(rdk_LogLevel log_level)
     return "NONE";
 }
 
-static void rdk_dyn_log_validate_component_name(const unsigned char *buf)
+static void rdk_dyn_log_validate_component_name(const unsigned char *buf, size_t length)
 {
     unsigned char log_level = 0;
-    int app_len, comp_len, i = DL_SIGNATURE_LEN;
     char comp_name[64] = {0};
+    rdk_LogLevel loggingLevel;
 
-    if(0 != memcmp(buf,DL_SIGNATURE,i)) {
+    if (!rdk_dyn_log_parse_request(buf, length, __progname, comp_name, sizeof(comp_name), &log_level))
         return;
-    }
-
-    log_level = buf[++i];
-    app_len = buf[++i];
-
-    if(0 != memcmp(buf+(++i),__progname,app_len)) {
-        /* The received msg is not intended for this process */
-        return;
-    }
-
-    i += app_len;
-    comp_len = buf[i];
-
-    rdk_LogLevel loggingLevel = (rdk_LogLevel) log_level;
 
+    loggingLevel = (rdk_LogLevel) log_level;
     if((loggingLevel >= RDK_LOG_FATAL) && (loggingLevel <= RDK_LOG_NONE))
     {
-        memcpy(comp_name,buf+(++i),comp_len);
         rdk_dbg_priv_log_reconfig(comp_name, loggingLevel);
         fprintf(stderr, "Log level change request to %s (%u) for the component %s, is success\n", rdk_dyn_log_logLevelToString(loggingLevel), loggingLevel, comp_name);
     }
     else
     {
         fprintf(stderr, "Log level change request with Invalid input (%u)\n", loggingLevel);
     }
-
-    return;
 }
 
 void rdk_dyn_log_process_pending_request()
 {
     char buf[128] = {0};
-    struct sockaddr_in sender_addr;
     struct timeval tv;
     int numbytes, ret;
-    socklen_t addr_len;
     fd_set rfds;
 
     if(-1 == g_dl_socket)
         return;
-    memset(&sender_addr,0,sizeof(sender_addr));
     while(1) {
         FD_ZERO(&rfds);
         FD_SET(g_dl_socket, &rfds);
-        addr_len = sizeof(sender_addr);
 
         tv.tv_sec = 0;
         tv.tv_usec = 0;
         ret = select(g_dl_socket+1,&rfds,NULL,NULL,&tv);
         if(ret <= 0)
             break;
 
-        if ((numbytes=recvfrom(g_dl_socket, buf, sizeof(buf), 0, (struct sockaddr *)&sender_addr, &addr_len)) == -1) {
+        if ((numbytes=recvfrom(g_dl_socket, buf, sizeof(buf), MSG_TRUNC, NULL, NULL)) == -1) {
             fprintf(stderr,"%s recvfrom failed %s\n",__func__,strerror(errno));
             return;
         }
@@ -132,9 +113,10 @@ void rdk_dyn_log_process_pending_request()
          *
          * Ensure that the we handle msgs only from localhost
          */
-        if((0 == strcmp("127.0.0.1",inet_ntoa(sender_addr.sin_addr))) &&
-                (numbytes == buf[4]+DL_SIGNATURE_LEN+1)) {
-            rdk_dyn_log_validate_component_name((const unsigned char *)buf);
+        if((numbytes >= DL_SIGNATURE_LEN + 1) &&
+                ((size_t)numbytes <= sizeof(buf)) &&
+                ((size_t)numbytes == (size_t)(unsigned char)buf[4] + DL_SIGNATURE_LEN + 1)) {
+            rdk_dyn_log_validate_component_name((const unsigned char *)buf, (size_t)numbytes);
         }
     }
 }

unittests/CMakeLists.txt

@@ -24,6 +24,11 @@ find_package(GTest REQUIRED)
 
 enable_testing()
 
+add_executable(rdk_dynamic_logger_parser_test rdkDynamicLoggerParserTest.c)
+target_include_directories(rdk_dynamic_logger_parser_test PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/../src/include)
+target_compile_options(rdk_dynamic_logger_parser_test PRIVATE -Wall -Wextra -Werror)
+add_test(NAME rdk_dynamic_logger_parser_test COMMAND rdk_dynamic_logger_parser_test)
+
 # Add the test executable
 add_executable(
   rdk_logger_gtest

unittests/rdkDynamicLoggerParserTest.c

@@ -0,0 +1,58 @@
+#include "rdk_dynamic_logger_parser.h"
+
+#include <string.h>
+
+static size_t make_request(unsigned char *buffer, size_t capacity, size_t component_length)
+{
+    const char program[] = "app";
+    size_t length = 7 + sizeof(program) - 1 + 1 + component_length;
+    if (length > capacity)
+        return 0;
+    memset(buffer, 0, capacity);
+    memcpy(buffer, "COMC", 4);
+    buffer[4] = (unsigned char)(length - 5);
+    buffer[5] = 4;
+    buffer[6] = sizeof(program) - 1;
+    memcpy(buffer + 7, program, sizeof(program) - 1);
+    buffer[10] = (unsigned char)component_length;
+    memset(buffer + 11, 'x', component_length);
+    return length;
+}
+
+int main(void)
+{
+    unsigned char request[128];
+    unsigned char level = 0;
+    char component[64];
+    size_t length = make_request(request, sizeof(request), 63);
+
+    if (!rdk_dyn_log_parse_request(request, length, "app", component, sizeof(component), &level))
+        return 1;
+    if (strlen(component) != 63 || level != 4)
+        return 1;
+    if (rdk_dyn_log_parse_request(request, length - 1, "app", component, sizeof(component), &level))
+        return 1;
+
+    component[0] = 'q';
+    request[4]--;
+    if (rdk_dyn_log_parse_request(request, length, "app", component, sizeof(component), &level) || component[0] != 'q')
+        return 1;
+    request[4]++;
+    if (rdk_dyn_log_parse_request(request, length + 1, "app", component, sizeof(component), &level))
+        return 1;
+    if (rdk_dyn_log_parse_request(request, length, "app", component, 1, &level))
+        return 1;
+    if (rdk_dyn_log_parse_request(NULL, length, "app", component, sizeof(component), &level))
+        return 1;
+
+    length = make_request(request, sizeof(request), 64);
+    if (rdk_dyn_log_parse_request(request, length, "app", component, sizeof(component), &level))
+        return 1;
+    if (rdk_dyn_log_parse_request(request, length, "other", component, sizeof(component), &level))
+        return 1;
+
+    request[6] = 127;
+    if (rdk_dyn_log_parse_request(request, length, "app", component, sizeof(component), &level))
+        return 1;
+    return 0;
+}