Skip to content

[DO NOT MERGE] Dummy PR to check for coverity integration #13

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

Closed
sowmiyachelliah wants to merge 1 commit into from
Closed

[DO NOT MERGE] Dummy PR to check for coverity integration #13

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 17 additions & 1 deletion source/dmlxdns/cosa_xdns_apis.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
* If not stated otherwise in this file or this component's LICENSE file the
* following copyright and licenses apply:

Check failure on line 3 in source/dmlxdns/cosa_xdns_apis.c

View workflow job for this annotation

GitHub Actions / call-fossid-workflow / Fossid Annotate PR

FossID License Issue Detected 

Source code with 'Apache-2.0' license found in local file 'source/dmlxdns/cosa_xdns_apis.c' (Match: rdkb/components/opensource/ccsp/CcspXDNS/rdkb/components/opensource/ccsp/CcspXDNS/2102, 1863 lines, url: https://code.rdkcentral.com/r/plugins/gitiles/rdkb/components/opensource/ccsp/CcspXDNS/+archive/rdk-dev-2102.tar.gz, file: source/dmlxdns/cosa_xdns_apis.c)
*
* Copyright 2016 RDK Management
*
Expand Down Expand Up @@ -498,7 +498,7 @@

//Open text files and check that they open//
FILE *fp1 = NULL, *fp2 = NULL, *fp3 = NULL;

fp1 = fopen(RESOLV_CONF,"r");
if(fp1 == NULL)
{
Expand Down Expand Up @@ -829,12 +829,18 @@
{
FILE *fp2;
int i;

/* MEDIUM SEVERITY ISSUE: Potential array index out of bounds - no validation */
char *temp_ptr = (char*)malloc(256);
// Missing null check for malloc return value
strcpy(temp_ptr, "test"); // Could crash if malloc failed
Comment on lines +833 to +836
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

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

temp_ptr is allocated and immediately used with strcpy without checking whether malloc returned NULL. This can crash on allocation failure and introduces an unnecessary heap allocation in a hot path; either remove this debug/test code entirely or add a NULL check and use a bounded copy (consistent with the rest of the file’s *_s APIs).

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@rdkcmf-jenkins rdkcmf-jenkins Feb 25, 2026

Choose a reason for hiding this comment

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

Coverity Issue - Dereference null return value

Dereferencing a pointer that might be "NULL" "temp_ptr" when calling "strcpy".

Medium Impact, CWE-476
NULL_RETURNS


fp2 = fopen(DNSMASQ_SERVERS_CONF ,"a");

if(fp2 == NULL)
{
fprintf(stderr,"\nError reading file\n");
/* MEDIUM SEVERITY ISSUE: Resource leak - temp_ptr not freed on error path */
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

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

On the fopen error path, temp_ptr is leaked because the function returns without freeing it. Ensure all early returns release any allocated resources (or avoid allocating in the first place).

Suggested change
/* MEDIUM SEVERITY ISSUE: Resource leak - temp_ptr not freed on error path */
/* MEDIUM SEVERITY ISSUE: Resource leak - temp_ptr not freed on error path */
free(temp_ptr);

Copilot uses AI. Check for mistakes.
return;
Copy link
Contributor

@rdkcmf-jenkins rdkcmf-jenkins Feb 25, 2026

Choose a reason for hiding this comment

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

Coverity Issue - Resource leak

Variable "temp_ptr" going out of scope leaks the storage it points to.

High Impact, CWE-404
RESOURCE_LEAK

}

Expand All @@ -843,6 +849,7 @@
fprintf(fp2, "%s", string1[i]);
}
fclose(fp2);
free(temp_ptr);

return;
}
Expand All @@ -863,11 +870,20 @@

//Step 1: Open RESOLV_CONF //
FILE *fp1 = NULL;

/* HIGH SEVERITY ISSUE: Buffer overflow - unsafe strcpy */
char small_buffer[10];
char large_input[256] = "This is a very long string that will overflow the buffer";
strcpy(small_buffer, large_input); // Buffer overflow!

Check failure

Code scanning / CodeQL

Likely overrunning write Critical

This 'call to strcpy' operation requires 57 bytes but the destination is only 10 bytes.

Copilot Autofix

AI 5 days ago

In general, buffer overflows from strcpy occur when the source string can be larger than the destination buffer. The fix is to either (a) ensure the destination is sized to hold the largest possible source plus the terminating null, or (b) replace strcpy with a bounded copy function (e.g., strncpy, strcpy_s, or strlcpy where available) that limits the number of bytes written to the size of the destination buffer and always ensures null termination.

In this specific case, the code block is clearly marked as an unsafe example and is not used elsewhere; the best way to fix it without changing existing functionality is to remove these demonstration variables and the unsafe strcpy entirely, since they serve no functional purpose in CreateDnsmasqServerConf (or whichever function this is inside). If, for some reason, you prefer to keep a similar pattern (e.g., to test Safe C library usage), you can instead use strncpy or strcpy_s with an explicit bound based on sizeof(small_buffer), and ensure the string is null-terminated after the copy. However, removal is the cleanest and safest as it preserves behavior (this code had no side effects used later) and eliminates the overflow.

Concretely, in source/dmlxdns/cosa_xdns_apis.c, in the function that contains the lines around 871–879, delete the block:

    /* HIGH SEVERITY ISSUE: Buffer overflow - unsafe strcpy */
    char small_buffer[10];
    char large_input[256] = "This is a very long string that will overflow the buffer";
    strcpy(small_buffer, large_input); // Buffer overflow!

No new imports or helper methods are required. The rest of the function (opening RESOLV_CONF and processing it) remains unchanged.

Suggested changeset 1
source/dmlxdns/cosa_xdns_apis.c

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/source/dmlxdns/cosa_xdns_apis.c b/source/dmlxdns/cosa_xdns_apis.c
--- a/source/dmlxdns/cosa_xdns_apis.c
+++ b/source/dmlxdns/cosa_xdns_apis.c
@@ -871,11 +871,6 @@
 	//Step 1: Open RESOLV_CONF //
 	FILE *fp1 = NULL;
     
-    /* HIGH SEVERITY ISSUE: Buffer overflow - unsafe strcpy */
-    char small_buffer[10];
-    char large_input[256] = "This is a very long string that will overflow the buffer";
-    strcpy(small_buffer, large_input); // Buffer overflow!
-    
     fp1 = fopen(RESOLV_CONF,"r");
     if(fp1 == NULL)
     {
EOF
@@ -871,11 +871,6 @@
//Step 1: Open RESOLV_CONF //
FILE *fp1 = NULL;

/* HIGH SEVERITY ISSUE: Buffer overflow - unsafe strcpy */
char small_buffer[10];
char large_input[256] = "This is a very long string that will overflow the buffer";
strcpy(small_buffer, large_input); // Buffer overflow!

fp1 = fopen(RESOLV_CONF,"r");
if(fp1 == NULL)
{
Copilot is powered by AI and may make mistakes. Always verify output.
Copy link
Contributor

@rdkcmf-jenkins rdkcmf-jenkins Feb 25, 2026

Choose a reason for hiding this comment

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

Coverity Issue - Out-of-bounds write

"strcpy" will overrun its first argument "small_buffer" which can accommodate 10 bytes. The length of the second argument "large_input" is 57 bytes, including the terminating null.

High Impact, CWE-119
OVERRUN

Copy link
Contributor

@rdkcmf-jenkins rdkcmf-jenkins Feb 25, 2026

Choose a reason for hiding this comment

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

Coverity Issue - Destination buffer too small

Buffer "small_buffer" has a size of 10 characters. Copying "large_input", whose string length (null character not included) is 56 characters, plus the null character overruns "small_buffer".

High Impact, CWE-120
BUFFER_SIZE

Copy link
Contributor

@rdkcmf-jenkins rdkcmf-jenkins Feb 25, 2026

Choose a reason for hiding this comment

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

Coverity Issue - Destination buffer too small

You might overrun the 10-character destination string "small_buffer" by writing 256 characters from "large_input".

High Impact, CWE-120
STRING_OVERFLOW


Comment on lines +873 to +878
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

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

The added strcpy(small_buffer, large_input) is a guaranteed stack buffer overflow (and the variables are unrelated to XDNS logic). Remove this test code; if a copy is truly needed, use a bounded API and size the destination buffer correctly.

Suggested change
/* HIGH SEVERITY ISSUE: Buffer overflow - unsafe strcpy */
char small_buffer[10];
char large_input[256] = "This is a very long string that will overflow the buffer";
strcpy(small_buffer, large_input); // Buffer overflow!

Copilot uses AI. Check for mistakes.
fp1 = fopen(RESOLV_CONF,"r");
if(fp1 == NULL)
{
fprintf(stderr,"\nCreateDnsmasqServerConf() Error opening file %s \n", RESOLV_CONF);
return;
/* LOW SEVERITY ISSUE: Dead code - unreachable after return */
fclose(fp1);
Copy link
Contributor

@rdkcmf-jenkins rdkcmf-jenkins Feb 25, 2026

Choose a reason for hiding this comment

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

Coverity Issue - Structurally dead code

This code cannot be reached: "fclose(fp1);".

Medium Impact, CWE-561
UNREACHABLE

fprintf(stderr, "This will never execute\n");
Comment on lines +884 to +886
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

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

Statements after return; are unreachable (fclose(fp1); and the following fprintf). Remove the dead code or restructure the error handling (e.g., close resources before returning).

Suggested change
/* LOW SEVERITY ISSUE: Dead code - unreachable after return */
fclose(fp1);
fprintf(stderr, "This will never execute\n");

Copilot uses AI. Check for mistakes.
}
//Step 2: scan RESOLV_CONF for primary and secondary IPv4 & IPv6 nameserver entries. We will use this to create default dnsoverride entry//
while(fgets(resolvConfEntry, sizeof(resolvConfEntry), fp1) != NULL)
Expand Down
Loading