cov-test: Fixing High and medium level issues[DONOT MERGE] by Sid2001-maker · Pull Request #107 · rdkcentral/rdkNativeScript

Sid2001-maker · 2026-01-16T18:35:14Z

RDKEMW-12252: Coverity Scan Report - Analyzing and Fixing all the Critical and High issues

Reason for change: Resolve Critical and high level issues in coverity
Test Procedure: build should be successful
Risk: low
Priority: P2

…tical and High issues Reason for change: Resolve Critical and high level issues in coverity Test Procedure: build should be successful Risk: low Priority: P2

rdkcmf-jenkins · 2026-01-16T18:36:08Z

b'## Blackduck scan failure details

Summary: 0 violations, 0 files pending approval, 1 file pending identification.

Protex Server Path: /home/blackduck/github/rdkNativeScript/107/rdkcentral/rdkNativeScript
Commit: a930ddf

Report detail: gist'

Copilot

Pull request overview

This pull request addresses Coverity scan findings by fixing critical and high-level issues related to error handling, resource management, and code quality. The PR aims to resolve issues flagged by static analysis tools.

Changes:

Added exception handling to main functions in multiple files
Fixed error checking for curl operations and file I/O
Improved resource cleanup and memory management
Corrected format specifiers and mutex handling
Added virtual destructor to interface class

Reviewed changes

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

Show a summary per file

File	Description
src/jsruntime.cpp	Added try-catch blocks around main function and commented out Wayland display validation
src/jsc/jsc_lib/jsc_lib.cpp	Enhanced curl error checking and improved path separator handling
src/jsc/JavaScriptUtils.cpp	Fixed mutex unlock, added error checking for file operations, and improved resource cleanup
src/jsc/JavaScriptEngine.cpp	Corrected format specifier from %d to %f for double variable
src/jsc/JavaScriptContext.cpp	Added cleanup for network metrics data and null pointer check for AAMP bindings
src/NativeJSRenderer.cpp	Added curl error checking and fixed variable scope issue
src/JSRuntimeServer.cpp	Initialized error return value
src/JSRuntimeClientContainer.cpp	Added exception handling to main function
src/JSRuntimeClient.cpp	Improved condition variable usage and added exception handling
include/IJavaScriptContext.h	Added virtual destructor

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-16T18:40:35Z

+  if (stat(fd, &buf) != 0)
+  {
+    rtLogError("Failed to stat file: %s", fd);
+    fclose(ptr);
+    return RT_ERROR;
+  }
+
  int size = buf.st_size;

  buffer = (char*)malloc(size);
-  fread(buffer,size,1,ptr); // read 10 bytes to our buffer
+  size_t bytesRead = fread(buffer, size, 1, ptr);
  fclose(ptr);


The fopen call may fail and return nullptr, but there's no check before using ptr. Before calling stat, fread, or fclose with ptr, verify that fopen succeeded and ptr is not null to prevent null pointer dereferences.

Copilot · 2026-01-16T18:40:36Z

 	else
 	{
-		NativeJSLogger::log(ERROR, "Unable to find application with id: %d and url: %s\n", id, mContextMap[id].url);
+		NativeJSLogger::log(ERROR, "Unable to find application with id: %d and url: %s\n", id, mContextMap[id].url.c_str());


Attempting to access mContextMap[id].url when id is not found in the map causes undefined behavior. This line is in the else branch that executes when the id is not found (line 445), so accessing mContextMap[id].url will either access an invalid entry or create a new one. Remove the .c_str() call or avoid accessing mContextMap[id] altogether in this error path.

Suggested change

NativeJSLogger::log(ERROR, "Unable to find application with id: %d and url: %s\n", id, mContextMap[id].url.c_str());

NativeJSLogger::log(ERROR, "Unable to find application with id: %d\n", id);

Copilot · 2026-01-16T18:40:36Z

        if(!mTestFileName.empty())
        {
            ModuleSettings settings;
+            uint32_t id;


The variable id is declared inside the if block on line 507 but is used to initialize ApplicationRequest on line 509. Since uint32_t is uninitialized, this will result in using an indeterminate value. Initialize id before using it or ensure it's assigned a valid value.

Suggested change

uint32_t id;

uint32_t id = 0;

Copilot · 2026-01-16T18:40:36Z

+    /*
+    //CID:430751:Use of untrusted string value (TAINTED_STRING)
+    // Validate waylanddisplay to prevent injection attacks
+    if (!waylanddisplay.empty()) {
+        if (waylanddisplay.length() > 256) {
+            NativeJSLogger::log(ERROR, "Invalid Wayland display name: too long\n");
+            waylanddisplay = "";
+        } else {
+            bool valid = true;
+            for (char c : waylanddisplay) {
+                if (!isalnum(c) && c != '-' && c != '_' && c != '/' && c != '.') {
+                    valid = false;
+                    break;
+                }
+            }
+            if (!valid) {
+                NativeJSLogger::log(ERROR, "Invalid Wayland display name: contains invalid characters\n");
+                waylanddisplay = "";
+            }
+        }
+    }
+    */


Commented-out code related to Coverity issue CID:430751 for tainted string validation has been left in the codebase. This validation code for the waylanddisplay parameter appears to address a security concern but is disabled. Either remove this commented code or uncomment and enable it if the validation is necessary for security.

Copilot · 2026-01-16T18:40:37Z

+            curl_easy_cleanup(curl);
+        }
+        res = curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 2);
+        if (res != CURLE_OK) {
+            NativeJSLogger::log(ERROR, "Failed to set CURLOPT_SSL_VERIFYHOST: %s\n", curl_easy_strerror(res));
+            curl_easy_cleanup(curl);
+        }
+        res = curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, true);
+        if (res != CURLE_OK) {
+            NativeJSLogger::log(ERROR, "Failed to set CURLOPT_SSL_VERIFYPEER: %s\n", curl_easy_strerror(res));
+            curl_easy_cleanup(curl);
+        }
+        res = curl_easy_setopt(curl, CURLOPT_USERAGENT, "libcurl-agent/1.0");
+        if (res != CURLE_OK) {
+            NativeJSLogger::log(ERROR, "Failed to set CURLOPT_USERAGENT: %s\n", curl_easy_strerror(res));
+            curl_easy_cleanup(curl);
+
+        }
+        res = curl_easy_setopt(curl, CURLOPT_PROXY, "");
+        if (res != CURLE_OK) {
+            NativeJSLogger::log(ERROR, "Failed to set CURLOPT_PROXY: %s\n", curl_easy_strerror(res));
+            curl_easy_cleanup(curl);


After checking if curl_easy_setopt fails and logging an error, the code calls curl_easy_cleanup but continues execution instead of returning immediately. This results in attempting to use an already-cleaned-up curl handle in subsequent curl_easy_setopt calls, which leads to undefined behavior. Add a return statement after curl_easy_cleanup to exit the function early on error.

Suggested change

curl_easy_cleanup(curl);

}

res = curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 2);

if (res != CURLE_OK) {

NativeJSLogger::log(ERROR, "Failed to set CURLOPT_SSL_VERIFYHOST: %s\n", curl_easy_strerror(res));

curl_easy_cleanup(curl);

}

res = curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, true);

if (res != CURLE_OK) {

NativeJSLogger::log(ERROR, "Failed to set CURLOPT_SSL_VERIFYPEER: %s\n", curl_easy_strerror(res));

curl_easy_cleanup(curl);

}

res = curl_easy_setopt(curl, CURLOPT_USERAGENT, "libcurl-agent/1.0");

if (res != CURLE_OK) {

NativeJSLogger::log(ERROR, "Failed to set CURLOPT_USERAGENT: %s\n", curl_easy_strerror(res));

curl_easy_cleanup(curl);

}

res = curl_easy_setopt(curl, CURLOPT_PROXY, "");

if (res != CURLE_OK) {

NativeJSLogger::log(ERROR, "Failed to set CURLOPT_PROXY: %s\n", curl_easy_strerror(res));

curl_easy_cleanup(curl);

curl_easy_cleanup(curl);

return ret;

}

res = curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 2);

if (res != CURLE_OK) {

NativeJSLogger::log(ERROR, "Failed to set CURLOPT_SSL_VERIFYHOST: %s\n", curl_easy_strerror(res));

curl_easy_cleanup(curl);

return ret;

}

res = curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, true);

if (res != CURLE_OK) {

NativeJSLogger::log(ERROR, "Failed to set CURLOPT_SSL_VERIFYPEER: %s\n", curl_easy_strerror(res));

curl_easy_cleanup(curl);

return ret;

}

res = curl_easy_setopt(curl, CURLOPT_USERAGENT, "libcurl-agent/1.0");

if (res != CURLE_OK) {

NativeJSLogger::log(ERROR, "Failed to set CURLOPT_USERAGENT: %s\n", curl_easy_strerror(res));

curl_easy_cleanup(curl);

return ret;

}

res = curl_easy_setopt(curl, CURLOPT_PROXY, "");

if (res != CURLE_OK) {

NativeJSLogger::log(ERROR, "Failed to set CURLOPT_PROXY: %s\n", curl_easy_strerror(res));

curl_easy_cleanup(curl);

return ret;

Copilot · 2026-01-16T18:40:37Z

@@ -85,6 +85,7 @@ class JsonWrap
        if (!itm || !cJSON_IsNumber(itm))
        {
            std::cerr << "Error: " << name << "is not a Uint32_t" << std::endl;


Missing space in error message. The error message concatenates name directly with "is" without a space separator, resulting in output like "valueis not a Uint32_t" instead of "value is not a Uint32_t".

Suggested change

std::cerr << "Error: " << name << "is not a Uint32_t" << std::endl;

std::cerr << "Error: " << name << " is not a Uint32_t" << std::endl;

RDKEMW-12252: Coverity Scan Report - Analyzing and Fixing all the Cri…

a930ddf

…tical and High issues Reason for change: Resolve Critical and high level issues in coverity Test Procedure: build should be successful Risk: low Priority: P2

Sid2001-maker requested a review from a team as a code owner January 16, 2026 18:35

Copilot AI review requested due to automatic review settings January 16, 2026 18:35

Copilot started reviewing on behalf of Sid2001-maker January 16, 2026 18:35 View session

Copilot AI reviewed Jan 16, 2026

View reviewed changes

Sid2001-maker closed this Jan 19, 2026

Sid2001-maker deleted the topic/RDKEMW-12252 branch January 19, 2026 13:14

github-actions Bot locked and limited conversation to collaborators Jan 19, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

cov-test: Fixing High and medium level issues[DONOT MERGE]#107

cov-test: Fixing High and medium level issues[DONOT MERGE]#107
Sid2001-maker wants to merge 1 commit into
developfrom
topic/RDKEMW-12252

Sid2001-maker commented Jan 16, 2026

Uh oh!

rdkcmf-jenkins commented Jan 16, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 16, 2026

Uh oh!

Copilot AI Jan 16, 2026

Uh oh!

Copilot AI Jan 16, 2026

Uh oh!

Copilot AI Jan 16, 2026

Uh oh!

Copilot AI Jan 16, 2026

Uh oh!

Copilot AI Jan 16, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	NativeJSLogger::log(ERROR, "Unable to find application with id: %d and url: %s\n", id, mContextMap[id].url.c_str());
	NativeJSLogger::log(ERROR, "Unable to find application with id: %d\n", id);

	std::cerr << "Error: " << name << "is not a Uint32_t" << std::endl;
	std::cerr << "Error: " << name << " is not a Uint32_t" << std::endl;

Uh oh!

Conversation

Sid2001-maker commented Jan 16, 2026

Uh oh!

rdkcmf-jenkins commented Jan 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 16, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants