RDK-60607: (cid:559820) Overflowed integer argument rtConnectedClient…

[dshett549]

ReasonForChange: (cid:559820) Overflowed integer argument rtConnectedClient_Read
Priority: P1
Signed-off-by: dshett549 [DEEPTHICHANDRASHEKAR_SHETTY@comcast.com]

[Copilot]

Pull request overview

This pull request addresses an integer overflow vulnerability (RDK-60607) in the rtConnectedClient_Read function by changing the data types used for tracking buffer sizes from signed 32-bit integers to unsigned 64-bit integers. This prevents overflow issues when handling large message payloads.

Changes:

• Modified struct fields in rtConnectedClient from int to uint64_t for bytes_read, bytes_to_read, and read_buffer_capacity
• Updated local variable types in rtConnectedClient_Read functions to match the new types (size_t for bytes_to_read, uint64_t for incoming_data_size)
• Added <inttypes.h> include for PRIu64 format specifier
• Updated log message format specifiers from %d to %" PRIu64 " to correctly print uint64_t values

Reviewed changes

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

File	Description
src/rtmessage/rtrouteBase.h	Changed three struct member types from int to uint64_t (bytes_read, bytes_to_read, read_buffer_capacity)
src/rtmessage/rtrouted.c	Updated local variable types, added inttypes.h include, and modified format specifiers in log messages
src/rtmessage/rtrouteBase.c	Updated local variable types, added inttypes.h include, and modified format specifiers in log messages

---

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

[gladish]

uint64_t                       bytes_read;
uint64_t                       bytes_to_read;
uint64_t                       read_buffer_capacity;

I think it's better to use size_t for these, which result in fewer casts between size_t and uint64_t.

size_t bytes_read;
size_t bytes_to_read;
size_t read_buffer_capacity;

[Copilot]

Pull request overview

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

---

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

[karuna2git]

@gladish
defining to size_t will create confusion to the wired-protocol on the message header becoz different device will have length for size_t (32-bit & 64 bit). But if we defined it to be uint64_t, this will always keep fixed length for rbus message header.

Thats the main reason behind this change.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 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]

The function _rtConnection_ReadAndDropBytes expects an 'unsigned int' parameter (as defined in rtConnection.h:270), but clnt->header.payload_length is of type uint32_t. While these types are typically the same size on most platforms, this assumes that unsigned int is at least 32 bits. On platforms where unsigned int is 16 bits, this could cause truncation. Consider either casting with an explicit check, or updating the _rtConnection_ReadAndDropBytes signature to accept uint32_t for better type safety and portability.

[Copilot]

The function _rtConnection_ReadAndDropBytes expects an 'unsigned int' parameter (as defined in rtConnection.h:270), but clnt->header.payload_length is of type uint32_t. While these types are typically the same size on most platforms, this assumes that unsigned int is at least 32 bits. On platforms where unsigned int is 16 bits, this could cause truncation. Consider either casting with an explicit check, or updating the _rtConnection_ReadAndDropBytes signature to accept uint32_t for better type safety and portability.