RDKB-61953 RDKB-62249 : Fixing coverity issues for hotspot repo by bunnam988 · Pull Request #23 · rdkcentral/hotspot

bunnam988 · 2025-10-28T09:34:31Z

RDKB-61953 RDKB-62249 : Fixing coverity issues for hotspot repo

Reason for change: Resolving coverity issues on hotspot repo
Test Procedure: Ensure no regression on functionality
Risks: medium
Priority: P1

parse_if_inet6 coverity

Copilot

Pull Request Overview

This PR focuses on improving code security and robustness through string safety enhancements, memory management fixes, and bounds checking improvements across multiple hotspot-related components.

Key Changes:

Enhanced string copy operations with explicit null-termination to prevent buffer overflows
Added proper error handling for string concatenation operations
Improved DHCP option parsing with better bounds checking
Fixed memory leaks and added malloc failure handling
Replaced weak random number generation with cryptographically secure alternative

Reviewed Changes

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

Show a summary per file

File	Description
source/hotspotfd/ssp_messagebus_interface.c	Simplified return logic by removing unused variable
source/hotspotfd/hotspotfd.c	Added error checking for strcat_s and improved strncpy null-termination
source/hotspotfd/dhcpsnooper.c	Fixed variable scope issue by moving declaration to function start
source/XfinityTestAgent/tunnelcheck.c	Enhanced string safety, improved DHCP parsing, replaced random() with secure alternative, added memory checks
source/HotspotApi/HotspotApi.c	Fixed memory leak and simplified control flow

Comments suppressed due to low confidence (2)

source/XfinityTestAgent/tunnelcheck.c:889

Accessing offer_packet->options[itr1] without verifying that itr1 is within bounds after the two increments at lines 883-884. Add a bounds check before this access: if (itr1 >= MAX_DHCP_OPTIONS_LENGTH) break;

            return offer_packet->options[itr1];

source/XfinityTestAgent/tunnelcheck.c:933

The memcpy reads sizeof(struct in_addr) bytes starting at offer_packet->options[itr1] without verifying there are enough bytes remaining in the buffer. Add a bounds check before this operation: if (itr1 + sizeof(struct in_addr) > MAX_DHCP_OPTIONS_LENGTH) break;

            memcpy(&server_ip, &offer_packet->options[itr1], sizeof(struct in_addr));

💡 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 3 comments.

Comments suppressed due to low confidence (2)

source/XfinityTestAgent/tunnelcheck.c:895

After the bounds check on line 874, this loop can still cause itr1 to exceed MAX_DHCP_OPTIONS_LENGTH. If option_length is large enough, incrementing itr1 by option_length could overflow the bounds. The loop should include a check to ensure itr1 doesn't exceed the array bounds: for(itr2=0;itr2<(int)option_length && itr1<MAX_DHCP_OPTIONS_LENGTH;itr2++,itr1++);

            for(itr2=0;itr2<(int)option_length;itr2++,itr1++);

source/XfinityTestAgent/tunnelcheck.c:939

Similar to the issue in dhcp_msg_type, this loop can cause itr1 to exceed MAX_DHCP_OPTIONS_LENGTH if option_length is large. The loop should include a bounds check: for(itr2=0;itr2<(int)option_length && itr1<MAX_DHCP_OPTIONS_LENGTH;itr2++,itr1++);

            for(itr2=0;itr2<(int)option_length;itr2++,itr1++);

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

Copilot · 2025-11-12T07:29:51Z

@bunnam988 I've opened a new pull request, #24, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot · 2025-11-12T07:30:15Z

@bunnam988 I've opened a new pull request, #25, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot · 2025-11-12T07:31:11Z

@bunnam988 I've opened a new pull request, #26, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot

Pull Request Overview

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

💡 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 2 comments.

💡 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 5 comments.

💡 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 2 comments.

💡 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

Pull Request Overview

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

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

apattu200 · 2025-11-19T09:15:10Z

To proceed with merge, please provide below informations.

Is this a Bug or a User Story (US)?
If it is a User Story:
- Please list all dependent PRs from other components, if any.
- The commit message must include both the User Story ticket and the Subtask ticket.
- All changes related to the User Story must be squashed and merged in a single commit.
- Do not raise pull requests for partial User Story changes.
- Has the code development for the User Story been completed?
  - If yes, please share the Gerrit topic or list all dependent PRs across components, including any meta-layer changes.
Is there a validation log available in the Jira ticket for verifying builds with the updated generic-srcrev.inc across all platforms?
- If yes, please share the links to the validation comments.

GoutamD2905 · 2025-11-20T06:14:22Z

Before merging, I have a few questions:

Is this a Bug or a User Story (US)?
If it is a User Story:
- Have all dependent PRs from other components been listed (if any)?
- Does the commit message include both the User Story ticket and the Subtask ticket?
- Will be all changes related to the User Story squashed and merged in a single commit?
- Has the PR been raised only after completing all changes for the User Story (no partial changes)?
- Has code development for the User Story been completed?
  - If yes, has the Gerrit topic or list of all dependent PRs across components (including meta-layer changes) been shared?
Is there a validation log available in the Jira ticket for verifying builds with the updated generic-srcrev.inc across all platforms?
- If yes, have the links to validation comments been shared?

bunnam988 · 2025-11-20T06:19:33Z

Before merging, I have a few questions:

Is this a Bug or a User Story (US)? yes it a user story

If it is a User Story:

Have all dependent PRs from other components been listed (if any)? No

Does the commit message include both the User Story ticket and the Subtask ticket? yes

Will be all changes related to the User Story squashed and merged in a single commit? yes

Has the PR been raised only after completing all changes for the User Story (no partial changes)? yes

Has code development for the User Story been completed? yes

If yes, has the Gerrit topic or list of all dependent PRs across components (including meta-layer changes) been shared? No dependent changes

Is there a validation log available in the Jira ticket for verifying builds with the updated generic-srcrev.inc across all platforms? yes attached in ticket

If yes, have the links to validation comments been shared? yes

Coverity fixes for hotspot repo

75d154a

bunnam988 requested review from a team as code owners October 28, 2025 09:34

Update tunnelcheck.c

6c27bf8

parse_if_inet6 coverity

Copilot AI review requested due to automatic review settings November 4, 2025 10:53

Copilot AI reviewed Nov 4, 2025

View reviewed changes

Comment thread source/XfinityTestAgent/tunnelcheck.c Outdated

Comment thread source/XfinityTestAgent/tunnelcheck.c Outdated

Comment thread source/XfinityTestAgent/tunnelcheck.c Outdated

Copilot AI review requested due to automatic review settings November 12, 2025 06:44

bunnam988 force-pushed the feature/coverity-fixes branch from 00c8690 to 6c27bf8 Compare November 12, 2025 06:44

Copilot started reviewing on behalf of bunnam988 November 12, 2025 06:44 View session

Copilot finished reviewing on behalf of bunnam988 November 12, 2025 06:46

Copilot AI reviewed Nov 12, 2025

View reviewed changes

Comment thread source/XfinityTestAgent/tunnelcheck.c Outdated

Comment thread source/XfinityTestAgent/tunnelcheck.c Outdated

Copilot AI mentioned this pull request Nov 12, 2025

Add missing bounds checks in dhcp_msg_type to prevent buffer overrun #24

Closed

Copilot AI mentioned this pull request Nov 12, 2025

Add bounds checks for DHCP option parsing in get_dhcp_server_identifier #25

Closed

Copilot AI review requested due to automatic review settings November 12, 2025 07:30

Copilot started reviewing on behalf of bunnam988 November 12, 2025 07:30 View session

Copilot AI mentioned this pull request Nov 12, 2025

Remove redundant srand() calls after switching to /dev/urandom #26

Closed

Copilot finished reviewing on behalf of bunnam988 November 12, 2025 07:32

Copilot AI reviewed Nov 12, 2025

View reviewed changes

Copilot AI review requested due to automatic review settings November 12, 2025 07:34

Copilot started reviewing on behalf of bunnam988 November 12, 2025 07:34 View session

Copilot finished reviewing on behalf of bunnam988 November 12, 2025 07:36

Copilot AI reviewed Nov 12, 2025

View reviewed changes

Comment thread source/XfinityTestAgent/tunnelcheck.c Outdated

Comment thread source/XfinityTestAgent/tunnelcheck.c Outdated

Copilot AI review requested due to automatic review settings November 12, 2025 09:57

Copilot started reviewing on behalf of bunnam988 November 12, 2025 09:57 View session

Copilot finished reviewing on behalf of bunnam988 November 12, 2025 09:58

Copilot AI reviewed Nov 12, 2025

View reviewed changes

Comment thread source/XfinityTestAgent/tunnelcheck.c Outdated

Copilot AI review requested due to automatic review settings November 12, 2025 10:04

Copilot started reviewing on behalf of bunnam988 November 12, 2025 10:19 View session

Copilot finished reviewing on behalf of bunnam988 November 12, 2025 10:21

Copilot AI reviewed Nov 12, 2025

View reviewed changes

Comment thread source/XfinityTestAgent/tunnelcheck.c Outdated

Copilot AI review requested due to automatic review settings November 12, 2025 10:24

Copilot started reviewing on behalf of bunnam988 November 12, 2025 10:24 View session

Copilot finished reviewing on behalf of bunnam988 November 12, 2025 10:28

Copilot AI reviewed Nov 12, 2025

View reviewed changes

Comment thread source/XfinityTestAgent/tunnelcheck.c Outdated

bunnam988 force-pushed the feature/coverity-fixes branch from b6bebb0 to 6c27bf8 Compare November 13, 2025 07:01

Update tunnelcheck.c

d5e5bd0

Copilot AI review requested due to automatic review settings November 17, 2025 09:06

rdkcentral deleted a comment from Copilot AI Nov 17, 2025

Copilot started reviewing on behalf of bunnam988 November 17, 2025 09:06 View session

rdkcentral deleted a comment from Copilot AI Nov 17, 2025

Copilot finished reviewing on behalf of bunnam988 November 17, 2025 09:08

Copilot AI reviewed Nov 17, 2025

View reviewed changes

Comment thread source/XfinityTestAgent/tunnelcheck.c Outdated

Comment thread source/XfinityTestAgent/tunnelcheck.c Outdated

Comment thread source/XfinityTestAgent/tunnelcheck.c Outdated

Comment thread source/XfinityTestAgent/tunnelcheck.c Outdated

Update tunnelcheck.c

949191d

bunnam988 changed the title ~~RDKB-62249 : Fixing coverity issues for hotspot repo~~ RDKB-61953 RDKB-62249 : Fixing coverity issues for hotspot repo Nov 18, 2025

apattu200 approved these changes Nov 19, 2025

View reviewed changes

GoutamD2905 merged commit bdeaa40 into develop Nov 20, 2025
7 of 8 checks passed

github-actions Bot locked and limited conversation to collaborators Nov 20, 2025

Conversation

bunnam988 commented Oct 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Key Changes:

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI commented Nov 12, 2025

Uh oh!

Copilot AI commented Nov 12, 2025

Uh oh!

Copilot AI commented Nov 12, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

apattu200 commented Nov 19, 2025

Uh oh!

GoutamD2905 commented Nov 20, 2025

Before merging, I have a few questions:

Uh oh!

bunnam988 commented Nov 20, 2025

Before merging, I have a few questions:

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bunnam988 commented Oct 28, 2025 •

edited

Loading