Fix memory safety issues blocking PR-161 L1 tests by shibu-kv · Pull Request #363 · rdkcentral/telemetry

shibu-kv · 2026-05-04T21:43:45Z

Fix double-free crash, NULL dereference, memory leaks, and buffer overflow risks identified in telemetry PR-161 code review.

Key fixes:

Expose freeProfile() and call on error in processConfiguration()
Add NULL checks before Vector_Create() and Vector_Size()
Free allocated strings before error returns in reportgen.c
Add bounds checking for strcat/strcpy operations
Move debug logging before free() calls
Update test mocks for new function signature

Fixes L1 test crash (exit code 134) and resolves all critical memory safety issues in PR-161.

Fix double-free crash, NULL dereference, memory leaks, and buffer overflow risks identified in telemetry PR-161 code review. Key fixes: - Expose freeProfile() and call on error in processConfiguration() - Add NULL checks before Vector_Create() and Vector_Size() - Free allocated strings before error returns in reportgen.c - Add bounds checking for strcat/strcpy operations - Move debug logging before free() calls - Update test mocks for new function signature Fixes L1 test crash (exit code 134) and resolves all critical memory safety issues in PR-161.

Copilot

Pull request overview

This PR targets memory-safety defects in the Telemetry 2.0 pipeline (profile parsing, report generation, and unit tests) to unblock PR-161 L1 stability by addressing crash/leak/overflow paths.

Changes:

Exposes freeProfile() publicly and uses it for cleanup on configuration-parse failures.
Adds additional NULL-guarding around vector creation/usage and adjusts debug logging conditions.
Hardens encodeParamResultInJSON() with added frees on some failure paths and bounds checks for key construction; updates a test mock expectation for the function’s signature.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
source/test/bulkdata/profilexconfTest.cpp	Updates gmock expectation to match the 4-arg `encodeParamResultInJSON` signature.
source/t2parser/t2parser.c	Avoids recreating vectors when already present; adds `freeProfile()` + `cJSON_Delete()` on a specific parse error path; adjusts debug logging condition.
source/reportgen/reportgen.c	Adds bounds checks for concatenated JSON key building and frees some allocated strings on early returns.
source/bulkdata/profile.h	Exposes `freeProfile(void*)` in the public header.
source/bulkdata/profile.c	Makes `freeProfile()` non-static and adds a NULL guard before checking `dataModelTableList` size.

…se of strcat' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

rdkcmf-jenkins · 2026-05-04T21:57:56Z

Coverity Issue - Free of array-typed value

"free" frees incorrect pointer "content".

High Impact, CWE-590
BAD_FREE

Issue location

This issue was discovered outside the diff for this Pull Request. You can find it at:
source/t2parser/t2parser.c:1375

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

+                                        size_t len = strlen(concatenatedKey);
+                                        if (len + 1 >= sizeof(concatenatedKey))
+                                        {
+                                            T2Error("concatenatedKey buffer overflow: path too long\n");
+                                            if (parameterName)
+                                            {
+                                                free(parameterName);
+                                            }
+                                            if (parameterWild)
+                                            {
+                                                free(parameterWild);
+                                            }
+                                            return T2ERROR_FAILURE;
+                                        }
+                                        strncat(concatenatedKey, ".", sizeof(concatenatedKey) - strlen(concatenatedKey) - 1);
+                                        len++; // Account for the '.'
+
+                                        if (len + strlen(token) >= sizeof(concatenatedKey))
+                                        {
+                                            T2Error("concatenatedKey buffer overflow: path too long\n");
+                                            if (parameterName)
+                                            {
+                                                free(parameterName);
+                                            }
+                                            if (parameterWild)
+                                            {
+                                                free(parameterWild);
+                                            }
+                                            return T2ERROR_FAILURE;
+                                        }
+                                        strncat(concatenatedKey, token, sizeof(concatenatedKey) - strlen(concatenatedKey) - 1);
+                                    }
+                                    else
+                                    {
+                                        if (strlen(token) >= sizeof(concatenatedKey))
+                                        {
+                                            T2Error("concatenatedKey buffer overflow: token too long\n");
+                                            if (parameterName)
+                                            {
+                                                free(parameterName);
+                                            }
+                                            if (parameterWild)
+                                            {
+                                                free(parameterWild);
+                                            }
+                                            return T2ERROR_FAILURE;
+                                        }


Resolve build error in telemetry (#364)

Copilot

Pull request overview

Copilot reviewed 25 out of 26 changed files in this pull request and generated 6 comments.

Comments suppressed due to low confidence (1)

source/test/bulkdata/Makefile.am:84

profile_dynamictable_Test.cpp defines its own main(), but this target also links -lgtest_main, which provides another main() and will cause a multiple-definition link error. Either remove -lgtest_main from this target’s LDFLAGS or drop the custom main() and rely on gtest_main (and/or add the shared gtest_main.cpp like other test binaries).

profile_dynamictable_gtest_bin_CPPFLAGS = $(profile_gtest_bin_CPPFLAGS)

profile_dynamictable_gtest_bin_SOURCES = profile_dynamictable_Test.cpp ../mocks/rdklogMock.cpp ../mocks/rbusMock.cpp ../mocks/profileStub.c ../../utils/vector.c ../../utils/t2log_wrapper.c ../../utils/t2common.c ../../utils/t2collection.c

profile_dynamictable_gtest_bin_LDFLAGS = -L/usr/src/googletest/googletest/lib/.libs -lgcov -L/src/googletest/googlemock/lib -L/usr/src/googletest/googlemock/lib/.libs -L/usr/include/glib-2.0 -lgmock -lcjson -lcurl -lmsgpackc -lgtest -lgtest_main -lglib-2.0

+    for report in `ls /tmp/Gtest_Report/*.json`; do
+        if [ -f "$report" ]; then
+            echo "Contents of $report:"
+            cat "$report"
+            echo "----------------------------------------"
+        else
+            echo "No JSON report files found in /tmp/Gtest_Report/"
+        fi
+    done


+#include <string.h>
 #include <gtest/gtest.h>
 #include <gmock/gmock.h>

 extern "C" {
 #include "t2log_wrapper.h"
 }

+#define GTEST_DEFAULT_RESULT_FILEPATH "/tmp/Gtest_Report/"
+#define GTEST_DEFAULT_RESULT_FILENAME "ccspinterface_gtest_report.json"
+#define GTEST_REPORT_FILEPATH_SIZE 128
+
 int main(int argc, char *argv[])
 {
+    char testresults_fullfilepath[GTEST_REPORT_FILEPATH_SIZE];
+
+    memset( testresults_fullfilepath, 0, GTEST_REPORT_FILEPATH_SIZE );
+    snprintf( testresults_fullfilepath, GTEST_REPORT_FILEPATH_SIZE, "json:%s%s" , GTEST_DEFAULT_RESULT_FILEPATH , GTEST_DEFAULT_RESULT_FILENAME);
+


                                    if (cJSON_AddStringToObject(currentObject, token, paramValues[valIndex]->parameterValue) == NULL)
                                    {
                                        T2Error("cJSON_AddStringToObject failed.\n");
+                                        if (parameterName)
+                                        {
+                                            free(parameterName);
+                                        }
+                                        if (parameterWild)
+                                        {
+                                            free(parameterWild);
+                                        }
                                        cJSON_Delete(currentObject);
                                        return T2ERROR_FAILURE;


    retvalue = encodingSet(profile, jprofileEncodingType, jprofileJSONReportFormat, jprofileJSONReportTimestamp);
    if(retvalue != T2ERROR_SUCCESS)
    {
        T2Error("Encoding parameters are not configured.Profile is invalid\n");
        return retvalue;
    }

    //Protocol Configuration
    retvalue = protocolSet (profile, jprofileProtocol, jprofileHTTPURL, jprofileHTTPRequestURIParameter, ThisprofileHTTPRequestURIParameter_count, jprofileRBUSMethodName, jprofileRBUSMethodParamArr, rbusMethodParamArrCount);
    if(retvalue != T2ERROR_SUCCESS)
    {
        T2Error("Protocol Configuration is invalid\n");
        return retvalue;
    }
    //Parameter Marker Configuration
    retvalue = addParameter_marker_config(profile, jprofileParameter, ThisProfileParameter_count);
-    if(retvalue != T2ERROR_SUCCESS)
+    if (retvalue != T2ERROR_SUCCESS)
    {
        T2Error("Parameter marker configuration is invalid\n");
+        freeProfile(profile);
+        cJSON_Delete(json_root);
+        return T2ERROR_FAILURE;
    }


Copilot

Pull request overview

Copilot reviewed 27 out of 32 changed files in this pull request and generated 8 comments.

+                                        {
+                                            free(parameterWild);
+                                        }
                                        cJSON_Delete(currentObject);


+                                            if (parameterWild)
+                                            {
+                                                free(parameterWild);
+                                            }


+        # Run the test and filter out lines starting with "LOG.RDK." to reduce noise in the output
+        # Store output and capture exit status in POSIX-compliant way
+        output=$("$test" 2>&1)
        status=$?
+        echo "=========== Output of $test execution: =========="
+        echo "$output" | grep -v "^LOG\.RDK\."


shibu-kv · 2026-05-06T16:48:41Z

L2 failures are with latest container release and needs to be fixed . This will be fixed in follow up PR merges.

Copilot AI review requested due to automatic review settings May 4, 2026 21:43

shibu-kv requested a review from a team as a code owner May 4, 2026 21:43

Copilot started reviewing on behalf of shibu-kv May 4, 2026 21:44 View session

github-advanced-security AI found potential problems May 4, 2026

View reviewed changes

Comment thread source/reportgen/reportgen.c Fixed

Copilot AI reviewed May 4, 2026

View reviewed changes

Comment thread source/t2parser/t2parser.c Outdated

Comment thread source/reportgen/reportgen.c

Comment thread source/reportgen/reportgen.c

shibu-kv added 2 commits May 4, 2026 14:49

Correct merge conflict errors

099b0d7

Fix astyle formatting compliance errors

bdcf379

Copilot AI review requested due to automatic review settings May 4, 2026 21:54

Potential fix for pull request finding 'CodeQL / Potentially unsafe u…

af33e46

…se of strcat' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

Copilot started reviewing on behalf of shibu-kv May 4, 2026 21:55 View session

rdkcmf-jenkins reviewed May 4, 2026

View reviewed changes

Comment thread source/t2parser/t2parser.c

Copilot AI reviewed May 4, 2026

View reviewed changes

Comment thread source/t2parser/t2parser.c Outdated

Comment thread source/reportgen/reportgen.c

Comment thread source/reportgen/reportgen.c

Comment thread source/t2parser/t2parser.c

shibu-kv added 2 commits May 4, 2026 15:02

Fix astyle formatting error checks

3696c7f

Fix coverity flagged issues

98b81ec

Copilot AI review requested due to automatic review settings May 4, 2026 22:18

Copilot started reviewing on behalf of shibu-kv May 4, 2026 22:18 View session

Copilot AI reviewed May 4, 2026

View reviewed changes

shibu-kv added 6 commits May 5, 2026 07:12

Merge pull request #365 from rdkcentral/develop

248277f

Resolve build error in telemetry (#364)

Update quality checker skill with L1 & L2 execution capabilities

a942767

Add unit tests for dynamic table feature

2b88211

Correct the test failures

661b865

Add L1 tests for dynamic table feature addition

6271dbc

Fix test report overwriting issue

8a5e9ef

Copilot AI review requested due to automatic review settings May 5, 2026 22:23

Copilot started reviewing on behalf of shibu-kv May 5, 2026 22:24 View session

Copilot AI reviewed May 5, 2026

View reviewed changes

shibu-kv added 2 commits May 5, 2026 15:38

Correct the documentation errors

5e013c8

Fix astyle formatting errors

c63936b

Copilot AI review requested due to automatic review settings May 5, 2026 22:50

Copilot started reviewing on behalf of shibu-kv May 5, 2026 22:50 View session

Copilot AI reviewed May 5, 2026

View reviewed changes

shibu-kv merged commit 39a4d65 into develop May 6, 2026
18 of 20 checks passed

github-actions Bot locked and limited conversation to collaborators May 6, 2026

Conversation

shibu-kv commented May 4, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rdkcmf-jenkins commented May 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverity Issue - Free of array-typed value

Issue location

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shibu-kv commented May 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rdkcmf-jenkins commented May 4, 2026 •

edited

Loading