RDKB-64588: High impact coverity issues in utopia by abhishek-kumaracee2 · Pull Request #293 · rdkcentral/utopia

abhishek-kumaracee2 · 2026-04-22T19:15:48Z

Reason for change: Fix high impact coverity issues
Test Procedure: NA
Risks: Low
Priority: P1
Signed-off-by: abhishek_kumaracee2@comcast.com

Copilot

Pull request overview

This PR addresses high-impact Coverity findings across the “utopia” codebase by tightening string parsing and improving memory cleanup to prevent buffer overruns and leaks.

Changes:

Fix potential stack buffer overflows by right-sizing local buffers used with bounded sscanf format widths.
Prevent a memory leak in gen_dibbler_conf() by freeing dhcpv6s_pool_cfg.opts on early-continue paths.
Reduce sscanf field width in ulog_GetProcId() to match the actual destination buffer size.

Reviewed changes

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

File	Description
source/utapi/lib/utapi.c	Adjusts port-range token buffers to correctly accommodate `sscanf` bounds plus NUL terminator.
source/ulog/ulog.c	Fixes potential overflow by bounding `%s` to the actual `ULOG_STR_SIZE` buffer.
source/service_ipv6/service_ipv6.c	Frees DHCPv6 pool options before continuing when a pool is disabled/invalid to avoid leaks.
source/firewall/firewall.c	Enlarges token/IP/MAC buffers by 1 byte to safely fit bounded `sscanf` reads plus NUL terminator.

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

Copilot · 2026-04-22T19:17:57Z

+    sscanf(buf, "%*d (%63s", str);

    len = strlen(str);


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.

apattu200

Looks ok to me.

@snayak002c Can you also please review once

snayak002c · 2026-04-23T17:40:24Z

   // FIREWALL_DEBUG("Entering *make_substitutions\n");         
    while (in_str_p < in_str_end && out_str_p < out_str_end) {
-       char token[50];
+       char token[51];


Avoid hardcoding the buffer size in the format string, use a macro-based approach instead
char sport[TOKEN_MAX_LEN + 1]

snayak002c · 2026-04-23T17:41:38Z

-            char ip[20];
-            char mac[20];
+            char ip[21];
+            char mac[21];


same as above comment

snayak002c · 2026-04-23T17:43:02Z


                Utopia_GetIndexed2(ctx, UtopiaValue_IAP_BlockPortRange, index, i+1, buf, sizeof(buf));
-                char sport[10], eport[10];
+                char sport[11], eport[11];


same as above comments, avoid hard coding of the values.

Reason for change: Fix high impact coverity issues Test Procedure: NA Risks: Low Priority: P1 Signed-off-by: abhishek_kumaracee2@comcast.com

Copilot

Pull request overview

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

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

Copilot · 2026-04-23T18:43:30Z

+            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);


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 · 2026-04-23T18:43:31Z

+                char sport[BUFLEN_10 + 1], eport[BUFLEN_10 + 1];
                if (2 == (sscanf(buf, "%10s %10s", sport, eport))) {
                    app[j].port.start = atoi(sport);


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 · 2026-04-23T18:43:31Z

+            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);


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 · 2026-04-23T18:43:31Z

+    sscanf(buf, "%*d (%63s", str);



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 · 2026-04-23T18:43:31Z

+    sscanf(buf, "%*d (%63s", str);

    len = strlen(str);


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 · 2026-04-23T18:43:31Z

+       char token[TOKEN_MAX_LEN + 1];
       if ('$' == *in_str_p) {
          sscanf(in_str_p, "%50s", token); 
          in_str_p += strlen(token);


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.

Need to handle

Reason for change: Fix high impact coverity issues Test Procedure: NA Risks: Low Priority: P1 Signed-off-by: abhishek_kumaracee2@comcast.com

… into topic/RDKB-64588

Reason for change: Fix high impact coverity issues Test Procedure: NA Risks: Low Priority: P1 Signed-off-by: abhishek_kumaracee2@comcast.com

Copilot

Pull request overview

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

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

Copilot AI review requested due to automatic review settings April 22, 2026 19:15

abhishek-kumaracee2 requested review from a team as code owners April 22, 2026 19:15

Copilot started reviewing on behalf of abhishek-kumaracee2 April 22, 2026 19:16 View session

Copilot AI reviewed Apr 22, 2026

View reviewed changes

apattu200 reviewed Apr 23, 2026

View reviewed changes

abhishek-kumaracee2 force-pushed the topic/RDKB-64588 branch from a073c16 to 08624da Compare April 23, 2026 17:28

snayak002c requested changes Apr 23, 2026

View reviewed changes

RDKB-64588: High impact coverity issues in utopia

1f644f8

Reason for change: Fix high impact coverity issues Test Procedure: NA Risks: Low Priority: P1 Signed-off-by: abhishek_kumaracee2@comcast.com

abhishek-kumaracee2 force-pushed the topic/RDKB-64588 branch from 08624da to 1f644f8 Compare April 23, 2026 18:39

Copilot AI review requested due to automatic review settings April 23, 2026 18:39

Copilot started reviewing on behalf of abhishek-kumaracee2 April 23, 2026 18:40 View session

Copilot AI reviewed Apr 23, 2026

View reviewed changes

snayak002c previously approved these changes Apr 23, 2026

View reviewed changes

Merge branch 'develop' into topic/RDKB-64588

8717c63

abhishek-kumaracee2 added 2 commits April 23, 2026 19:24

RDKB-64588: High impact coverity issues in utopia

286632e

Reason for change: Fix high impact coverity issues Test Procedure: NA Risks: Low Priority: P1 Signed-off-by: abhishek_kumaracee2@comcast.com

Merge branch 'topic/RDKB-64588' of https://github.com/rdkcentral/utopia…

7923245

… into topic/RDKB-64588

Copilot AI review requested due to automatic review settings April 23, 2026 19:34

Copilot started reviewing on behalf of abhishek-kumaracee2 April 23, 2026 19:34 View session

RDKB-64588: High impact coverity issues in utopia

1eb7081

Reason for change: Fix high impact coverity issues Test Procedure: NA Risks: Low Priority: P1 Signed-off-by: abhishek_kumaracee2@comcast.com

Copilot AI reviewed Apr 23, 2026

View reviewed changes

Conversation

abhishek-kumaracee2 commented Apr 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

apattu200 left a comment

Choose a reason for hiding this comment

Uh oh!

snayak002c Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

snayak002c Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

snayak002c Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants