From 928d1c05a6c0b0e7547e9588bc7b720af1123ea1 Mon Sep 17 00:00:00 2001 From: azerom960 Date: Fri, 25 Sep 2026 11:56:22 -0400 Subject: [PATCH 1/3] RDKEMW-25735: Validate dynamic logging requests Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- src/include/rdk_dynamic_logger_parser.h | 45 +++++++++++++++++++ src/rdk_dynamic_logger.c | 33 +++++--------- unittests/CMakeLists.txt | 5 +++ unittests/rdkDynamicLoggerParserTest.c | 58 +++++++++++++++++++++++++ 4 files changed, 118 insertions(+), 23 deletions(-) create mode 100644 src/include/rdk_dynamic_logger_parser.h create mode 100644 unittests/rdkDynamicLoggerParserTest.c diff --git a/src/include/rdk_dynamic_logger_parser.h b/src/include/rdk_dynamic_logger_parser.h new file mode 100644 index 0000000..7ab2b97 --- /dev/null +++ b/src/include/rdk_dynamic_logger_parser.h @@ -0,0 +1,45 @@ +#ifndef RDK_DYNAMIC_LOGGER_PARSER_H +#define RDK_DYNAMIC_LOGGER_PARSER_H + +#include +#include + +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 diff --git a/src/rdk_dynamic_logger.c b/src/rdk_dynamic_logger.c index 7b9ecf7..95fba4d 100644 --- a/src/rdk_dynamic_logger.c +++ b/src/rdk_dynamic_logger.c @@ -30,6 +30,7 @@ #include #include "rdk_dynamic_logger.h" +#include "rdk_dynamic_logger_parser.h" #include "rdk_debug_priv.h" #define DL_PORT 12035 @@ -54,32 +55,18 @@ 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); } @@ -87,8 +74,6 @@ static void rdk_dyn_log_validate_component_name(const unsigned char *buf) { fprintf(stderr, "Log level change request with Invalid input (%u)\n", loggingLevel); } - - return; } void rdk_dyn_log_process_pending_request() @@ -114,7 +99,7 @@ void rdk_dyn_log_process_pending_request() 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, (struct sockaddr *)&sender_addr, &addr_len)) == -1) { fprintf(stderr,"%s recvfrom failed %s\n",__func__,strerror(errno)); return; } @@ -133,8 +118,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); + (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); } } } diff --git a/unittests/CMakeLists.txt b/unittests/CMakeLists.txt index 47563b0..717388a 100644 --- a/unittests/CMakeLists.txt +++ b/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 diff --git a/unittests/rdkDynamicLoggerParserTest.c b/unittests/rdkDynamicLoggerParserTest.c new file mode 100644 index 0000000..6da0a52 --- /dev/null +++ b/unittests/rdkDynamicLoggerParserTest.c @@ -0,0 +1,58 @@ +#include "rdk_dynamic_logger_parser.h" + +#include + +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; +} From 1604a70b71c6a0ea23cc4a5ff7d2b7c470c0398e Mon Sep 17 00:00:00 2001 From: azerom960 Date: Fri, 25 Sep 2026 23:00:30 -0400 Subject: [PATCH 2/3] RDKEMW-25735: Isolate packet framing validation --- src/rdk_dynamic_logger.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/rdk_dynamic_logger.c b/src/rdk_dynamic_logger.c index 95fba4d..43d9648 100644 --- a/src/rdk_dynamic_logger.c +++ b/src/rdk_dynamic_logger.c @@ -117,11 +117,12 @@ 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 >= 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); + if(0 == strcmp("127.0.0.1",inet_ntoa(sender_addr.sin_addr))) { + 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); + } } } } From 6ab517d885f24a61bef8c1041169a0f63349cfcd Mon Sep 17 00:00:00 2001 From: azerom960 Date: Sat, 26 Sep 2026 19:12:01 -0400 Subject: [PATCH 3/3] RDKEMW-25735: Rely on loopback socket boundary --- src/rdk_dynamic_logger.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/rdk_dynamic_logger.c b/src/rdk_dynamic_logger.c index 43d9648..d70fba0 100644 --- a/src/rdk_dynamic_logger.c +++ b/src/rdk_dynamic_logger.c @@ -79,19 +79,15 @@ static void rdk_dyn_log_validate_component_name(const unsigned char *buf, size_t 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; @@ -99,7 +95,7 @@ void rdk_dyn_log_process_pending_request() if(ret <= 0) break; - if ((numbytes=recvfrom(g_dl_socket, buf, sizeof(buf), MSG_TRUNC, (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; } @@ -117,12 +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))) { - 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); - } + 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); } } }