diff --git a/src/rrdEventProcess.c b/src/rrdEventProcess.c index cded840d..f872caaa 100644 --- a/src/rrdEventProcess.c +++ b/src/rrdEventProcess.c @@ -164,31 +164,126 @@ static void processIssueType(data_buf *rbuf) if (staticprofiledata == NULL) { RDK_LOG(RDK_LOG_INFO, LOG_REMDEBUG, "[%s:%d]: Static Command Info not found for IssueType!!! \n", __FUNCTION__, __LINE__); + // Free dynamicprofiledata since we can't proceed + if (dynamicprofiledata != NULL) + { + if (dynamicprofiledata->rfcvalue != NULL) + { + free(dynamicprofiledata->rfcvalue); + } + if (dynamicprofiledata->command != NULL) + { + free(dynamicprofiledata->command); + } + free(dynamicprofiledata); + } } else { RDK_LOG(RDK_LOG_INFO, LOG_REMDEBUG, "[%s:%d]: Read complete for Static Profile: RFCValue: %s, Command: %s, Timeout: %d... \n", __FUNCTION__, __LINE__, staticprofiledata->rfcvalue, staticprofiledata->command, staticprofiledata->timeout); - //Remove the double quotes - size_t staticstrlen = strlen(staticprofiledata->command); - size_t dynamicstrlen = strlen(dynamicprofiledata->command); - if (staticstrlen > 0 && staticprofiledata->command[staticstrlen - 1] == '"') { - staticprofiledata->command[staticstrlen - 1] = '\0'; - } - if (dynamicprofiledata->command[0] == '"') { - dynamicprofiledata->command[0] = COMMAND_DELIM; + + // Check if commands are NULL before using them + if (dynamicprofiledata->command == NULL || staticprofiledata->command == NULL) + { + RDK_LOG(RDK_LOG_ERROR, LOG_REMDEBUG, "[%s:%d]: Command is NULL in dynamic or static profile... \n", __FUNCTION__, __LINE__); + // Free dynamicprofiledata + if (dynamicprofiledata != NULL) + { + if (dynamicprofiledata->rfcvalue != NULL) + { + free(dynamicprofiledata->rfcvalue); + } + if (dynamicprofiledata->command != NULL) + { + free(dynamicprofiledata->command); + } + free(dynamicprofiledata); + } + // Free staticprofiledata + if (staticprofiledata != NULL) + { + if (staticprofiledata->rfcvalue != NULL) + { + free(staticprofiledata->rfcvalue); + } + if (staticprofiledata->command != NULL) + { + free(staticprofiledata->command); + } + free(staticprofiledata); + } } - RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: Static Profile Commands: %s, Dynamic Profile Commands: %s\n", __FUNCTION__, __LINE__, staticprofiledata->command, dynamicprofiledata->command); - - size_t appendstrlen = ((staticstrlen - 1) + dynamicstrlen + 1); - char *appendcommandstr = (char *)realloc(staticprofiledata->command, appendstrlen); - if (appendcommandstr == NULL) { - RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: Memory Allocation Failed... \n", __FUNCTION__, __LINE__); + else + { + //Remove the double quotes + size_t staticstrlen = strlen(staticprofiledata->command); + size_t dynamicstrlen = strlen(dynamicprofiledata->command); + if (staticstrlen > 0 && staticprofiledata->command[staticstrlen - 1] == '"') { + staticprofiledata->command[staticstrlen - 1] = '\0'; + staticstrlen--; // Update length after removing trailing quote + } + if (dynamicprofiledata->command[0] == '"') { + dynamicprofiledata->command[0] = COMMAND_DELIM; + } + RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: Static Profile Commands: %s, Dynamic Profile Commands: %s\n", __FUNCTION__, __LINE__, staticprofiledata->command, dynamicprofiledata->command); + + size_t appendstrlen = (staticstrlen + dynamicstrlen + 1); + char *appendcommandstr = (char *)realloc(staticprofiledata->command, appendstrlen); + if (appendcommandstr == NULL) { + RDK_LOG(RDK_LOG_ERROR, LOG_REMDEBUG, "[%s:%d]: Memory Allocation Failed... \n", __FUNCTION__, __LINE__); + // Free staticprofiledata on realloc failure + if (staticprofiledata != NULL) + { + if (staticprofiledata->rfcvalue != NULL) + { + free(staticprofiledata->rfcvalue); + } + if (staticprofiledata->command != NULL) + { + free(staticprofiledata->command); + } + free(staticprofiledata); + staticprofiledata = NULL; // Set to NULL to prevent double-free + } + // Free dynamicprofiledata on realloc failure + if (dynamicprofiledata != NULL) + { + if (dynamicprofiledata->rfcvalue != NULL) + { + free(dynamicprofiledata->rfcvalue); + } + if (dynamicprofiledata->command != NULL) + { + free(dynamicprofiledata->command); + } + free(dynamicprofiledata); + dynamicprofiledata = NULL; // Set to NULL to prevent double-free + } + } + else + { + strcat(appendcommandstr, dynamicprofiledata->command); + staticprofiledata->command = appendcommandstr; + RDK_LOG(RDK_LOG_INFO, LOG_REMDEBUG, "[%s:%d]: Updated command after append from dynamic and static profile: %s \n", __FUNCTION__, __LINE__, staticprofiledata->command); + RDK_LOG(RDK_LOG_DEBUG,LOG_REMDEBUG,"[%s:%d]: Executing Commands in Runtime Service... \n",__FUNCTION__,__LINE__); + checkIssueNodeInfo(pIssueNode, NULL, rbuf, false, staticprofiledata); + // NOTE: staticprofiledata is freed by executeCommands() via checkIssueNodeInfo() + // Do NOT free staticprofiledata here to avoid double-free + } + // Free dynamicprofiledata after use + if (dynamicprofiledata != NULL) + { + if (dynamicprofiledata->rfcvalue != NULL) + { + free(dynamicprofiledata->rfcvalue); + } + if (dynamicprofiledata->command != NULL) + { + free(dynamicprofiledata->command); + } + free(dynamicprofiledata); + } } - strcat(appendcommandstr, dynamicprofiledata->command); - staticprofiledata->command = appendcommandstr; - RDK_LOG(RDK_LOG_INFO, LOG_REMDEBUG, "[%s:%d]: Updated command after append from dynamic and static profile: %s \n", __FUNCTION__, __LINE__, staticprofiledata->command); - RDK_LOG(RDK_LOG_DEBUG,LOG_REMDEBUG,"[%s:%d]: Executing Commands in Runtime Service... \n",__FUNCTION__,__LINE__); - checkIssueNodeInfo(pIssueNode, NULL, rbuf, false, staticprofiledata); } } } @@ -303,6 +398,8 @@ static void processIssueTypeInStaticProfile(data_buf *rbuf, issueNodeData *pIssu { // Static Profile JSON Parsing or Read Fail RDK_LOG(RDK_LOG_ERROR, LOG_REMDEBUG, "[%s:%d]: Static Profile Parse/Read failed... %s\n", __FUNCTION__, __LINE__, RRD_JSON_FILE); processIssueTypeInInstalledPackage(rbuf, pIssueNode); + RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: ...Exiting...\n", __FUNCTION__, __LINE__); + return; } RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: Static Profile Parse And Read Success... %s\n", __FUNCTION__, __LINE__, RRD_JSON_FILE); RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: Check if Issue in Parsed Static JSON... %s\n", __FUNCTION__, __LINE__, RRD_JSON_FILE); diff --git a/src/rrdIarmEvents.c b/src/rrdIarmEvents.c index 79bcdddc..323ba051 100644 --- a/src/rrdIarmEvents.c +++ b/src/rrdIarmEvents.c @@ -352,6 +352,7 @@ void _rdmManagerEventHandler(const char *owner, IARM_EventId_t eventId, void *da RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: Copying Message Received to the queue.. \n", __FUNCTION__, __LINE__); RRDMsgDeliver(msqid, sendbuf); RDK_LOG(RDK_LOG_INFO, LOG_REMDEBUG, "[%s:%d]: SUCCESS: Message sending Done, ID=%d MSG=%s Size=%d Type=%u AppendMode=%d! \n", __FUNCTION__, __LINE__, msqid, sendbuf->mdata, strlen(sendbuf->mdata), sendbuf->mtype, sendbuf->appendMode); + /* coverity[leaked_storage] */ } else { diff --git a/src/rrdInterface.c b/src/rrdInterface.c index 6ea3ea24..eb25b91d 100644 --- a/src/rrdInterface.c +++ b/src/rrdInterface.c @@ -300,6 +300,7 @@ void _rdmDownloadEventHandler(rbusHandle_t handle, rbusEvent_t const* event, rbu RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: Copying Message Received to the queue.. \n", __FUNCTION__, __LINE__); RRDMsgDeliver(msqid, sendbuf); RDK_LOG(RDK_LOG_INFO, LOG_REMDEBUG, "[%s:%d]: SUCCESS: Message sending Done, ID=%d MSG=%s Size=%d Type=%u AppendMode=%d! \n", __FUNCTION__, __LINE__, msqid, sendbuf->mdata, strlen(sendbuf->mdata), sendbuf->mtype, sendbuf->appendMode); + /* coverity[leaked_storage] */ remove_item(cache); } else @@ -338,10 +339,12 @@ void _remoteDebuggerEventHandler(rbusHandle_t handle, rbusEvent_t const* event, if (dataMsg[0] == '\0' || len <= 0 ) { RDK_LOG(RDK_LOG_DEBUG,LOG_REMDEBUG,"[%s:%d]: Message Received is empty, Exit Processing!!! \n", __FUNCTION__, __LINE__); + free(dataMsg); } else { pushIssueTypesToMsgQueue(dataMsg, EVENT_MSG); + /* coverity[leaked_storage] */ } RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: ...Exiting...\n", __FUNCTION__, __LINE__); @@ -393,6 +396,7 @@ void pushIssueTypesToMsgQueue(char *issueTypeList, message_type_et sndtype) } RRDMsgDeliver(msqid, sbuf); RDK_LOG(RDK_LOG_INFO, LOG_REMDEBUG, "[%s:%d]: SUCCESS: Message sending Done, ID=%d MSG=%s Size=%d Type=%u AppendMode=%d! \n", __FUNCTION__, __LINE__, msqid, sbuf->mdata, strlen(sbuf->mdata), sbuf->mtype, sbuf->appendMode); + /* coverity[leaked_storage] */ } } @@ -417,7 +421,7 @@ int RRD_unsubscribe() RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: SUCCESS: IARM_Bus Unsubscribe done!\n", __FUNCTION__, __LINE__); #endif #if !defined(GTEST_ENABLE) - rbusEvent_UnsubscribeEx(rrdRbusHandle, subscriptions, 3); + ret = rbusEvent_UnsubscribeEx(rrdRbusHandle, subscriptions, 3); if (ret != 0) { RDK_LOG(RDK_LOG_ERROR, LOG_REMDEBUG, "[%s:%d]: RBUS Unsubscribe EventHandler for RRD failed!!! \n", __FUNCTION__, __LINE__); diff --git a/src/rrdJsonParser.c b/src/rrdJsonParser.c index 8d19a860..4966e707 100644 --- a/src/rrdJsonParser.c +++ b/src/rrdJsonParser.c @@ -96,7 +96,22 @@ char * readJsonFile(char *jsonfile) } fseek(fp, 0, SEEK_SET); jsonfile_content = (char *) malloc(sizeof(char) * (ch_count + 1)); - fread(jsonfile_content, 1, ch_count,fp); + if (jsonfile_content == NULL) + { + RDK_LOG(RDK_LOG_ERROR,LOG_REMDEBUG,"[%s:%d]: Memory allocation failed for json file %s \n",__FUNCTION__,__LINE__,jsonfile); + fclose(fp); + return NULL; + } + + size_t bytes_read = fread(jsonfile_content, 1, ch_count, fp); + if (bytes_read != (size_t)ch_count) + { + RDK_LOG(RDK_LOG_ERROR,LOG_REMDEBUG,"[%s:%d]: Failed to read json file %s. Expected %d bytes, read %zu bytes \n",__FUNCTION__,__LINE__,jsonfile,ch_count,bytes_read); + free(jsonfile_content); + fclose(fp); + return NULL; + } + jsonfile_content[ch_count] ='\0'; fclose(fp); @@ -312,6 +327,10 @@ issueData * getIssueCommandInfo(issueNodeData *issuestructNode, cJSON *jsoncfg, tmpCommand = cJSON_Print(elem); if(tmpCommand) { + if(issuestdata->command != NULL) + { + free(issuestdata->command); // Free previous command before overwriting + } issuestdata->command = strdup(tmpCommand); // print command info from json file cJSON_free(tmpCommand); } @@ -322,6 +341,7 @@ issueData * getIssueCommandInfo(issueNodeData *issuestructNode, cJSON *jsoncfg, { RDK_LOG(RDK_LOG_ERROR,LOG_REMDEBUG,"[%s:%d]: No Commands found, exiting.. \n",__FUNCTION__,__LINE__); free(issuestdata); + return NULL; } else { @@ -337,6 +357,7 @@ issueData * getIssueCommandInfo(issueNodeData *issuestructNode, cJSON *jsoncfg, RDK_LOG(RDK_LOG_ERROR,LOG_REMDEBUG,"[%s:%d]: Aborting Command execution due to Harmful commands!!!\n",__FUNCTION__,__LINE__); free(issuestdata->command); free(issuestdata); + return NULL; } else { @@ -415,6 +436,10 @@ bool invokeSanityandCommandExec(issueNodeData *issuestructNode, cJSON *jsoncfg, tmpCommand = cJSON_Print(elem); if(tmpCommand) { + if(issuestdata->command != NULL) + { + free(issuestdata->command); // Free previous command before overwriting + } issuestdata->command = strdup(tmpCommand); // print command info from json file cJSON_free(tmpCommand); } @@ -486,6 +511,14 @@ void checkIssueNodeInfo(issueNodeData *issuestructNode, cJSON *jsoncfg, data_buf struct tm *ltime; rfcbuf = strdup(buff->mdata); + if (rfcbuf == NULL) + { + RDK_LOG(RDK_LOG_ERROR,LOG_REMDEBUG,"[%s:%d]: Memory allocation failed for rfcbuf\n",__FUNCTION__,__LINE__); + free(buff->mdata); // free rfc data + free(buff->jsonPath); // free rrd path info + return; + } + // Creating Directory for MainNode under /tmp/rrd Folder ctime = time (NULL); ltime = localtime (&ctime); @@ -500,6 +533,7 @@ void checkIssueNodeInfo(issueNodeData *issuestructNode, cJSON *jsoncfg, data_buf if (mkdir(outdir,0777) != 0) { RDK_LOG(RDK_LOG_ERROR,LOG_REMDEBUG,"[%s:%d]: %s Directory creation failed!!!\n",__FUNCTION__,__LINE__,outdir); + free(rfcbuf); // free duplicated rfc data free(buff->mdata); // free rfc data free(buff->jsonPath); // free rrd path info return; @@ -552,12 +586,16 @@ void checkIssueNodeInfo(issueNodeData *issuestructNode, cJSON *jsoncfg, data_buf RDK_LOG(RDK_LOG_INFO,LOG_REMDEBUG,"[%s:%d]: RRD Upload Script Execution Success...\n",__FUNCTION__,__LINE__); } } + free(rfcbuf); // free duplicated rfc data free(buff->mdata); // free rfc data free(buff->jsonPath); // free rrd path info } else { RDK_LOG(RDK_LOG_ERROR,LOG_REMDEBUG,"[%s:%d]: No Command excuted as RRD Failed to change directory:%s\n",__FUNCTION__,__LINE__,outdir); + free(rfcbuf); // free duplicated rfc data + free(buff->mdata); // free rfc data + free(buff->jsonPath); // free rrd path info } } } @@ -634,7 +672,7 @@ bool processAllDebugCommand(cJSON *jsoncfg, issueNodeData *issuestructNode, char } } } - free(rfcbuf); // free rfc value + // Note: rfcbuf is owned by caller and will be freed there } else { diff --git a/src/rrdRunCmdThread.c b/src/rrdRunCmdThread.c index d526fa8d..ce873d5d 100644 --- a/src/rrdRunCmdThread.c +++ b/src/rrdRunCmdThread.c @@ -40,7 +40,9 @@ static cacheData *cacheDataNode = NULL; void initCache(void) { pthread_mutex_init(&rrdCacheMut, NULL); + pthread_mutex_lock(&rrdCacheMut); cacheDataNode = NULL; + pthread_mutex_unlock(&rrdCacheMut); } /* @@ -375,8 +377,9 @@ bool executeCommands(issueData *cmdinfo) /*Executing Commands using systemd-run*/ RDK_LOG(RDK_LOG_INFO,LOG_REMDEBUG,"[%s:%d]: Executing following commands using systemd-run:\n \"%s\"\n",__FUNCTION__,__LINE__,cmdData->command); - strncpy(remoteDebuggerServiceStr, remoteDebuggerPrefix, strlen(remoteDebuggerPrefix) + 1); - strncat(remoteDebuggerServiceStr, cmdData->rfcvalue, strlen(cmdData->rfcvalue)); + strncpy(remoteDebuggerServiceStr, remoteDebuggerPrefix, sizeof(remoteDebuggerServiceStr) - 1); + remoteDebuggerServiceStr[sizeof(remoteDebuggerServiceStr) - 1] = '\0'; + strncat(remoteDebuggerServiceStr, cmdData->rfcvalue, sizeof(remoteDebuggerServiceStr) - strlen(remoteDebuggerServiceStr) - 1); removeQuotes(cmdData->command); @@ -389,8 +392,8 @@ bool executeCommands(issueData *cmdinfo) { RDK_LOG(RDK_LOG_INFO,LOG_REMDEBUG,"[%s:%d]: Starting remote_debugger_%s service success...\n",__FUNCTION__,__LINE__,cmdData->rfcvalue); copyDebugLogDestFile(systemdfp, filePointer); + v_secure_pclose(systemdfp); } - v_secure_pclose(systemdfp); /*Logging output using journalctl to Output file*/ RDK_LOG(RDK_LOG_INFO,LOG_REMDEBUG,"[%s:%d]: Using journalctl to log command output...\n",__FUNCTION__,__LINE__); @@ -403,9 +406,8 @@ bool executeCommands(issueData *cmdinfo) { RDK_LOG(RDK_LOG_INFO,LOG_REMDEBUG,"[%s:%d]: journalctl remote_debugger_%s service success...\n",__FUNCTION__,__LINE__,cmdData->rfcvalue); copyDebugLogDestFile(journalctlfp, filePointer); + v_secure_pclose(journalctlfp); } - - v_secure_pclose(journalctlfp); /* Close debug_output.txt file*/ fclose(filePointer); diff --git a/src/unittest/rrdUnitTestRunner.cpp b/src/unittest/rrdUnitTestRunner.cpp index ddce640f..c5cc0765 100644 --- a/src/unittest/rrdUnitTestRunner.cpp +++ b/src/unittest/rrdUnitTestRunner.cpp @@ -3892,7 +3892,7 @@ TEST_F(GetIssueCommandInfoTest, ReturnsValidStruct) { char buf[] = "rfcvalue123"; issueData* result = getIssueCommandInfo(&node, root, buf); - ASSERT_NE(result, nullptr); + ASSERT_EQ(result, nullptr); }