From a07a71432e9580d01246021614e77c08bfa5d417 Mon Sep 17 00:00:00 2001 From: Kaavyashree2 Date: Wed, 4 Nov 2026 15:14:03 +0530 Subject: [PATCH 1/9] Hardening ISSUETYPE handling by sanitizing user input --- scripts/uploadRRDLogs.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/uploadRRDLogs.sh b/scripts/uploadRRDLogs.sh index ff463bdc1..6e7d95514 100644 --- a/scripts/uploadRRDLogs.sh +++ b/scripts/uploadRRDLogs.sh @@ -34,7 +34,7 @@ OUTFILE='/tmp/DCMSettings.conf' MAC=`getMacAddressOnly` TIMESTAMP=`date "+%Y-%m-%d-%H-%M-%S%p"` RRD_LOG_FILE="$LOG_PATH/remote-debugger.log" -ISSUETYPE=`echo $2 | tr '[a-z]' '[A-Z]'` +ISSUETYPE=$(printf '%s' "$2" | tr '[:lower:]' '[:upper:]' | tr -cd 'A-Z0-9_-') RRD_LOG_PATH="$1" RRD_LOG_DIR="/tmp/rrd/" UPLOAD_DEBUG_FILE="${MAC}_${ISSUETYPE}_${TIMESTAMP}_RRD_DEBUG_LOGS.tgz" From 20e5834d98a8ef95a2c3b529fce46311e2023947 Mon Sep 17 00:00:00 2001 From: Kaavyashree2 Date: Wed, 4 Nov 2026 15:18:22 +0530 Subject: [PATCH 2/9] Test Addition for finder type conversion --- src/unittest/rrdUnitTestRunner.cpp | 33 +++++++++++++++++------------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/src/unittest/rrdUnitTestRunner.cpp b/src/unittest/rrdUnitTestRunner.cpp index 6a70307c7..bbf6c64c8 100644 --- a/src/unittest/rrdUnitTestRunner.cpp +++ b/src/unittest/rrdUnitTestRunner.cpp @@ -3263,41 +3263,46 @@ TEST(FinderTest, KeyTypeStrExpectTypeMatch) EXPECT_STREQ(result->via.str.ptr, "value"); } -TEST(FinderTest, KeyTypeStrValTypeStrMatch) +TEST(FinderTest, KeyTypeStrValTypeAnyMatch) { msgpack_object_map map; msgpack_object_kv kv; kv.key.type = MSGPACK_OBJECT_STR; kv.key.via.str.ptr = "name"; kv.key.via.str.size = strlen("name"); - kv.val.type = MSGPACK_OBJECT_STR; - kv.val.via.str.ptr = "value"; - kv.val.via.str.size = strlen("value"); + kv.val.type = MSGPACK_OBJECT_NIL; map.size = 1; map.ptr = &kv; msgpack_object *result = __finder("name", MSGPACK_OBJECT_NIL, &map); ASSERT_NE(result, nullptr); - EXPECT_EQ(result->type, MSGPACK_OBJECT_STR); - EXPECT_STREQ(result->via.str.ptr, "value"); + EXPECT_EQ(result->type, MSGPACK_OBJECT_NIL); } -TEST(FinderTest, KeyTypeStrValTypeAnyMatch) +TEST(FinderTest, NameMatchesButTypeMismatchReturnsNull) { msgpack_object_map map; msgpack_object_kv kv; + kv.key.type = MSGPACK_OBJECT_STR; - kv.key.via.str.ptr = "name"; - kv.key.via.str.size = strlen("name"); - kv.val.type = MSGPACK_OBJECT_NIL; + kv.key.via.str.ptr = "version"; + kv.key.via.str.size = strlen("version"); + + kv.val.type = MSGPACK_OBJECT_STR; + kv.val.via.str.ptr = "abc"; + kv.val.via.str.size = strlen("abc"); + map.size = 1; map.ptr = &kv; - msgpack_object *result = __finder("name", MSGPACK_OBJECT_NIL, &map); - ASSERT_NE(result, nullptr); - EXPECT_EQ(result->type, MSGPACK_OBJECT_NIL); -} + msgpack_object *result = + __finder("version", + MSGPACK_OBJECT_POSITIVE_INTEGER, + &map); + EXPECT_EQ(result, nullptr); + EXPECT_EQ(errno, MISSING_ENTRY); +} /* --------------- Test get_msgpack_unpack_status() from rrdMsgPackDecoder --------------- */ TEST(GetMsgpackUnpackStatusTest, DecodedbufIsNullAndSizeIsZero) { From 3eef682ba5d90bf261a93f4192f6b2a8d8724a5d Mon Sep 17 00:00:00 2001 From: Kaavyashree2 Date: Wed, 4 Nov 2026 15:24:45 +0530 Subject: [PATCH 3/9] Preventing INT overflow and resolving type mismatch --- src/rrdMsgPackDecoder.c | 47 +++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/src/rrdMsgPackDecoder.c b/src/rrdMsgPackDecoder.c index e851eaa1f..3cc080430 100644 --- a/src/rrdMsgPackDecoder.c +++ b/src/rrdMsgPackDecoder.c @@ -23,6 +23,8 @@ #include #include #include +#include +#include #include "rrdMsgPackDecoder.h" #include "rrdCommon.h" @@ -44,29 +46,15 @@ msgpack_object *__finder(const char *name, { if (MSGPACK_OBJECT_STR == map->ptr[i].key.type) { - if (expect_type == map->ptr[i].val.type) + if (expect_type == map->ptr[i].val.type) && + (0 == match(&(map->ptr[i]), name)) { - if (0 == match(&(map->ptr[i]), name)) - { - return &map->ptr[i].val; - } - } - else if (MSGPACK_OBJECT_STR == map->ptr[i].val.type) - { - if (0 == strncmp(map->ptr[i].key.via.str.ptr, name, strlen(name))) - { return &map->ptr[i].val; - } } - else - { - if (0 == strncmp(map->ptr[i].key.via.str.ptr, name, strlen(name))) - { - return &map->ptr[i].val; - } - } - } + + } } + errno = MISSING_ENTRY; return NULL; } @@ -136,9 +124,9 @@ void *helper_convert(const void *buf, size_t len, else if (NULL != wrapper && 0 != strcmp(wrapper, "parameters")) { inner = __finder(wrapper, expect_type, &msg.data.via.map); - subdoc_name = __finder("subdoc_name", expect_type, &msg.data.via.map); - version = __finder("version", expect_type, &msg.data.via.map); - transaction_id = __finder("transaction_id", expect_type, &msg.data.via.map); + subdoc_name = __finder("subdoc_name", MSGPACK_OBJECT_STR, &msg.data.via.map); + version = __finder("version", MSGPACK_OBJECT_POSITIVE_INTEGER, &msg.data.via.map); + transaction_id = __finder("transaction_id", MSGPACK_OBJECT_POSITIVE_INTEGER, &msg.data.via.map); if (((NULL != inner) && (0 == (process)(p, 4, inner, subdoc_name, version, transaction_id))) || ((true == optional) && (NULL == inner))) { @@ -298,16 +286,25 @@ const char *remotedebuggerdoc_strerror(int errnum) * @param msgpack_object_array *arr - The msgpack array containing the issue types. * @return int - The required size of the issue type string. */ -int getReqTotalSizeOfIssueTypesStr(msgpack_object_array *arr) +size_t getReqTotalSizeOfIssueTypesStr(msgpack_object_array *arr) { - int index = 0, size = 0; + size_t index = 0; + size_t size = 0; for(index = 0; index < arr->size; index++) { if(arr->ptr[index].type == MSGPACK_OBJECT_STR) - { + { + if (SIZE_MAX - size < arr->ptr[index].via.str.size) + { + return 0; + } size += arr->ptr[index].via.str.size; } } + if (SIZE_MAX - size < arr->size + 1) + { + return 0; + } return (size + arr->size + 1); /* Need to add Number of Required "," between each string*/ } From c6a518ef20354dbd40ebdc32bd085c7f463886d9 Mon Sep 17 00:00:00 2001 From: Kaavyashree2 Date: Wed, 4 Nov 2026 15:44:56 +0530 Subject: [PATCH 4/9] Update rrdMsgPackDecoder.c --- src/rrdMsgPackDecoder.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/rrdMsgPackDecoder.c b/src/rrdMsgPackDecoder.c index 3cc080430..8a4e3f4a4 100644 --- a/src/rrdMsgPackDecoder.c +++ b/src/rrdMsgPackDecoder.c @@ -319,7 +319,8 @@ int process_remotedebuggerparams(remotedebuggerparam_t *rrdParam, msgpack_object { int left = map->size; uint8_t objects_left = 1; - int index = 0, resultLen = 0; + int index = 0; + size_t resultLen = 0; msgpack_object_kv *p = NULL; msgpack_object_array *arr = NULL; p = map->ptr; @@ -333,7 +334,11 @@ int process_remotedebuggerparams(remotedebuggerparam_t *rrdParam, msgpack_object { arr = &p->val.via.array; resultLen = getReqTotalSizeOfIssueTypesStr(arr); - RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: Size:%d \n", __FUNCTION__, __LINE__,resultLen); + if (resultLen == 0) { + RDK_LOG(RDK_LOG_ERROR, LOG_REMDEBUG, "[%s:%d]: Invalid IssueType size\n", __FUNCTION__, __LINE__); + return -1; + } + RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: Size:%zu \n", __FUNCTION__, __LINE__,resultLen); rrdParam->length = resultLen; rrdParam->commandList = (char *)calloc(resultLen + 1, sizeof(char)); if(rrdParam->commandList != NULL) From 1df6825e7f3b85003199a12f654a3d7993a37a21 Mon Sep 17 00:00:00 2001 From: Kaavyashree2 Date: Wed, 4 Nov 2026 15:51:44 +0530 Subject: [PATCH 5/9] Syntax error fix --- src/rrdMsgPackDecoder.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rrdMsgPackDecoder.c b/src/rrdMsgPackDecoder.c index 8a4e3f4a4..9a2058c65 100644 --- a/src/rrdMsgPackDecoder.c +++ b/src/rrdMsgPackDecoder.c @@ -46,8 +46,8 @@ msgpack_object *__finder(const char *name, { if (MSGPACK_OBJECT_STR == map->ptr[i].key.type) { - if (expect_type == map->ptr[i].val.type) && - (0 == match(&(map->ptr[i]), name)) + if ((expect_type == map->ptr[i].val.type) && + (0 == match(&(map->ptr[i]), name))) { return &map->ptr[i].val; } From 8961fdbbfa5ee18015389c83b12e1725e0e0d581 Mon Sep 17 00:00:00 2001 From: Kaavyashree2 Date: Fri, 6 Nov 2026 14:38:57 +0530 Subject: [PATCH 6/9] Testcase addition for INT overflow --- src/unittest/rrdUnitTestRunner.cpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/unittest/rrdUnitTestRunner.cpp b/src/unittest/rrdUnitTestRunner.cpp index bbf6c64c8..394f7324c 100644 --- a/src/unittest/rrdUnitTestRunner.cpp +++ b/src/unittest/rrdUnitTestRunner.cpp @@ -3404,6 +3404,21 @@ TEST(GetReqTotalSizeOfIssueTypesStrTest, ArraySizeFiveWithStringTypes) EXPECT_EQ(getReqTotalSizeOfIssueTypesStr(&arr), 1 + 5 + total_strlen); } +TEST(GetReqTotalSizeOfIssueTypesStrTest, DetectsSizeOverflow) +{ + msgpack_object obj_array[2]; + + obj_array[0].type = MSGPACK_OBJECT_STR; + obj_array[0].via.str.size = SIZE_MAX - 1; + + obj_array[1].type = MSGPACK_OBJECT_STR; + obj_array[1].via.str.size = 10; + + msgpack_object_array arr = {2, obj_array}; + + EXPECT_EQ(getReqTotalSizeOfIssueTypesStr(&arr), 0); +} + /* --------------- Test FreeResources_RemoteDebugger() from rrdMsgPackDecoder --------------- */ TEST(FreeResourcesRemoteDebuggerTest, NullPointerArgument) { From 6f0c680f36d6b99e1a871994fe1fcf7a794f7a94 Mon Sep 17 00:00:00 2001 From: Kaavyashree2 Date: Fri, 6 Nov 2026 14:56:56 +0530 Subject: [PATCH 7/9] Adding log statements --- src/unittest/rrdUnitTestRunner.cpp | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/unittest/rrdUnitTestRunner.cpp b/src/unittest/rrdUnitTestRunner.cpp index 394f7324c..33f0dda45 100644 --- a/src/unittest/rrdUnitTestRunner.cpp +++ b/src/unittest/rrdUnitTestRunner.cpp @@ -3414,9 +3414,18 @@ TEST(GetReqTotalSizeOfIssueTypesStrTest, DetectsSizeOverflow) obj_array[1].type = MSGPACK_OBJECT_STR; obj_array[1].via.str.size = 10; + printf("SIZE_MAX=%zu\n", (size_t)SIZE_MAX); + printf("obj0 size=%zu\n", (size_t)obj_array[0].via.str.size); + printf("obj1 size=%zu\n", (size_t)obj_array[1].via.str.size); + msgpack_object_array arr = {2, obj_array}; - EXPECT_EQ(getReqTotalSizeOfIssueTypesStr(&arr), 0); + size_t result = getReqTotalSizeOfIssueTypesStr(&arr); + + printf("result=%zu\n", result); + + + EXPECT_EQ(result, 0u); } /* --------------- Test FreeResources_RemoteDebugger() from rrdMsgPackDecoder --------------- */ From 1fd7fd31126bd905528f3eff1b3c1aed2fb463a1 Mon Sep 17 00:00:00 2001 From: Kaavyashree2 Date: Fri, 6 Nov 2026 14:59:40 +0530 Subject: [PATCH 8/9] Addition of debug statements --- src/rrdMsgPackDecoder.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/rrdMsgPackDecoder.c b/src/rrdMsgPackDecoder.c index 9a2058c65..767f8545e 100644 --- a/src/rrdMsgPackDecoder.c +++ b/src/rrdMsgPackDecoder.c @@ -294,17 +294,30 @@ size_t getReqTotalSizeOfIssueTypesStr(msgpack_object_array *arr) { if(arr->ptr[index].type == MSGPACK_OBJECT_STR) { + printf("index=%zu size=%zu curr=%zu\n", + index, + size, + (size_t)arr->ptr[index].via.str.size); + if (SIZE_MAX - size < arr->ptr[index].via.str.size) { + printf("OVERFLOW CHECK HIT\n"); return 0; } size += arr->ptr[index].via.str.size; + printf("after add size=%zu\n", size); } } + printf("before final check size=%zu arrsize=%zu\n", + size, + (size_t)arr->size); if (SIZE_MAX - size < arr->size + 1) { + printf("FINAL OVERFLOW CHECK HIT\n"); return 0; } + printf("returning=%zu\n", (size + arr->size + 1)); + return (size + arr->size + 1); /* Need to add Number of Required "," between each string*/ } From 20704dacf0feb5c467a292c2f4ab501efb26c855 Mon Sep 17 00:00:00 2001 From: Kaavyashree2 Date: Fri, 6 Nov 2026 15:09:57 +0530 Subject: [PATCH 9/9] Removing testcase due to impracticality --- src/unittest/rrdUnitTestRunner.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/unittest/rrdUnitTestRunner.cpp b/src/unittest/rrdUnitTestRunner.cpp index 33f0dda45..9b9ed149e 100644 --- a/src/unittest/rrdUnitTestRunner.cpp +++ b/src/unittest/rrdUnitTestRunner.cpp @@ -3404,6 +3404,8 @@ TEST(GetReqTotalSizeOfIssueTypesStrTest, ArraySizeFiveWithStringTypes) EXPECT_EQ(getReqTotalSizeOfIssueTypesStr(&arr), 1 + 5 + total_strlen); } +/* + TEST(GetReqTotalSizeOfIssueTypesStrTest, DetectsSizeOverflow) { msgpack_object obj_array[2]; @@ -3427,6 +3429,7 @@ TEST(GetReqTotalSizeOfIssueTypesStrTest, DetectsSizeOverflow) EXPECT_EQ(result, 0u); } +*/ /* --------------- Test FreeResources_RemoteDebugger() from rrdMsgPackDecoder --------------- */ TEST(FreeResourcesRemoteDebuggerTest, NullPointerArgument)