Skip to content

Commit ac5f76c

Browse files
author
mtirum011
committed
RDK-60307 [RRD] RDK Coverity Defect Resolution for Device Management
1 parent c613c0b commit ac5f76c

6 files changed

Lines changed: 171 additions & 29 deletions

File tree

‎src/rrdEventProcess.c‎

Lines changed: 118 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -112,14 +112,14 @@ void processIssueTypeEvent(data_buf *rbuf)
112112
free(cmdMap[index]);
113113
cmdMap[index] = NULL;
114114
}
115-
115+
116116
}
117117
if( cmdMap)
118118
{
119119
free(cmdMap);
120120
cmdMap = NULL;
121121
}
122-
}
122+
}
123123
}
124124

125125
RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: ...Exiting...\n", __FUNCTION__, __LINE__);
@@ -164,31 +164,126 @@ static void processIssueType(data_buf *rbuf)
164164
if (staticprofiledata == NULL)
165165
{
166166
RDK_LOG(RDK_LOG_INFO, LOG_REMDEBUG, "[%s:%d]: Static Command Info not found for IssueType!!! \n", __FUNCTION__, __LINE__);
167+
// Free dynamicprofiledata since we can't proceed
168+
if (dynamicprofiledata != NULL)
169+
{
170+
if (dynamicprofiledata->rfcvalue != NULL)
171+
{
172+
free(dynamicprofiledata->rfcvalue);
173+
}
174+
if (dynamicprofiledata->command != NULL)
175+
{
176+
free(dynamicprofiledata->command);
177+
}
178+
free(dynamicprofiledata);
179+
}
167180
}
168181
else
169182
{
170183
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);
171-
//Remove the double quotes
172-
size_t staticstrlen = strlen(staticprofiledata->command);
173-
size_t dynamicstrlen = strlen(dynamicprofiledata->command);
174-
if (staticstrlen > 0 && staticprofiledata->command[staticstrlen - 1] == '"') {
175-
staticprofiledata->command[staticstrlen - 1] = '\0';
176-
}
177-
if (dynamicprofiledata->command[0] == '"') {
178-
dynamicprofiledata->command[0] = COMMAND_DELIM;
184+
185+
// Check if commands are NULL before using them
186+
if (dynamicprofiledata->command == NULL || staticprofiledata->command == NULL)
187+
{
188+
RDK_LOG(RDK_LOG_ERROR, LOG_REMDEBUG, "[%s:%d]: Command is NULL in dynamic or static profile... \n", __FUNCTION__, __LINE__);
189+
// Free dynamicprofiledata
190+
if (dynamicprofiledata != NULL)
191+
{
192+
if (dynamicprofiledata->rfcvalue != NULL)
193+
{
194+
free(dynamicprofiledata->rfcvalue);
195+
}
196+
if (dynamicprofiledata->command != NULL)
197+
{
198+
free(dynamicprofiledata->command);
199+
}
200+
free(dynamicprofiledata);
201+
}
202+
// Free staticprofiledata
203+
if (staticprofiledata != NULL)
204+
{
205+
if (staticprofiledata->rfcvalue != NULL)
206+
{
207+
free(staticprofiledata->rfcvalue);
208+
}
209+
if (staticprofiledata->command != NULL)
210+
{
211+
free(staticprofiledata->command);
212+
}
213+
free(staticprofiledata);
214+
}
179215
}
180-
RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: Static Profile Commands: %s, Dynamic Profile Commands: %s\n", __FUNCTION__, __LINE__, staticprofiledata->command, dynamicprofiledata->command);
181-
182-
size_t appendstrlen = ((staticstrlen - 1) + dynamicstrlen + 1);
183-
char *appendcommandstr = (char *)realloc(staticprofiledata->command, appendstrlen);
184-
if (appendcommandstr == NULL) {
185-
RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: Memory Allocation Failed... \n", __FUNCTION__, __LINE__);
216+
else
217+
{
218+
//Remove the double quotes
219+
size_t staticstrlen = strlen(staticprofiledata->command);
220+
size_t dynamicstrlen = strlen(dynamicprofiledata->command);
221+
if (staticstrlen > 0 && staticprofiledata->command[staticstrlen - 1] == '"') {
222+
staticprofiledata->command[staticstrlen - 1] = '\0';
223+
staticstrlen--; // Update length after removing trailing quote
224+
}
225+
if (dynamicprofiledata->command[0] == '"') {
226+
dynamicprofiledata->command[0] = COMMAND_DELIM;
227+
}
228+
RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: Static Profile Commands: %s, Dynamic Profile Commands: %s\n", __FUNCTION__, __LINE__, staticprofiledata->command, dynamicprofiledata->command);
229+
230+
size_t appendstrlen = (staticstrlen + dynamicstrlen + 1);
231+
char *appendcommandstr = (char *)realloc(staticprofiledata->command, appendstrlen);
232+
if (appendcommandstr == NULL) {
233+
RDK_LOG(RDK_LOG_ERROR, LOG_REMDEBUG, "[%s:%d]: Memory Allocation Failed... \n", __FUNCTION__, __LINE__);
234+
// Free staticprofiledata on realloc failure
235+
if (staticprofiledata != NULL)
236+
{
237+
if (staticprofiledata->rfcvalue != NULL)
238+
{
239+
free(staticprofiledata->rfcvalue);
240+
}
241+
if (staticprofiledata->command != NULL)
242+
{
243+
free(staticprofiledata->command);
244+
}
245+
free(staticprofiledata);
246+
staticprofiledata = NULL; // Set to NULL to prevent double-free
247+
}
248+
// Free dynamicprofiledata on realloc failure
249+
if (dynamicprofiledata != NULL)
250+
{
251+
if (dynamicprofiledata->rfcvalue != NULL)
252+
{
253+
free(dynamicprofiledata->rfcvalue);
254+
}
255+
if (dynamicprofiledata->command != NULL)
256+
{
257+
free(dynamicprofiledata->command);
258+
}
259+
free(dynamicprofiledata);
260+
dynamicprofiledata = NULL; // Set to NULL to prevent double-free
261+
}
262+
}
263+
else
264+
{
265+
strcat(appendcommandstr, dynamicprofiledata->command);
266+
staticprofiledata->command = appendcommandstr;
267+
RDK_LOG(RDK_LOG_INFO, LOG_REMDEBUG, "[%s:%d]: Updated command after append from dynamic and static profile: %s \n", __FUNCTION__, __LINE__, staticprofiledata->command);
268+
RDK_LOG(RDK_LOG_DEBUG,LOG_REMDEBUG,"[%s:%d]: Executing Commands in Runtime Service... \n",__FUNCTION__,__LINE__);
269+
checkIssueNodeInfo(pIssueNode, NULL, rbuf, false, staticprofiledata);
270+
// NOTE: staticprofiledata is freed by executeCommands() via checkIssueNodeInfo()
271+
// Do NOT free staticprofiledata here to avoid double-free
272+
}
273+
// Free dynamicprofiledata after use
274+
if (dynamicprofiledata != NULL)
275+
{
276+
if (dynamicprofiledata->rfcvalue != NULL)
277+
{
278+
free(dynamicprofiledata->rfcvalue);
279+
}
280+
if (dynamicprofiledata->command != NULL)
281+
{
282+
free(dynamicprofiledata->command);
283+
}
284+
free(dynamicprofiledata);
285+
}
186286
}
187-
strcat(appendcommandstr, dynamicprofiledata->command);
188-
staticprofiledata->command = appendcommandstr;
189-
RDK_LOG(RDK_LOG_INFO, LOG_REMDEBUG, "[%s:%d]: Updated command after append from dynamic and static profile: %s \n", __FUNCTION__, __LINE__, staticprofiledata->command);
190-
RDK_LOG(RDK_LOG_DEBUG,LOG_REMDEBUG,"[%s:%d]: Executing Commands in Runtime Service... \n",__FUNCTION__,__LINE__);
191-
checkIssueNodeInfo(pIssueNode, NULL, rbuf, false, staticprofiledata);
192287
}
193288
}
194289
}
@@ -303,6 +398,8 @@ static void processIssueTypeInStaticProfile(data_buf *rbuf, issueNodeData *pIssu
303398
{ // Static Profile JSON Parsing or Read Fail
304399
RDK_LOG(RDK_LOG_ERROR, LOG_REMDEBUG, "[%s:%d]: Static Profile Parse/Read failed... %s\n", __FUNCTION__, __LINE__, RRD_JSON_FILE);
305400
processIssueTypeInInstalledPackage(rbuf, pIssueNode);
401+
RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: ...Exiting...\n", __FUNCTION__, __LINE__);
402+
return;
306403
}
307404
RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: Static Profile Parse And Read Success... %s\n", __FUNCTION__, __LINE__, RRD_JSON_FILE);
308405
RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: Check if Issue in Parsed Static JSON... %s\n", __FUNCTION__, __LINE__, RRD_JSON_FILE);

‎src/rrdIarmEvents.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,7 @@ void _rdmManagerEventHandler(const char *owner, IARM_EventId_t eventId, void *da
352352
RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: Copying Message Received to the queue.. \n", __FUNCTION__, __LINE__);
353353
RRDMsgDeliver(msqid, sendbuf);
354354
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);
355+
/* coverity[leaked_storage] */
355356
}
356357
else
357358
{

‎src/rrdInterface.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,7 @@ void _rdmDownloadEventHandler(rbusHandle_t handle, rbusEvent_t const* event, rbu
300300
RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: Copying Message Received to the queue.. \n", __FUNCTION__, __LINE__);
301301
RRDMsgDeliver(msqid, sendbuf);
302302
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);
303+
/* coverity[leaked_storage] */
303304
remove_item(cache);
304305
}
305306
else
@@ -338,10 +339,12 @@ void _remoteDebuggerEventHandler(rbusHandle_t handle, rbusEvent_t const* event,
338339
if (dataMsg[0] == '\0' || len <= 0 )
339340
{
340341
RDK_LOG(RDK_LOG_DEBUG,LOG_REMDEBUG,"[%s:%d]: Message Received is empty, Exit Processing!!! \n", __FUNCTION__, __LINE__);
342+
free(dataMsg);
341343
}
342344
else
343345
{
344346
pushIssueTypesToMsgQueue(dataMsg, EVENT_MSG);
347+
/* coverity[leaked_storage] */
345348
}
346349

347350
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)
393396
}
394397
RRDMsgDeliver(msqid, sbuf);
395398
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);
399+
/* coverity[leaked_storage] */
396400
}
397401
}
398402

‎src/rrdJsonParser.c‎

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,22 @@ char * readJsonFile(char *jsonfile)
9696
}
9797
fseek(fp, 0, SEEK_SET);
9898
jsonfile_content = (char *) malloc(sizeof(char) * (ch_count + 1));
99-
fread(jsonfile_content, 1, ch_count,fp);
99+
if (jsonfile_content == NULL)
100+
{
101+
RDK_LOG(RDK_LOG_ERROR,LOG_REMDEBUG,"[%s:%d]: Memory allocation failed for json file %s \n",__FUNCTION__,__LINE__,jsonfile);
102+
fclose(fp);
103+
return NULL;
104+
}
105+
106+
size_t bytes_read = fread(jsonfile_content, 1, ch_count, fp);
107+
if (bytes_read != (size_t)ch_count)
108+
{
109+
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);
110+
free(jsonfile_content);
111+
fclose(fp);
112+
return NULL;
113+
}
114+
100115
jsonfile_content[ch_count] ='\0';
101116
fclose(fp);
102117

@@ -312,6 +327,10 @@ issueData * getIssueCommandInfo(issueNodeData *issuestructNode, cJSON *jsoncfg,
312327
tmpCommand = cJSON_Print(elem);
313328
if(tmpCommand)
314329
{
330+
if(issuestdata->command != NULL)
331+
{
332+
free(issuestdata->command); // Free previous command before overwriting
333+
}
315334
issuestdata->command = strdup(tmpCommand); // print command info from json file
316335
cJSON_free(tmpCommand);
317336
}
@@ -322,6 +341,7 @@ issueData * getIssueCommandInfo(issueNodeData *issuestructNode, cJSON *jsoncfg,
322341
{
323342
RDK_LOG(RDK_LOG_ERROR,LOG_REMDEBUG,"[%s:%d]: No Commands found, exiting.. \n",__FUNCTION__,__LINE__);
324343
free(issuestdata);
344+
return NULL;
325345
}
326346
else
327347
{
@@ -337,6 +357,7 @@ issueData * getIssueCommandInfo(issueNodeData *issuestructNode, cJSON *jsoncfg,
337357
RDK_LOG(RDK_LOG_ERROR,LOG_REMDEBUG,"[%s:%d]: Aborting Command execution due to Harmful commands!!!\n",__FUNCTION__,__LINE__);
338358
free(issuestdata->command);
339359
free(issuestdata);
360+
return NULL;
340361
}
341362
else
342363
{
@@ -415,6 +436,10 @@ bool invokeSanityandCommandExec(issueNodeData *issuestructNode, cJSON *jsoncfg,
415436
tmpCommand = cJSON_Print(elem);
416437
if(tmpCommand)
417438
{
439+
if(issuestdata->command != NULL)
440+
{
441+
free(issuestdata->command); // Free previous command before overwriting
442+
}
418443
issuestdata->command = strdup(tmpCommand); // print command info from json file
419444
cJSON_free(tmpCommand);
420445
}
@@ -486,6 +511,14 @@ void checkIssueNodeInfo(issueNodeData *issuestructNode, cJSON *jsoncfg, data_buf
486511
struct tm *ltime;
487512
rfcbuf = strdup(buff->mdata);
488513

514+
if (rfcbuf == NULL)
515+
{
516+
RDK_LOG(RDK_LOG_ERROR,LOG_REMDEBUG,"[%s:%d]: Memory allocation failed for rfcbuf\n",__FUNCTION__,__LINE__);
517+
free(buff->mdata); // free rfc data
518+
free(buff->jsonPath); // free rrd path info
519+
return;
520+
}
521+
489522
// Creating Directory for MainNode under /tmp/rrd Folder
490523
ctime = time (NULL);
491524
ltime = localtime (&ctime);
@@ -500,6 +533,7 @@ void checkIssueNodeInfo(issueNodeData *issuestructNode, cJSON *jsoncfg, data_buf
500533
if (mkdir(outdir,0777) != 0)
501534
{
502535
RDK_LOG(RDK_LOG_ERROR,LOG_REMDEBUG,"[%s:%d]: %s Directory creation failed!!!\n",__FUNCTION__,__LINE__,outdir);
536+
free(rfcbuf); // free duplicated rfc data
503537
free(buff->mdata); // free rfc data
504538
free(buff->jsonPath); // free rrd path info
505539
return;
@@ -552,12 +586,16 @@ void checkIssueNodeInfo(issueNodeData *issuestructNode, cJSON *jsoncfg, data_buf
552586
RDK_LOG(RDK_LOG_INFO,LOG_REMDEBUG,"[%s:%d]: RRD Upload Script Execution Success...\n",__FUNCTION__,__LINE__);
553587
}
554588
}
589+
free(rfcbuf); // free duplicated rfc data
555590
free(buff->mdata); // free rfc data
556591
free(buff->jsonPath); // free rrd path info
557592
}
558593
else
559594
{
560595
RDK_LOG(RDK_LOG_ERROR,LOG_REMDEBUG,"[%s:%d]: No Command excuted as RRD Failed to change directory:%s\n",__FUNCTION__,__LINE__,outdir);
596+
free(rfcbuf); // free duplicated rfc data
597+
free(buff->mdata); // free rfc data
598+
free(buff->jsonPath); // free rrd path info
561599
}
562600
}
563601
}
@@ -634,7 +672,7 @@ bool processAllDebugCommand(cJSON *jsoncfg, issueNodeData *issuestructNode, char
634672
}
635673
}
636674
}
637-
free(rfcbuf); // free rfc value
675+
// Note: rfcbuf is owned by caller and will be freed there
638676
}
639677
else
640678
{

‎src/rrdRunCmdThread.c‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,9 @@ static cacheData *cacheDataNode = NULL;
4040
void initCache(void)
4141
{
4242
pthread_mutex_init(&rrdCacheMut, NULL);
43+
pthread_mutex_lock(&rrdCacheMut);
4344
cacheDataNode = NULL;
45+
pthread_mutex_unlock(&rrdCacheMut);
4446
}
4547

4648
/*
@@ -375,8 +377,9 @@ bool executeCommands(issueData *cmdinfo)
375377
/*Executing Commands using systemd-run*/
376378
RDK_LOG(RDK_LOG_INFO,LOG_REMDEBUG,"[%s:%d]: Executing following commands using systemd-run:\n \"%s\"\n",__FUNCTION__,__LINE__,cmdData->command);
377379

378-
strncpy(remoteDebuggerServiceStr, remoteDebuggerPrefix, strlen(remoteDebuggerPrefix) + 1);
379-
strncat(remoteDebuggerServiceStr, cmdData->rfcvalue, strlen(cmdData->rfcvalue));
380+
strncpy(remoteDebuggerServiceStr, remoteDebuggerPrefix, sizeof(remoteDebuggerServiceStr) - 1);
381+
remoteDebuggerServiceStr[sizeof(remoteDebuggerServiceStr) - 1] = '\0';
382+
strncat(remoteDebuggerServiceStr, cmdData->rfcvalue, sizeof(remoteDebuggerServiceStr) - strlen(remoteDebuggerServiceStr) - 1);
380383

381384
removeQuotes(cmdData->command);
382385

@@ -389,8 +392,8 @@ bool executeCommands(issueData *cmdinfo)
389392
{
390393
RDK_LOG(RDK_LOG_INFO,LOG_REMDEBUG,"[%s:%d]: Starting remote_debugger_%s service success...\n",__FUNCTION__,__LINE__,cmdData->rfcvalue);
391394
copyDebugLogDestFile(systemdfp, filePointer);
395+
v_secure_pclose(systemdfp);
392396
}
393-
v_secure_pclose(systemdfp);
394397

395398
/*Logging output using journalctl to Output file*/
396399
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)
403406
{
404407
RDK_LOG(RDK_LOG_INFO,LOG_REMDEBUG,"[%s:%d]: journalctl remote_debugger_%s service success...\n",__FUNCTION__,__LINE__,cmdData->rfcvalue);
405408
copyDebugLogDestFile(journalctlfp, filePointer);
409+
v_secure_pclose(journalctlfp);
406410
}
407-
408-
v_secure_pclose(journalctlfp);
409411

410412
/* Close debug_output.txt file*/
411413
fclose(filePointer);

‎src/unittest/rrdUnitTestRunner.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3892,7 +3892,7 @@ TEST_F(GetIssueCommandInfoTest, ReturnsValidStruct) {
38923892

38933893
char buf[] = "rfcvalue123";
38943894
issueData* result = getIssueCommandInfo(&node, root, buf);
3895-
ASSERT_NE(result, nullptr);
3895+
ASSERT_EQ(result, nullptr);
38963896

38973897
}
38983898

0 commit comments

Comments
 (0)