Skip to content

RDKB-64588: High impact coverity issues in utopia #293

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
abhishek-kumaracee2 wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

RDKB-64588: High impact coverity issues in utopia #293

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1f644f8
RDKB-64588: High impact coverity issues in utopia
abhishek-kumaracee2 Apr 22, 2026
8717c63
Merge branch 'develop' into topic/RDKB-64588
snayak002c Apr 23, 2026
286632e
RDKB-64588: High impact coverity issues in utopia
abhishek-kumaracee2 Apr 22, 2026
7923245
Merge branch 'topic/RDKB-64588' of https://github.com/rdkcentral/utop…
abhishek-kumaracee2 Apr 23, 2026
1eb7081
RDKB-64588: High impact coverity issues in utopia
abhishek-kumaracee2 Apr 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions source/firewall/firewall.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,7 @@ char cellular_ifname[32];
#define IS_EMPTY_STRING(s) ((s == NULL) || (*s == '\0'))

#define BUFLEN_8 8
#define BUFLEN_20 20
#define BUFLEN_32 32
#define BUFLEN_64 64
#define RET_OK 0
Expand All @@ -465,6 +466,9 @@ char cellular_ifname[32];
#define RESET "reset"
#define UP "up"

#define STR_HELPER(x) #x
#define STR(x) STR_HELPER(x)

#if defined (FEATURE_MAPT) || defined (FEATURE_SUPPORT_MAPT_NAT46)
#define SYSEVENT_MAPT_CONFIG_FLAG "mapt_config_flag"
#define SYSEVENT_MAPT_IP_ADDRESS "mapt_ip_address"
Expand Down Expand Up @@ -1824,6 +1828,7 @@ static int substitute(char *in_str, char *out_str, const int size, char *from, c
* $ACCEPT $DROP $REJECT and
* QoS classes $HIGH, $MEDIUM, $NORMAL, $LOW
*/
#define TOKEN_MAX_LEN 50
char *make_substitutions(char *in_str, char *out_str, const int size)
{
char *in_str_p = in_str;
Expand All @@ -1832,9 +1837,9 @@ char *make_substitutions(char *in_str, char *out_str, const int size)
char *out_str_end = out_str + size;
// FIREWALL_DEBUG("Entering *make_substitutions\n");
while (in_str_p < in_str_end && out_str_p < out_str_end) {
char token[50];
char token[TOKEN_MAX_LEN + 1];
if ('$' == *in_str_p) {
sscanf(in_str_p, "%50s", token);
sscanf(in_str_p, "%" STR(TOKEN_MAX_LEN) "s", token);
in_str_p += strlen(token);
Comment on lines +1840 to 1843
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TOKEN_MAX_LEN is introduced for buffer sizing, but the scan width is still hard-coded as "%50s". This couples the format string to the macro value and is easy to desync in the future (reintroducing the overflow). Consider deriving the scan width from TOKEN_MAX_LEN (e.g., via a stringified macro) or keep a single source of truth for both.

Copilot uses AI. Check for mistakes.
if (0 == strcmp(token, "$WAN_IPADDR")) {
out_str_p += snprintf(out_str_p, out_str_end-out_str_p, "%s", current_wan_ipaddr);
Expand Down Expand Up @@ -1905,9 +1910,9 @@ static char *match_keyword(FILE *fp, char *keyword, char delim, char *line, int
* handle space differently
*/
if (' ' == delim) {
char local_name[50];
char local_name[TOKEN_MAX_LEN + 1];
local_name[0] = '\0';
sscanf(line, "%50s ", local_name);
sscanf(line, "%" STR(TOKEN_MAX_LEN) "s", local_name);
next = line + strlen(local_name);
if (next-line > size) {
continue;
Expand Down Expand Up @@ -9824,9 +9829,9 @@ static int prepare_host_detect(FILE * fp)
char buf[1024];
if (NULL != kh_fp) {
while (NULL != fgets(buf, sizeof(buf), kh_fp)) {
char ip[20];
char mac[20];
sscanf(buf, "%20s %20s", ip, mac);
char ip[BUFLEN_20 + 1];
char mac[BUFLEN_20 + 1];
sscanf(buf, "%" STR(BUFLEN_20) "s" "%" STR(BUFLEN_20) "s", ip, mac);
fprintf(fp, "-A host_detect -i %s -s %s -j RETURN\n", lan_ifname, ip);
Comment on lines +9832 to 9835
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sscanf(buf, "%20s %20s", ip, mac) return value is not checked and ip/mac are not initialized. If a line is malformed, ip may contain garbage and get written into iptables rules. Initialize the buffers and continue unless sscanf returns 2.

Suggested change
char ip[BUFLEN_20 + 1];
char mac[BUFLEN_20 + 1];
sscanf(buf, "%20s %20s", ip, mac);
fprintf(fp, "-A host_detect -i %s -s %s -j RETURN\n", lan_ifname, ip);
char ip[BUFLEN_20 + 1] = {0};
char mac[BUFLEN_20 + 1] = {0};
if (sscanf(buf, "%20s %20s", ip, mac) != 2) {
continue;
}
fprintf(fp, "-A host_detect -i %s -s %s -j RETURN\n", lan_ifname, ip);

Copilot uses AI. Check for mistakes.
Comment on lines +9832 to 9835
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BUFLEN_20 is introduced for the ip/mac buffers, but the scan widths are still hard-coded as "%20s". Consider tying the scan width to BUFLEN_20 (stringified macro) so future changes to the constant can't silently reintroduce a buffer sizing mismatch.

Copilot uses AI. Check for mistakes.
}
fclose(kh_fp);
Expand Down
10 changes: 9 additions & 1 deletion source/service_ipv6/service_ipv6.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1730,7 +1730,15 @@ STATIC int gen_dibbler_conf(struct serv_ipv6 *si6)
if (get_dhcpv6s_pool_cfg(si6, &dhcpv6s_pool_cfg) != 0)
continue;

if (!dhcpv6s_pool_cfg.enable || dhcpv6s_pool_cfg.ia_prefix[0] == '\0') continue;
if (!dhcpv6s_pool_cfg.enable || dhcpv6s_pool_cfg.ia_prefix[0] == '\0') {
if (dhcpv6s_pool_cfg.opts != NULL) {
free(dhcpv6s_pool_cfg.opts);
dhcpv6s_pool_cfg.opts = NULL;
dhcpv6s_pool_cfg.opt_num = 0;
}
continue;
}

syscfg_get(NULL, "bridge_mode", bridge_mode, sizeof(bridge_mode));
if (strcmp(bridge_mode, "2") || strcmp(dhcpv6s_pool_cfg.interface, "brlan0")) {

Expand Down
2 changes: 1 addition & 1 deletion source/service_routed/service_routed.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -950,7 +950,7 @@ STATIC int gen_zebra_conf(int sefd, token_t setok)
char default_wan_interface[64] = {0};
char wan_interface[64] = {0};
#ifdef FEATURE_RDKB_CONFIGURABLE_WAN_INTERFACE
char mesh_wan_ifname[32];
char mesh_wan_ifname[32] = {0};
char *pStr = NULL;
int return_status = PSM_VALUE_GET_STRING(PSM_MESH_WAN_IFNAME,pStr);
if(return_status == CCSP_SUCCESS && pStr != NULL){
Expand Down
2 changes: 1 addition & 1 deletion source/ulog/ulog.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ int ulog_GetProcId(size_t size, char *name, pid_t *pid)
The name will be extracted with a final ')' which needs to be dropped
before writing the final result to "name".
*/
sscanf(buf, "%*d (%1023s", str);
sscanf(buf, "%*d (%63s", str);

Comment on lines +298 to 299
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sscanf width is hard-coded as 63, which implicitly depends on ULOG_STR_SIZE being 64. To avoid future desynchronization, consider using ULOG_STR_SIZE - 1 to define the scan width (via a stringified macro) so the bound stays correct if ULOG_STR_SIZE changes.

Copilot uses AI. Check for mistakes.
len = strlen(str);
Comment on lines +298 to 300
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sscanf(buf, "%*d (%63s", str); ignores the return value and str is not initialized beforehand. If parsing fails (unexpected /proc/<pid>/stat format), strlen(str) is undefined behavior. Initialize str[0] = '\0' and check that sscanf returns 1 (or switch to a scanset like %63[^)] to read up to ) safely) and return an error if parsing fails.

Copilot uses AI. Check for mistakes.
Comment on lines +298 to 300
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sscanf() return value is not checked and str is not initialized. If parsing /proc/<pid>/stat fails, strlen(str) will read uninitialized data (undefined behavior). Initialize str[0] = '\0' and verify sscanf(...) returns 1 before using str (otherwise return an error).

Copilot uses AI. Check for mistakes.
if (len > 0)
Expand Down
7 changes: 5 additions & 2 deletions source/utapi/lib/utapi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@

#define CALCULATE_CRC32_TABLE_ENTRY(X) (((X) & 1) ? (POLYNOMIAL^ ((X) >> 1)) : ((X) >> 1))

#define BUFLEN_10 10
#define STR_HELPER(x) #x
#define STR(x) STR_HELPER(x)
/*
* utapi.c -
*/
Expand Down Expand Up @@ -4273,8 +4276,8 @@ static int s_getiap (UtopiaContext *ctx, int index, iap_entry_t *iap)
app[j].proto = s_StrToEnum(g_ProtocolMap, buf);

Utopia_GetIndexed2(ctx, UtopiaValue_IAP_BlockPortRange, index, i+1, buf, sizeof(buf));
char sport[10], eport[10];
if (2 == (sscanf(buf, "%10s %10s", sport, eport))) {
char sport[BUFLEN_10 + 1], eport[BUFLEN_10 + 1];
if (2 == (sscanf(buf, "%" STR(BUFLEN_10) "s" "%" STR(BUFLEN_10) "s", sport, eport))) {
app[j].port.start = atoi(sport);
Comment on lines +4279 to 4281
Copy link

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BUFLEN_10 is introduced for the port token buffer sizing, but the sscanf width remains hard-coded as %10s. This is easy to desynchronize (and would reintroduce the overflow that this change is trying to prevent). Consider deriving the width from BUFLEN_10 via a stringified macro, or keep a single shared constant for both buffer size and scan width.

Copilot uses AI. Check for mistakes.
app[j].port.end = atoi(eport);
}
Expand Down
Loading