Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
cd795e2
Refactor issueTypeSplitter to include suffix handling
Abhinavpv28 Apr 30, 2026
a950f13
Update rrdEventProcess.c
Abhinavpv28 Apr 30, 2026
ba3c8ca
Update rrdExecuteScript.c
Abhinavpv28 Apr 30, 2026
63bc161
Refactor rrdEventProcess.h to rrdExecuteScript.h
Abhinavpv28 Apr 30, 2026
c883bd2
Update rrdExecuteScript.h
Abhinavpv28 Apr 30, 2026
9380442
Update rrdExecuteScript.h
Abhinavpv28 Apr 30, 2026
61a9e2c
Update rrdEventProcess.h
Abhinavpv28 Apr 30, 2026
d672210
Update rrdExecuteScript.h
Abhinavpv28 Apr 30, 2026
65d30d0
Update rrdJsonParser.c
Abhinavpv28 Apr 30, 2026
93df789
Update rrdJsonParser.h
Abhinavpv28 Apr 30, 2026
6d37d71
Update rrdInterface.c
Abhinavpv28 Apr 30, 2026
c7588d3
Update rrdCommon.h
Abhinavpv28 Apr 30, 2026
d1345ab
Update rrdEventProcess.c
Abhinavpv28 Apr 30, 2026
e720479
Update rrdEventProcess.c
Abhinavpv28 May 1, 2026
faacd2a
Update rrdEventProcess.c
Abhinavpv28 May 1, 2026
7daeac3
Update rrdEventProcess.c
Abhinavpv28 May 1, 2026
52539a3
Fix indentation for appendMode assignment
Abhinavpv28 May 1, 2026
ec1680e
Update rrdCommon.h
Abhinavpv28 May 1, 2026
bfb7f1e
Fix appendMode assignment in rrdEventProcess.c
Abhinavpv28 May 1, 2026
e71f82e
Update rrdEventProcess.c
Abhinavpv28 May 1, 2026
72b9562
Update rrdJsonParser.c
Abhinavpv28 May 1, 2026
e6b8690
Update rrdEventProcess.c
Abhinavpv28 May 1, 2026
79f6e64
Update rrdExecuteScript.h
Abhinavpv28 May 1, 2026
bd49f43
Update rrdExecuteScript.c
Abhinavpv28 May 1, 2026
3a5f988
Update rrdExecuteScript.c
Abhinavpv28 May 1, 2026
4587984
Update rrdExecuteScript.c
Abhinavpv28 May 1, 2026
5c3f460
Remove redundant logging from rrdJsonParser
Abhinavpv28 May 1, 2026
3d77c29
Update rrdJsonParser.c
Abhinavpv28 May 1, 2026
b5f062b
Update rrdEventProcess.c
Abhinavpv28 May 1, 2026
f21ad0f
Update rrdJsonParser.c
Abhinavpv28 May 1, 2026
02a0507
Update rrdEventProcess.c
Abhinavpv28 May 1, 2026
f509d0a
Update rrdJsonParser.c
Abhinavpv28 May 1, 2026
d10faa4
Apply suggestions from code review
Abhinavpv28 May 1, 2026
5835261
Update rrdEventProcess.c
Abhinavpv28 May 1, 2026
c9dfede
Apply suggestion from @Copilot
Abhinavpv28 May 1, 2026
3c19ac2
Update rrdJsonParser.c
Abhinavpv28 May 1, 2026
3084d70
Fix heap overflow in issueTypeSplitter and memory leaks in suffix han…
Copilot May 2, 2026
eca3fb2
Update rrdUnitTestRunner.cpp
Abhinavpv28 May 2, 2026
2cca47b
Delete .gitignore
Abhinavpv28 May 2, 2026
f44f131
Potential fix for pull request finding
Abhinavpv28 May 2, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/rrdCommon.h
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ typedef struct mbuffer {
bool inDynamic;
bool appendMode;
deepsleep_event_et dsEvent;
char *suffix; // Holds the suffix split from issue type string, if any
} data_buf;

/*Structure for Message Header*/
Expand Down
59 changes: 47 additions & 12 deletions src/rrdEventProcess.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ static void processIssueTypeInDynamicProfile(data_buf *rbuf, issueNodeData *pIss
static void processIssueTypeInStaticProfile(data_buf *rbuf, issueNodeData *pIssueNode);
static void processIssueTypeInInstalledPackage(data_buf *rbuf, issueNodeData *pIssueNode);
static void removeSpecialCharacterfromIssueTypeList(char *str);
static int issueTypeSplitter(char *input_str, const char delimeter, char ***args);
static int issueTypeSplitter(char *input_str, char *outsuffix, const char delimeter, char ***args);
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issueTypeSplitter is a static function that is included and unit-tested via src/unittest/rrdUnitTestRunner.cpp (it directly includes rrdEventProcess.c). This signature change adds the outsuffix parameter, so the existing unit tests will no longer compile until they are updated to pass the extra argument.

Suggested change
static int issueTypeSplitter(char *input_str, char *outsuffix, const char delimeter, char ***args);
static int issueTypeSplitter(char *input_str, char *outsuffix, const char delimeter, char ***args);
#ifdef __cplusplus
static int issueTypeSplitter(char *input_str, const char delimeter, char ***args)
{
return issueTypeSplitter(input_str, NULL, delimeter, args);
}
#endif

Copilot uses AI. Check for mistakes.
static void freeParsedJson(cJSON *jsonParsed);
Comment on lines +32 to 33
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing issueTypeSplitter’s signature (adding outsuffix) will break existing unit tests and any other call sites that still use the old (input_str, delimiter, &args) form. There are GTest unit tests under src/unittest/rrdUnitTestRunner.cpp that call issueTypeSplitter(str, ',', &args) and will fail to compile until updated (or until a backward-compatible wrapper is provided).

Copilot uses AI. Check for mistakes.

/*
Expand Down Expand Up @@ -62,13 +62,18 @@ void processIssueTypeEvent(data_buf *rbuf)
char **cmdMap = NULL;
int index = 0, count = 0, dataMsgLen = 0;
data_buf *cmdBuff = NULL;
char suffix[128] = {0};

RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: ...Entering.. \n", __FUNCTION__, __LINE__);
if (NULL != rbuf)
{
RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: IssueType List [%s]... \n", __FUNCTION__, __LINE__, rbuf->mdata);
count = issueTypeSplitter(rbuf->mdata, ',', &cmdMap);

count = issueTypeSplitter(rbuf->mdata, suffix, ',', &cmdMap);
if (rbuf->suffix) {
Comment on lines +71 to +72
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unit tests exist for processIssueTypeEvent, but there are no cases covering the new “issue type suffix” behavior. Please add a test where rbuf->mdata contains a suffix (e.g., Device.DeviceTime_Search-...) and assert that the base issue type is processed while the suffix is preserved for later upload naming.

Copilot uses AI. Check for mistakes.
free(rbuf->suffix);
}
rbuf->suffix = (suffix[0] != '\0') ? strdup(suffix) : NULL;
Comment on lines +73 to +75
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (rbuf->suffix) free(rbuf->suffix); assumes rbuf->suffix is either NULL or a valid heap pointer. If a caller passes a non-zero-initialized data_buf (e.g., stack-allocated without calling RRD_data_buff_init), this can attempt to free an uninitialized/garbage pointer and crash. Please ensure data_buf is always initialized before calling this, or make this function robust by not freeing a potentially uninitialized field here (leave it to RRD_data_buff_deAlloc).

Copilot uses AI. Check for mistakes.
Comment on lines +72 to +75
RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: Assigned rbuf->suffix='%s' after issueTypeSplitter\n", __FUNCTION__, __LINE__, rbuf->suffix ? rbuf->suffix : "(null)");
if (count > 0)
{
Comment on lines +74 to 78
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rbuf->suffix is heap-allocated with strdup, but processIssueTypeEvent never frees rbuf (or calls RRD_data_buff_deAlloc) after processing the message. With this change, each EVENT_MSG will also leak rbuf->suffix. Consider freeing the original rbuf/rbuf->mdata once the per-issue cmdBuff messages have been dispatched/processed, or ensure cleanup happens in the caller.

Copilot generated this review using guidance from repository custom instructions.
RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: IssueType Count [%d]... \n", __FUNCTION__, __LINE__, count);
Expand All @@ -88,6 +93,13 @@ void processIssueTypeEvent(data_buf *rbuf)
}
cmdBuff->appendMode = rbuf->appendMode;
cmdBuff->mdata = (char *)calloc(1, dataMsgLen);

/* Persist suffix */
if (rbuf->suffix) {
cmdBuff->suffix = strdup(rbuf->suffix);
} else {
cmdBuff->suffix = NULL;
}
Comment on lines +97 to +102
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cmdBuff->suffix is allocated via strdup() but cmdBuff is freed with plain free() (not RRD_data_buff_deAlloc), and checkIssueNodeInfo() currently does not free buff->suffix. This introduces a memory leak for every issued command buffer. Either free cmdBuff->suffix in this function when cmdBuff->mdata allocation fails / after processIssueType(), or switch to using RRD_data_buff_deAlloc() for cmdBuff so all owned fields (mdata/jsonPath/suffix) are released consistently.

Copilot uses AI. Check for mistakes.
if (cmdBuff->mdata)
{
Comment on lines +100 to 104
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cmdBuff->suffix = strdup(rbuf->suffix) is not checked for NULL. If this allocation fails, downstream code may behave inconsistently and the cleanup path becomes harder to reason about. Please check the return value and handle failure explicitly.

Copilot generated this review using guidance from repository custom instructions.
strncpy((char *)cmdBuff->mdata, cmdMap[index], dataMsgLen);
Expand All @@ -97,11 +109,16 @@ void processIssueTypeEvent(data_buf *rbuf)
{
RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: Memory Allocation Failed... \n", __FUNCTION__, __LINE__);
}
if(cmdBuff)
{
if(cmdBuff)
{
Copy link

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cmdBuff->suffix is allocated via strdup() but cmdBuff is freed with free(cmdBuff) (not RRD_data_buff_deAlloc()), and checkIssueNodeInfo() currently does not free buff->suffix. This leaks memory for every processed issue type. Either switch to RRD_data_buff_deAlloc(cmdBuff) here, or explicitly free cmdBuff->suffix (and ensure checkIssueNodeInfo() frees buff->suffix alongside mdata/jsonPath if it owns it).

Suggested change
{
{
if (cmdBuff->suffix)
{
free(cmdBuff->suffix);
cmdBuff->suffix = NULL;
}

Copilot uses AI. Check for mistakes.
if (cmdBuff->suffix)
{
free(cmdBuff->suffix);
cmdBuff->suffix = NULL;
}
free(cmdBuff);
Copy link
Copy Markdown
Contributor

@rdkcmf-jenkins rdkcmf-jenkins Apr 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Coverity issue no longer present as of: undefined

Show issue

Coverity Issue - Resource leak

Freeing "cmdBuff" without freeing its pointer field "suffix" leaks the storage that "suffix" points to.

High Impact, CWE-404
RESOURCE_LEAK

cmdBuff = NULL;
}
cmdBuff = NULL;
}
}
Comment on lines +113 to 122
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cleanup block frees only cmdBuff but not the newly-added cmdBuff->suffix. This leaks memory for each processed issue type (and also leaks on the cmdBuff->mdata allocation failure path). Free cmdBuff->suffix (if set) before freeing the struct.

Copilot uses AI. Check for mistakes.
else
Comment on lines 119 to 123
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cmdBuff->suffix is newly allocated but cmdBuff is released via free(cmdBuff) rather than RRD_data_buff_deAlloc, and checkIssueNodeInfo does not free buff->suffix. This leaks cmdBuff->suffix on the success path. Prefer calling RRD_data_buff_deAlloc(cmdBuff) here, or explicitly free cmdBuff->suffix (and any other owned fields) before freeing the struct.

Copilot generated this review using guidance from repository custom instructions.
{
Expand Down Expand Up @@ -140,7 +157,8 @@ static void processIssueType(data_buf *rbuf)
issueData *staticprofiledata = NULL;

RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: ...Entering.. \n", __FUNCTION__, __LINE__);
if (rbuf->mdata != NULL) // issue data exits

if (rbuf->mdata != NULL) // issue data exists
{
pIssueNode = (issueNodeData *)malloc(sizeof(issueNodeData));
if(pIssueNode)
Expand Down Expand Up @@ -653,15 +671,33 @@ static void removeSpecialCharacterfromIssueTypeList(char *str)
* @function issueTypeSplitter
* @brief Splits a given string into tokens based on a specified delimiter, and removes any
* special characters from the string before splitting.
* @param char *input_str - The input string to be split.
* @param const char delimiter - The character used to split the string.
* The first underscore in input_str is treated as a suffix separator: the base
* (part before the underscore) is written back into input_str, and the suffix
* (underscore + remainder) is written into outsuffix.
* @param char *input_str - The input string to be split (modified in place: base written back).
* @param char *outsuffix - Buffer (at least BUF_LEN_128 bytes) to receive the suffix portion,
* or NULL if the caller does not need the suffix.
* @param const char delimeter - The character used to split the string.
* @param char ***args - Pointer to an array of strings where the tokens will be stored.
* @return int - The number of tokens found in the string.
*/
static int issueTypeSplitter(char *input_str, const char delimeter, char ***args)
static int issueTypeSplitter(char *input_str, char *outsuffix, const char delimeter, char ***args)
Comment on lines 683 to +684
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are existing unit tests for issueTypeSplitter in src/unittest/rrdUnitTestRunner.cpp that call the old signature (issueTypeSplitter(str, ',', &args)). This signature change adds outsuffix, so the unit-test build will break unless those tests are updated, and new assertions should be added to cover the suffix-splitting behavior.

Copilot uses AI. Check for mistakes.
{
int cnt = 1, i = 0;
char *str = input_str;
char base[ BUF_LEN_128] = {0};
char suffix[ BUF_LEN_128] = {0};
split_issue_type(str, base, sizeof(base), suffix, sizeof(suffix));
RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: issueTypeSplitter (pre-clean): input='%s', base='%s', suffix='%s'\n", __FUNCTION__, __LINE__, str, base, suffix);

/* Copy only the actual base string back into str. base is always a prefix
* of the original str so it fits within the original allocation. */
memmove(str, base, strlen(base) + 1);
Comment on lines +690 to +695

if (outsuffix != NULL)
{
snprintf(outsuffix, BUF_LEN_128, "%s", suffix);
Comment on lines +690 to +699
}

Comment on lines +684 to 701
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issueTypeSplitter writes to outsuffix with strncpy(outsuffix, suffix, 128) but the function doesn't take outsuffix's length. This makes the helper unsafe to reuse and also doesn't guarantee NUL-termination for smaller buffers. Pass an outsuffix_len parameter and bound/terminate the copy accordingly (and validate outsuffix != NULL).

Copilot uses AI. Check for mistakes.
removeSpecialCharacterfromIssueTypeList(str);
while (*str == delimeter)
Expand Down Expand Up @@ -698,4 +734,3 @@ static int issueTypeSplitter(char *input_str, const char delimeter, char ***args

return cnt;
}

5 changes: 5 additions & 0 deletions src/rrdInterface.c
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,7 @@ void RRD_data_buff_init(data_buf *sbuf, message_type_et sndtype, deepsleep_event
sbuf->inDynamic = false;
sbuf->appendMode = false;
sbuf->dsEvent = deepSleepEvent;
sbuf->suffix = NULL;
}

/*Function: RRD_data_buff_deAlloc
Expand All @@ -295,6 +296,10 @@ void RRD_data_buff_deAlloc(data_buf *sbuf)
{
free(sbuf->jsonPath);
}
if (sbuf->suffix)
{
free(sbuf->suffix);
}
free(sbuf);
}
}
Expand Down
68 changes: 67 additions & 1 deletion src/rrdJsonParser.c
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,49 @@ void removeSpecialChar(char *str)
}
}

/*
* @function split_issue_type
* @brief Utility to split base and suffix from issue type string.
* Example: Input: Device.DeviceTime_Search-b6877385-9463-45fc-b19d-a24d77fd0790
* Output: base = Device.DeviceTime, suffix = _Search-b6877385-9463-45fc-b19d-a24d77fd0790
* @param const char *input - The input string to split.
* @param char *base - Buffer to store the base part (before the first underscore).
* @param size_t base_len - Size of the base buffer.
* @param char *suffix - Buffer to store the suffix part (from the first underscore onwards).
* @param size_t suffix_len - Size of the suffix buffer.
* @return void
*/

void split_issue_type(const char *input, char *base, size_t base_len, char *suffix, size_t suffix_len) {
if (!input || !base || !suffix) return;
Comment thread
Abhinavpv28 marked this conversation as resolved.
Comment thread
Abhinavpv28 marked this conversation as resolved.

if (base_len == 0 || suffix_len == 0) {
if (base && base_len > 0) {
base[0] = '\0';
}
if (suffix && suffix_len > 0) {
suffix[0] = '\0';
}
return;
}
RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: split_issue_type called with input='%s'\n", __FUNCTION__, __LINE__, input);
const char *underscore = strchr(input, '_');
if (underscore) {
Comment on lines +65 to +76
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

split_issue_type has redundant length checks (if (base_len == 0 || suffix_len == 0) is performed twice) and logs at INFO level for every call, including echoing the full input string. Removing the duplicate check and lowering/guarding the log level would reduce noise and keep the utility easier to maintain.

Copilot uses AI. Check for mistakes.
size_t b_len = underscore - input;
if (b_len >= base_len) b_len = base_len - 1;
Comment on lines +54 to +78
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

split_issue_type and issueTypeSplitter log at RDK_LOG_INFO on every call and include full input/output strings. This looks like per-event code and may generate noisy logs in production. Consider lowering these to RDK_LOG_DEBUG (or gating behind a debug flag) to avoid operational log volume.

Copilot uses AI. Check for mistakes.
strncpy(base, input, b_len);
base[b_len] = '\0';
strncpy(suffix, underscore, suffix_len - 1);
suffix[suffix_len - 1] = '\0';
} else {
Comment on lines +79 to +83
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

split_issue_type does base_len - 1 / suffix_len - 1 arithmetic (and writes at suffix[suffix_len - 1]). If either length is 0, this underflows and becomes an out-of-bounds write. Since this function is now exposed in the header, please defensively check base_len > 0 and suffix_len > 0 up front (and return/clear outputs when invalid).

Copilot uses AI. Check for mistakes.
strncpy(base, input, base_len - 1);
base[base_len - 1] = '\0';
suffix[0] = '\0';
Comment on lines +62 to +86
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

split_issue_type does not validate base_len/suffix_len before doing base_len - 1 / suffix_len - 1. If either length is 0, this underflows and leads to out-of-bounds writes and large strncpy calls (UB). Add guards like if (base_len == 0 || suffix_len == 0) return; (and ensure outputs are NUL-terminated when returning early).

Copilot uses AI. Check for mistakes.
}
RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: split_issue_type result: base='%s', suffix='%s'\n", __FUNCTION__, __LINE__, base, suffix);
}


/*
* @function getParamcount
* @brief Calculates the total number of nodes (elements) in the input string, excluding delimiters.
Expand Down Expand Up @@ -515,7 +558,11 @@ void checkIssueNodeInfo(issueNodeData *issuestructNode, cJSON *jsoncfg, data_buf
{
RDK_LOG(RDK_LOG_ERROR,LOG_REMDEBUG,"[%s:%d]: Memory allocation failed for rfcbuf\n",__FUNCTION__,__LINE__);
free(buff->mdata); // free rfc data
buff->mdata = NULL;
free(buff->jsonPath); // free rrd path info
buff->jsonPath = NULL;
free(buff->suffix); // free suffix
buff->suffix = NULL;
return;
}

Expand All @@ -535,7 +582,11 @@ void checkIssueNodeInfo(issueNodeData *issuestructNode, cJSON *jsoncfg, data_buf
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
buff->mdata = NULL;
free(buff->jsonPath); // free rrd path info
buff->jsonPath = NULL;
free(buff->suffix); // free suffix
buff->suffix = NULL;
return;
}
else
Expand Down Expand Up @@ -576,7 +627,14 @@ void checkIssueNodeInfo(issueNodeData *issuestructNode, cJSON *jsoncfg, data_buf
else
{
RDK_LOG(RDK_LOG_DEBUG,LOG_REMDEBUG,"[%s:%d]: Continue uploading Debug Report for %s from %s... \n",__FUNCTION__,__LINE__,buff->mdata,outdir);
status = uploadDebugoutput(outdir,buff->mdata);
// Use the persisted suffix from buff for upload
char tarName[512] = {0};
if (buff->suffix && buff->suffix[0] != '\0') {
snprintf(tarName, sizeof(tarName), "%s%s", buff->mdata, buff->suffix);
} else {
snprintf(tarName, sizeof(tarName), "%s", buff->mdata);
}
status = uploadDebugoutput(outdir, tarName);
Comment on lines +633 to +637
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tarName is now built from buff->mdata + buff->suffix, but checkIssueNodeInfo still only frees buff->mdata and buff->jsonPath on its exit paths. Since suffix is allocated via strdup in the event flow, this introduces a leak. Free (and ideally NULL) buff->suffix in the same cleanup paths as the other data_buf fields.

Copilot uses AI. Check for mistakes.
Comment on lines +630 to +637
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

buff->suffix is appended into tarName and then passed to uploadDebugoutput, which ultimately can invoke a shell/script (v_secure_system). Unlike the base issue type, the suffix is not run through removeSpecialCharacterfromIssueTypeList and can contain arbitrary characters from the event payload. Please validate/sanitize the suffix to a safe character set (e.g., [A-Za-z0-9_-]) and/or normalize it before constructing tarName.

Copilot uses AI. Check for mistakes.
if(status != 0)
Comment on lines +631 to 638
Copy link

Copilot AI May 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checkIssueNodeInfo now uses buff->suffix to build tarName, but the function’s cleanup paths free buff->mdata and buff->jsonPath without freeing buff->suffix. This introduces a leak for every processed issue buffer that has a suffix (and also in early-return error paths like rfcbuf == NULL). Add free(buff->suffix) (and preferably set it to NULL) alongside the other frees in each cleanup path.

Copilot uses AI. Check for mistakes.
{
RDK_LOG(RDK_LOG_ERROR,LOG_REMDEBUG,"[%s:%d]: RRD Upload Script Execution Failed!!! status:%d\n",__FUNCTION__,__LINE__,status);
Expand All @@ -588,14 +646,22 @@ void checkIssueNodeInfo(issueNodeData *issuestructNode, cJSON *jsoncfg, data_buf
}
free(rfcbuf); // free duplicated rfc data
free(buff->mdata); // free rfc data
buff->mdata = NULL;
free(buff->jsonPath); // free rrd path info
buff->jsonPath = NULL;
free(buff->suffix); // free suffix
buff->suffix = NULL;
}
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
buff->mdata = NULL;
free(buff->jsonPath); // free rrd path info
buff->jsonPath = NULL;
free(buff->suffix); // free suffix
buff->suffix = NULL;
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/rrdJsonParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ issueData* getIssueCommandInfo(issueNodeData *issuestructNode, cJSON *jsoncfg,ch
bool processAllDebugCommand(cJSON *jsoncfg, issueNodeData *issuestructNode, char *rfcbuf);
bool processAllDeepSleepAwkMetricsCommands(cJSON *jsoncfg, issueNodeData *issuestructNode, char *rfcbuf);

void split_issue_type(const char *input, char *base, size_t base_len, char *suffix, size_t suffix_len);

#ifdef __cplusplus
}
#endif
Expand Down
Empty file.
48 changes: 44 additions & 4 deletions src/unittest/rrdUnitTestRunner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1799,11 +1799,13 @@ TEST_F(RemoveItemTest, HandlesCacheNotNullAndCacheNotEqualsRrdCachecnode)
node->mdata = strdup("PkgData");
node->issueString = strdup("IssueString");
node->next = NULL;
node->prev = NULL;
cacheDataNode = node;
cacheData *node_dummy = (cacheData *)malloc(sizeof(cacheData));
node_dummy->mdata = strdup("PkgData");
node_dummy->issueString = strdup("IssueString");
node_dummy->next = NULL;
node_dummy->prev = NULL;
remove_item(node_dummy);

EXPECT_NE(cacheDataNode, nullptr);
Expand Down Expand Up @@ -1865,7 +1867,7 @@ TEST(IssueTypeSplitterTest, HandlesStringWithSpecialCharacters)
{
char str[] = "a@,b,&,cd,ef";
char **args = NULL;
int count = issueTypeSplitter(str, ',', &args);
int count = issueTypeSplitter(str, NULL, ',', &args);

ASSERT_EQ(count, 4);
ASSERT_STREQ(args[0], "a");
Expand All @@ -1884,7 +1886,7 @@ TEST(IssueTypeSplitterTest, HandlesStringWithNoSpecialCharacters)
{
char str[] = "abcd";
char **args = NULL;
int count = issueTypeSplitter(str, ',', &args);
int count = issueTypeSplitter(str, NULL, ',', &args);

ASSERT_EQ(count, 1);
ASSERT_STREQ(args[0], "abcd");
Expand All @@ -1900,13 +1902,48 @@ TEST(IssueTypeSplitterTest, HandlesEmptyString)
{
char str[] = "";
char **args = NULL;
int count = issueTypeSplitter(str, ',', &args);
int count = issueTypeSplitter(str, NULL, ',', &args);

ASSERT_EQ(count, 1);

free(args);
}

TEST(IssueTypeSplitterTest, ExtractsSuffixFromIssueType)
{
char str[] = "Device.DeviceTime_Search";
char outsuffix[128] = {0};
char **args = NULL;
int count = issueTypeSplitter(str, outsuffix, ',', &args);

ASSERT_EQ(count, 1);
ASSERT_STREQ(args[0], "Device.DeviceTime");
ASSERT_STREQ(outsuffix, "_Search");

for (int i = 0; i < count; i++)
{
free(args[i]);
}
free(args);
}

TEST(IssueTypeSplitterTest, NoSuffixWhenNoUnderscore)
{
char str[] = "Device.Network";
char outsuffix[128] = {0};
char **args = NULL;
int count = issueTypeSplitter(str, outsuffix, ',', &args);
Comment on lines +1912 to +1935

ASSERT_EQ(count, 1);
ASSERT_STREQ(outsuffix, "");

for (int i = 0; i < count; i++)
{
free(args[i]);
}
free(args);
}

/* --------------- Test processIssueTypeInDynamicProfile() from rrdEventProcess --------------- */
class ProcessIssueTypeInDynamicProfileTest : public ::testing::Test
{
Expand Down Expand Up @@ -1989,11 +2026,13 @@ TEST(ProcessIssueTypeEvntTest, RBufIsNull){
}

TEST(ProcessIssueTypeEvntTest, inDynamic_NoJson){
data_buf rbuf;
data_buf rbuf = {};
rbuf.mdata = strdup("a");
rbuf.inDynamic = true;
rbuf.jsonPath = nullptr;
processIssueTypeEvent(&rbuf);
free(rbuf.mdata);
rbuf.mdata = NULL;
}

/* ======================== rrdExecuteScript ==============*/
Expand Down Expand Up @@ -3684,6 +3723,7 @@ TEST_F(RRDEventThreadFuncTest, MessageReceiveSuccessEventMsgType) {
rbuf.mdata = strdup("Test");
rbuf.inDynamic = true;
rbuf.jsonPath = nullptr;
rbuf.suffix = nullptr;
msgRRDHdr msgHdr;
msgHdr.mbody = malloc(sizeof(data_buf));
ASSERT_NE(msgHdr.mbody, nullptr);
Expand Down
Loading