RDKB-64347: Fixing coverity issues by vanshika-singhal24 · Pull Request #1060 · rdkcentral/OneWifi

vanshika-singhal24 · 2026-04-13T16:55:23Z

Reason for change: Fixing high medium priority coverity issues.
Test Procedure: Build should be successful and the regression test should also succeed.

Risks: Low
Priority: P1
Signed-off-by: vanshika_lnu@comcast.com

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull request overview

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

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

Copilot · 2026-04-15T07:46:17Z

            snprintf(recName, sizeof(recName),BeaconRateCtl, instance_number);
            retPsmSet = PSM_Set_Record_Value2(bus_handle, g_Subsystem, recName, ccsp_string, instanceNumStr);
            if(retPsmSet == CCSP_SUCCESS) {
-                strcpy(cfg->beacon_rate_ctl, acfg->u.bss_info.beaconRateCtl);
+                snprintf(cfg->beacon_rate_ctl, sizeof(cfg->beacon_rate_ctl), "%s", acfg->u.bss_info.beaconRateCtl);
                wifi_util_dbg_print(WIFI_PSM,"%s:%d BeaconRateCtl cfg->beacon_rate_ctl is %s\n",__func__, __LINE__,cfg->beacon_rate_ctl);


In this BeaconRateCtl update block, instanceNumStr is cleared but never populated with the new BeaconRateCtl value before calling PSM_Set_Record_Value2(). As a result, the PSM record will be set to an empty string even though cfg->beacon_rate_ctl is updated from acfg->u.bss_info.beaconRateCtl. Populate instanceNumStr from acfg->u.bss_info.beaconRateCtl (and consider validating/truncation) before the PSM_Set_Record_Value2 call.

Copilot · 2026-04-15T07:46:20Z

@@ -939,7 +939,7 @@ void Psm_Db_Write_Global(wifi_global_param_t *gcfg)
    if(strncmp(gcfg->wifi_region_code, cfg->wifi_region_code, strlen(cfg->wifi_region_code)) != 0){


The comparison uses strncmp(..., strlen(cfg->wifi_region_code)). If cfg->wifi_region_code is empty (len 0) this will never detect changes, and if cfg is a prefix of gcfg it can incorrectly treat values as equal. Prefer strcmp() for full-string comparison or strncmp() with a bound like sizeof(cfg->wifi_region_code).

Suggested change

if(strncmp(gcfg->wifi_region_code, cfg->wifi_region_code, strlen(cfg->wifi_region_code)) != 0){

if(strcmp(gcfg->wifi_region_code, cfg->wifi_region_code) != 0){

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

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

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

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

Copilot · 2026-04-15T12:37:49Z

    for (i=0; i < (sizeof(wifi_api_list)/sizeof(struct hal_api_info)); i++) {
        if (strcmp(args[0], wifi_api_list[i].name) == 0) {
            if (num_args == 0 || (num_args-1 < wifi_api_list[i].num_args)) {
-                sprintf(buff, "wifi_api2: Error - Invalid number of arguments\nhelp: %s %s\n", 
+                snprintf(buff, sizeof(buff), "wifi_api2: Error - Invalid number of arguments\nhelp: %s %s\n", 
                                wifi_api_list[i].name, wifi_api_list[i].help);


process_wifiapi_command() copies len bytes into a fixed 1024-byte input buffer (via memcpy) without bounding len or guaranteeing NUL-termination before strtok_r(). Since the RBUS setter enqueues (strlen(pTmp)+1) with no max length, a long command can overflow input and corrupt memory. Add a size check (reject/return an error if len >= sizeof(input)), and do a bounded copy that always terminates the string (e.g., copy at most sizeof(input)-1 and set the last byte to '\0').

Copilot

Pull request overview

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

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

Copilot · 2026-04-15T12:54:06Z

    if (found == 0) {
-        sprintf(buff, "wifi_api2: Invalid API '%s'", args[0]);
+        snprintf(buff, sizeof(buff), "wifi_api2: Invalid API '%s'", args[0]);
        goto publish;


args[0] is used in error reporting without guaranteeing any tokens were parsed. If command is empty/whitespace, num_args stays 0 and args[0] is uninitialized, leading to undefined behavior/crash. Add an early guard after tokenization (e.g., if num_args==0) to return/publish a helpful error before referencing args[0] (and similarly avoid the later strcmp loop on args[0] in that case).

Copilot · 2026-04-15T12:54:07Z

@@ -3543,7 +3543,7 @@ int wifidb_get_wifi_global_config(wifi_global_param_t *config)
        if (strlen(pcfg->wps_pin) != 0) {
            strncpy(config->wps_pin,pcfg->wps_pin,sizeof(config->wps_pin)-1);


strncpy(config->wps_pin, pcfg->wps_pin, sizeof(config->wps_pin)-1) does not guarantee NUL-termination when the source string length is >= sizeof(config->wps_pin)-1. Ensure config->wps_pin[sizeof(config->wps_pin)-1] is set to '\0' (or use snprintf) to avoid unterminated strings being consumed by later string operations.

Suggested change

strncpy(config->wps_pin,pcfg->wps_pin,sizeof(config->wps_pin)-1);

snprintf(config->wps_pin, sizeof(config->wps_pin), "%s", pcfg->wps_pin);

Reason for change: Fixing high medium priority coverity issues. Test Procedure: Build should be successful and the regression test should also succeed. Risks: Low Priority: P1 Signed-off-by: vanshika_lnu@comcast.com

vanshika-singhal24 requested a review from a team as a code owner April 13, 2026 16:55

navyasher requested a review from Copilot April 15, 2026 06:48

Copilot started reviewing on behalf of navyasher April 15, 2026 06:48 View session