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" diff --git a/src/rrdMsgPackDecoder.c b/src/rrdMsgPackDecoder.c index e851eaa1f..767f8545e 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,38 @@ 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) - { + { + 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*/ } @@ -322,7 +332,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; @@ -336,7 +347,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) diff --git a/src/unittest/rrdUnitTestRunner.cpp b/src/unittest/rrdUnitTestRunner.cpp index 6a70307c7..9b9ed149e 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) { @@ -3399,6 +3404,33 @@ 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; + + 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}; + + size_t result = getReqTotalSizeOfIssueTypesStr(&arr); + + printf("result=%zu\n", result); + + + EXPECT_EQ(result, 0u); +} +*/ + /* --------------- Test FreeResources_RemoteDebugger() from rrdMsgPackDecoder --------------- */ TEST(FreeResourcesRemoteDebuggerTest, NullPointerArgument) {