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

Author: Sid2001-maker

include/IJavaScriptContext.h

@@ -25,6 +25,7 @@
 class IJavaScriptContext
 {
   public:
+    virtual ~IJavaScriptContext() = default; 
     virtual bool runScript(const char *script, bool isModule=true, std::string name="", const char *args = nullptr, bool isApplication=false) = 0;
     virtual bool runFile(const char *file, const char* args, bool isApplication=false) = 0;
     virtual std::string getUrl()  = 0;

include/JSRuntimeClient.h

@@ -40,17 +40,23 @@ template <typename Derived>
 class CommandInterface
 {
 public:
-    CommandInterface() = default;
+    CommandInterface() : mResponseReceived(false) {}
 
     bool sendCommand(std::string command, std::string &response)
     {
         Derived &derived = static_cast<Derived &>(*this);
         if (derived.send(command))
         {
             std::unique_lock<std::mutex> lock(mResponseMutex);
-            mResponseCondition.wait_for(lock, std::chrono::seconds(5));
-            response = mLastResponse;
-            return true;
+            mResponseReceived = false;
+            bool gotResponse = mResponseCondition.wait_for(lock, std::chrono::seconds(5), 
+                [this]() { return mResponseReceived; });
+            
+            if (gotResponse)
+            {
+                response = mLastResponse;
+                return true;
+            }
         }
 
         return false;
@@ -63,6 +69,7 @@ class CommandInterface
     {
         std::lock_guard<std::mutex> lock(mResponseMutex);
         mLastResponse = message;
+        mResponseReceived = true;
         mResponseCondition.notify_one();
     }
 
@@ -71,6 +78,7 @@ class CommandInterface
     CommandInterface &operator=(const CommandInterface &) = delete;
 
     std::string mLastResponse;
+    bool mResponseReceived;
     std::mutex mResponseMutex;
     std::condition_variable mResponseCondition;
 };

src/JSRuntimeClient.cpp

@@ -76,7 +76,9 @@ bool JSRuntimeClient::run()
     t.detach();
 
     std::unique_lock<std::mutex> lock(mStateMutex);
-    mStateCondition.wait_for(lock, std::chrono::seconds(5));
+    mStateCondition.wait_for(lock, std::chrono::seconds(5), [this]() { 
+        return mState != "none"; 
+    });
     return mState == "open";
 }
 
@@ -152,37 +154,45 @@ void JSRuntimeClient::onClose(websocketpp::connection_hdl hdl)
 #ifndef UNIT_TEST_BUILD
 int main(int argc, char **argv)
 {
-    std::string command;
-    std::string response;
+    try {
+        std::string command;
+        std::string response;
 
-    if (argc > 1)
-    {
-	NativeJSLogger::log(INFO, "Send input commands at ws://localhost:%s\n", std::to_string(WS_SERVER_PORT).c_str());
-        return -1;
-    }
-
-    JSRuntimeClient *client = JSRuntimeClient::getInstance();
-    client->initialize(WS_SERVER_PORT);
-    if (!client->run())
-    {
-	NativeJSLogger::log(ERROR, "Unable to connect to server\n");
-        return -1;
-    }
+        if (argc > 1)
+        {
+	    NativeJSLogger::log(INFO, "Send input commands at ws://localhost:%s\n", std::to_string(WS_SERVER_PORT).c_str());
+            return -1;
+        }
 
-    while (client->getState() == "open" && std::getline(std::cin, command))
-    {
-        client->sendCommand(command, response);
-        if (!response.empty())
+        JSRuntimeClient *client = JSRuntimeClient::getInstance();
+        client->initialize(WS_SERVER_PORT);
+        if (!client->run())
         {
-	     NativeJSLogger::log(INFO, "Response: %s\n", response.c_str());
+	    NativeJSLogger::log(ERROR, "Unable to connect to server\n");
+            return -1;
         }
-        else
+
+        while (client->getState() == "open" && std::getline(std::cin, command))
         {
-            NativeJSLogger::log(WARN, "Missing response\n");
-            break;
+            client->sendCommand(command, response);
+            if (!response.empty())
+            {
+	        NativeJSLogger::log(INFO, "Response: %s\n", response.c_str());
+            }
+            else
+            {
+                NativeJSLogger::log(WARN, "Missing response\n");
+                break;
+            }
         }
-    }
 
-    return 0;
+        return 0;
+    } catch (const std::exception& e) {
+        NativeJSLogger::log(ERROR, "Uncaught exception in main: %s\n", e.what());
+        return -1;
+    } catch (...) {
+        NativeJSLogger::log(ERROR, "Unknown exception caught in main\n");
+        return -1;
+    }
 }
 #endif

src/JSRuntimeClientContainer.cpp

@@ -6,26 +6,36 @@
 #include "NativeJSLogger.h"
 int main()
 {
-    std::string containerId = "com.sky.as.apps_TestApp";
-    const std::string basePath = "/opt/twocontext"; // constant base path
-    const std::vector<std::string> apps = {"app1", "app2"};
-    
-    std::string ipAddress = JSRuntimeContainer::getContainerIpAddress(containerId);
-    if (ipAddress.empty()) {
-	NativeJSLogger::log(ERROR, "Failed to retrieve IP address for container");
-	return 1; 
-    }
+    try {
+        std::string containerId = "com.sky.as.apps_TestApp";
+        const std::string basePath = "/opt/twocontext"; // constant base path
+        const std::vector<std::string> apps = {"app1", "app2"};
+        
+        std::string ipAddress = JSRuntimeContainer::getContainerIpAddress(containerId);
+        if (ipAddress.empty()) {
+            NativeJSLogger::log(ERROR, "Failed to retrieve IP address for container\n");
+            return 1; 
+        }
 
-    for (const auto &app : apps) {
-        std::string url = basePath + std::string("/") + app + std::string("/index.html");
-        if (access(url.c_str(), F_OK) == 0) {
-            std::string pathAppConfig = basePath + std::string("/") + app + std::string("/app.config");
-            std::string options = JSRuntimeContainer::parseAppConfig(pathAppConfig);
-            std::string message = JSRuntimeContainer::buildLaunchMessage(url, options);
-            JSRuntimeContainer::connectAndSend(ipAddress, message);
+        for (const auto &app : apps) {
+            std::string url = basePath + std::string("/") + app + std::string("/index.html");
+            if (access(url.c_str(), F_OK) == 0) {
+                std::string pathAppConfig = basePath + std::string("/") + app + std::string("/app.config");
+                std::string options = JSRuntimeContainer::parseAppConfig(pathAppConfig);
+                std::string message = JSRuntimeContainer::buildLaunchMessage(url, options);
+                JSRuntimeContainer::connectAndSend(ipAddress, message);
+            }
         }
-    }
 
-    return 0;
+        return 0;
+    }
+    catch (const std::exception& e) {
+        NativeJSLogger::log(ERROR, "Exception in main: %s\n", e.what());
+        return 1;
+    }
+    catch (...) {
+        NativeJSLogger::log(ERROR, "Unknown exception in main\n");
+        return 1;
+    }
 }
 

src/JSRuntimeServer.cpp

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

src/NativeJSRenderer.cpp

@@ -225,10 +225,8 @@ void NativeJSRenderer::setEnvForConsoleMode(ModuleSettings& moduleSettings)
 
 uint32_t NativeJSRenderer::createApplicationIdentifier()
 {
-    static uint32_t id = 1;
-    uint32_t ret = id;
-    id++;
-    return ret;
+    static std::atomic<uint32_t> id{1};
+    return id++;
 }
 
 uint32_t  NativeJSRenderer::createApplication(ModuleSettings& moduleSettings, std::string userAgent)
@@ -292,6 +290,7 @@ bool NativeJSRenderer::terminateApplication(uint32_t id)
 	return true;
 }
 
+// REQUIRES: Caller must hold mUserMutex
 void NativeJSRenderer::createApplicationInternal(ApplicationRequest& appRequest)
 {
         double startTime = getTimeInMilliSec();
@@ -330,9 +329,9 @@ void NativeJSRenderer::createApplicationInternal(ApplicationRequest& appRequest)
         context->setCreateApplicationEndTime(endTime, id);
 
         mContextMap[id].context=context;
-        mUserMutex.unlock();
 }
 
+// REQUIRES: Caller must hold mUserMutex
 void NativeJSRenderer::runApplicationInternal(ApplicationRequest& appRequest)
 {
 	uint32_t id = appRequest.mId;
@@ -365,7 +364,7 @@ void NativeJSRenderer::runApplicationInternal(ApplicationRequest& appRequest)
            			NativeJSLogger::log(INFO, "Adding the window location: %s to js file\n", window.str().c_str());
             			context->runScript(window.str().c_str(),true, url, nullptr, true);
 			}
-			NativeJSLogger::log(INFO, "nativeJS application thunder execution url: %s, result: %d\n", url.c_str(), ret ? 1 : 0);
+			NativeJSLogger::log(INFO, "nativeJS application thunder execution url: %s\n", url.c_str());
 			ret = context->runScript(chunk.contentsBuffer, true, url, nullptr, true);
 			NativeJSLogger::log(INFO, "nativeJS application execution result: %d\n", ret ? 1 : 0);
 			double duration = context->getExecutionDuration();
@@ -398,6 +397,7 @@ void NativeJSRenderer::runApplicationInternal(ApplicationRequest& appRequest)
 	}
 }
 
+// REQUIRES: Caller must hold mUserMutex
 void NativeJSRenderer::runJavaScriptInternal(ApplicationRequest& appRequest)
 {
 	uint32_t id = appRequest.mId;
@@ -427,6 +427,7 @@ void NativeJSRenderer::runJavaScriptInternal(ApplicationRequest& appRequest)
 	}
 }
 
+// REQUIRES: Caller must hold mUserMutex
 void NativeJSRenderer::terminateApplicationInternal(ApplicationRequest& AppRequest)
 {
 	uint32_t id = AppRequest.mId;
@@ -445,7 +446,7 @@ void NativeJSRenderer::terminateApplicationInternal(ApplicationRequest& AppReque
 
 	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\n", id);
 		return ;
 	}
 
@@ -468,11 +469,10 @@ void NativeJSRenderer::run()
 {
     while(mRunning)
     {
-	uint32_t id;
-	mUserMutex.lock();
         if (mConsoleMode) {
             processDevConsoleRequests();
         }
+	mUserMutex.lock();
         for (int i=0; i<gPendingRequests.size(); i++)
         {
 
@@ -506,10 +506,13 @@ void NativeJSRenderer::run()
         if(!mTestFileName.empty())
         {
             ModuleSettings settings;
+            uint32_t id = createApplicationIdentifier();
 	    settings.enableJSDOM = mEnableTestFileDOMSupport;
 	    ApplicationRequest appRequest(id, RUN, mTestFileName, settings.enableHttp, settings.enableXHR, settings.enableWebSocket, settings.enableWebSocketEnhanced, settings.enableFetch, settings.enableJSDOM, settings.enableWindow, settings.enablePlayer);
+	    mUserMutex.lock();
 	    NativeJSRenderer::createApplicationInternal(appRequest);
 	    NativeJSRenderer::runApplicationInternal(appRequest);
+	    mUserMutex.unlock();
 	    mTestFileName = "";
         }
 
@@ -531,27 +534,30 @@ void NativeJSRenderer::run()
 
 void NativeJSRenderer::processDevConsoleRequests()
 {
+    std::deque<std::string> localQueue;
+    
+    // Move items from shared queue to local queue while holding the lock
     mConsoleState->inputMutex.lock();
-
     if (mConsoleState->codeToExecute.empty()) {
         mConsoleState->inputMutex.unlock();
         return;
     }
+    localQueue.swap(mConsoleState->codeToExecute);
+    mConsoleState->inputMutex.unlock();
 
     std::lock_guard<std::mutex> lockg(mConsoleState->isProcessing_cv_m);
     bool dataProcessed = false;
 
-    for (; !mConsoleState->codeToExecute.empty(); mConsoleState->codeToExecute.pop_front()) {
-        bool ret = mConsoleState->consoleContext->runScript(mConsoleState->codeToExecute.front().c_str(), false);
+    // Process items from local queue (no race condition)
+    for (const auto& code : localQueue) {
+        bool ret = mConsoleState->consoleContext->runScript(code.c_str(), false);
         dataProcessed = true;
     }
 
     if (dataProcessed) {
         mConsoleState->isProcessing = false;
         mConsoleState->isProcessing_cv.notify_one();
     }
-
-    mConsoleState->inputMutex.unlock();
 }
 
 std::atomic_bool NativeJSRenderer::consoleLoop = true;
@@ -620,18 +626,34 @@ bool NativeJSRenderer::downloadFile(std::string& url, MemoryStruct& chunk)
     curl = curl_easy_init();
     if (curl)
     {
-        curl_easy_setopt(curl, CURLOPT_URL, url.c_str());
-        curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1);
-        curl_easy_setopt(curl, CURLOPT_HEADERFUNCTION, HeaderCallback);
-        curl_easy_setopt(curl, CURLOPT_HEADERDATA, (void *)&chunk);
-        curl_easy_setopt(curl, CURLOPT_WRITEFUNCTION, WriteMemoryCallback);
-        curl_easy_setopt(curl, CURLOPT_WRITEDATA, (void *)&chunk);
-        curl_easy_setopt(curl, CURLOPT_TIMEOUT, 30);
-        curl_easy_setopt(curl, CURLOPT_NOSIGNAL, 1L);
-        curl_easy_setopt(curl, CURLOPT_SSL_VERIFYHOST, 2);
-        curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, true);
-        curl_easy_setopt(curl, CURLOPT_USERAGENT, "libcurl-agent/1.0");
-        curl_easy_setopt(curl, CURLOPT_PROXY, "");
+        // Helper lambda to check curl_easy_setopt results
+        auto setOptWithCheck = [curl](CURLoption option, auto value, const char* optionName) -> bool {
+            CURLcode optRes = curl_easy_setopt(curl, option, value);
+            if (optRes != CURLE_OK) {
+                NativeJSLogger::log(ERROR, "Failed to set %s: %s\n", optionName, curl_easy_strerror(optRes));
+                return false;
+            }
+            return true;
+        };
+
+        // Set options; cleanup and return if any fail
+        if (
+            !setOptWithCheck(CURLOPT_URL, url.c_str(), "CURLOPT_URL") ||
+            !setOptWithCheck(CURLOPT_FOLLOWLOCATION, 1L, "CURLOPT_FOLLOWLOCATION") ||
+            !setOptWithCheck(CURLOPT_HEADERFUNCTION, HeaderCallback, "CURLOPT_HEADERFUNCTION") ||
+            !setOptWithCheck(CURLOPT_HEADERDATA, (void *)&chunk, "CURLOPT_HEADERDATA") ||
+            !setOptWithCheck(CURLOPT_WRITEFUNCTION, WriteMemoryCallback, "CURLOPT_WRITEFUNCTION") ||
+            !setOptWithCheck(CURLOPT_WRITEDATA, (void *)&chunk, "CURLOPT_WRITEDATA") ||
+            !setOptWithCheck(CURLOPT_TIMEOUT, 30L, "CURLOPT_TIMEOUT") ||
+            !setOptWithCheck(CURLOPT_NOSIGNAL, 1L, "CURLOPT_NOSIGNAL") ||
+            !setOptWithCheck(CURLOPT_SSL_VERIFYHOST, 2L, "CURLOPT_SSL_VERIFYHOST") ||
+            !setOptWithCheck(CURLOPT_SSL_VERIFYPEER, 1L, "CURLOPT_SSL_VERIFYPEER") ||
+            !setOptWithCheck(CURLOPT_USERAGENT, "libcurl-agent/1.0", "CURLOPT_USERAGENT") ||
+            !setOptWithCheck(CURLOPT_PROXY, "", "CURLOPT_PROXY")
+        ) {
+            curl_easy_cleanup(curl);
+            return ret;
+        }
 
 
         //curl_easy_setopt(curl, CURLOPT_WRITEDATA, fp);

src/jsc/JavaScriptContext.cpp

@@ -119,6 +119,13 @@ if (mModuleSettings.enablePlayer)
         gTopLevelContext = nullptr;
     }
     mPriv->releaseAllProtected();
+
+    if (mNetworkMetricsData)
+    {
+        delete mNetworkMetricsData;
+        mNetworkMetricsData = nullptr;
+    }
+    
     JSGlobalContextRelease(mContext);
     JSContextGroupRelease(mContextGroup);
     rtLogInfo("%s end", __FUNCTION__);
@@ -394,7 +401,7 @@ if (mModuleSettings.enablePlayer)
         gAAMPJSBindings = new AAMPJSBindings();
         loadAAMPJSBindingsLib();
     }
-    if (gAAMPJSBindings->fnLoadJS)
+    if (gAAMPJSBindings && gAAMPJSBindings->fnLoadJS)
     {
         gAAMPJSBindings->fnLoadJS(mContext);
     }