RDKB-64347: Fixing coverity issues#1065
Open
bharathivelp wants to merge 1 commit intordkcentral:developfrom
Open
RDKB-64347: Fixing coverity issues#1065bharathivelp wants to merge 1 commit intordkcentral:developfrom
bharathivelp wants to merge 1 commit intordkcentral:developfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR targets Coverity findings by tightening bounds/NULL handling around string copies and file I/O in WiFi validation and sample apps.
Changes:
- Replace several unbounded string copies with
snprintf(...)in config validation paths. - Add error checking for
stat(),fread(), andrename()in CSI log rotation. - Add additional NULL handling / cleanup paths in webconfig consumer sample test helpers.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| source/utils/wifi_validator.c | Uses snprintf instead of copy_string for several JSON-derived fields (OUI/auth/MCC/MNC/HESSID/RADIUS/global strings). |
| source/sampleapps/wifievents_consumer_sample.c | Adds file operation return-value checks while rotating /tmp/CSI.bin. |
| source/sampleapps/webconfig_consumer_apis.c | Adds early-return cleanup when encoded subdoc string is NULL; simplifies some frees. |
| source/dml/wifi_ssp/ssp_loop.c | Replaces strcat with bounded append to avoid overflow building index lists. |
| source/dml/dml_webconfig/dml_onewifi_api.c | Replaces strcpy with bounded snprintf for WPS pin defaults. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ddfde57 to
3a6f391
Compare
verann462
reviewed
Apr 22, 2026
3a6f391 to
75f9ddd
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
source/utils/wifi_validator.c:1001
- Same preprocessor/brace issue as the primary radius IP handling: with
WIFI_HAL_VERSION_3_PHASE2defined, the inet_pton branch ends up outside theif (validate_ipv4_address... || validate_ipv6_address...)block, so validation/elseerror handling no longer gates the phase2 parsing. Please restructure the#ifndef/#elseso both branches are entirely within the same validationifand theelsepairs correctly.
validate_param_string(radius, "SecondaryRadiusServerIPAddr", param);
if (validate_ipv4_address(param->valuestring) == RETURN_OK || validate_ipv6_address(param->valuestring) == RETURN_OK) {
#ifndef WIFI_HAL_VERSION_3_PHASE2
snprintf((char *)vap_info->u.bss_info.security.u.radius.s_ip, sizeof(vap_info->u.bss_info.security.u.radius.s_ip), "%s", param->valuestring);
}
else {
wifi_util_dbg_print(WIFI_PASSPOINT,"%s:%d: Validation failed for SecondaryRadiusServerIPAddr\n", __func__, __LINE__);
strncpy(execRetVal->ErrorMsg, "Invalid Secondary Radius server IP",sizeof(execRetVal->ErrorMsg)-1);
return RETURN_ERR;
}
#else
/* check the INET family and update the radius ip address */
if(inet_pton(AF_INET, param->valuestring, &(vap_info->u.bss_info.security.u.radius.s_ip.u.IPv4addr)) > 0) {
vap_info->u.bss_info.security.u.radius.s_ip.family = wifi_ip_family_ipv4;
} else if(inet_pton(AF_INET6, param->valuestring, &(vap_info->u.bss_info.security.u.radius.s_ip.u.IPv6addr)) > 0) {
vap_info->u.bss_info.security.u.radius.s_ip.family = wifi_ip_family_ipv6;
} else {
get_wificcsp_obj()->desc.CcspTraceErrorRdkb_fn("WIFI_PASSPOINT, <%s> <%d> : inet_pton falied for primary radius IP\n", __FUNCTION__, __LINE__);
return RETURN_ERR;
}
#endif
source/utils/wifi_validator.c:973
- The
#ifndef WIFI_HAL_VERSION_3_PHASE2/#elseblocks are structured so that, whenWIFI_HAL_VERSION_3_PHASE2is defined, the inet_pton parsing compiles outside theif (validate_ipv4_address... || validate_ipv6_address...)block. This makes the phase2 parsing run unconditionally and also breaks the intendedelseerror handling. Restructure so both phase2 and non-phase2 implementations live inside the same validationif(and keep theelsepaired with thatif).
validate_param_string(radius, "RadiusServerIPAddr", param);
if (validate_ipv4_address(param->valuestring) == RETURN_OK || validate_ipv6_address(param->valuestring) == RETURN_OK) {
#ifndef WIFI_HAL_VERSION_3_PHASE2
snprintf((char *)vap_info->u.bss_info.security.u.radius.ip, sizeof(vap_info->u.bss_info.security.u.radius.ip), "%s", param->valuestring);
}
else {
wifi_util_dbg_print(WIFI_PASSPOINT,"%s:%d: Validation failed for RadiusServerIPAddr\n", __func__, __LINE__);
strncpy(execRetVal->ErrorMsg, "Invalid Radius server IP",sizeof(execRetVal->ErrorMsg)-1);
return RETURN_ERR;
}
#else
/* check the INET family and update the radius ip address */
if(inet_pton(AF_INET, param->valuestring, &(vap_info->u.bss_info.security.u.radius.ip.u.IPv4addr)) > 0) {
vap_info->u.bss_info.security.u.radius.ip.family = wifi_ip_family_ipv4;
} else if(inet_pton(AF_INET6, param->valuestring, &(vap_info->u.bss_info.security.u.radius.ip.u.IPv6addr)) > 0) {
vap_info->u.bss_info.security.u.radius.ip.family = wifi_ip_family_ipv6;
} else {
get_wificcsp_obj()->desc.CcspTraceErrorRdkb_fn("WIFI_PASSPOINT, <%s> <%d> : inet_pton falied for primary radius IP\n", __FUNCTION__, __LINE__);
return RETURN_ERR;
}
#endif
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
c5f64fb to
c7692bc
Compare
Reason for change: Fixing Low priority Coverity issues. Test Procedure: Build should be successful and the regression test should also succeed. Risks: Low Priority: P1 Signed-off-by: Velpula_Bharathi@comcast.com
c7692bc to
9bd09f9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reason for change: Fixing Low priority coverity issues.
Test Procedure: Build should be successful and the regression test should also succeed.
Risks: Low
Priority: P1
Signed-off-by: Velpula_Bharathi@comcast.com