[rdkdevpilot]

Automated Fix for Coverity Defects

Triggered by: dev-user

Fixed Files

File	Line(s)	CIDs	Checker Types	Description
c_sourcecode/src/main.c	L248	68691	SIZEOF_MISMATCH	Wrong sizeof argument
c_sourcecode/src/utils/prerequisites.c	L72	68702	TAINTED_SCALAR	Untrusted value as argument
c_sourcecode/src/ratelimit/ratelimit.c	L124, L185, L200	68700, 68701, 68694	OVERRUN, TOCTOU	Out-of-bounds read, Time of check time of use
c_sourcecode/src/scanner/scanner.c	L610	68698	OVERRUN	Out-of-bounds access
c_sourcecode/src/archive/archive.c	L265	68699	BUFFER_SIZE	Overlapping buffer in memory copy
c_sourcecode/src/platform/platform.c	L93	68697	UNUSED_VALUE	Unused value
c_sourcecode/src/utils/cleanup_batch.c	L249, L531	68692, 68695	RW.EXP_SEMICOLON, TOCTOU	Parse recovery warning, Time of check time of use

Fix Summaries

c_sourcecode/src/main.c

The fix correctly addresses the SIZEOF_MISMATCH defect by changing sizeof(archive) to sizeof(*archive). The original code used sizeof on the pointer variable 'archive' (which would be the size of a pointer, typically 4 or 8 bytes), while the corrected code uses sizeof(*archive) to get the size of the archive_info_t structure that the pointer points to. This ensures memset correctly zeroes out the entire allocated array of archive_info_t structures. The fix is minimal, correct, and does not introduce any new defects or break existing logic.

c_sourcecode/src/utils/prerequisites.c

The fix correctly addresses the TAINTED_SCALAR defect by adding a uptime_val >= 0 guard to the condition on line 66. The value uptime_val is derived from fscanf reading an external file (/proc/uptime or /opt/uptime), making it tainted. Without the guard, a negative uptime_val (e.g., from a malformed file) would pass the < FOUR_EIGHTY_SECS check and cause sleep_time = FOUR_EIGHTY_SECS - uptime_val to produce a very large positive integer, potentially causing an extremely long sleep or integer overflow when passed to sleep(). The added && uptime_val >= 0 check clamps the tainted input to a valid range before use. The fix is minimal (one condition change), does not introduce new defects, preserves all existing logic and branches, and correctly handles the error path (the block is simply skipped for negative values). The whitespace-only changes inside the block are cosmetic and do not affect behavior.

c_sourcecode/src/ratelimit/ratelimit.c

The fix correctly addresses the TOCTOU defect in is_recovery_time_reached() by removing the stat() call that checked file existence before fopen(), and instead relying on fopen() itself to atomically report file absence. This eliminates the window between the stat() check and the subsequent fopen() where the file could be created or deleted by another process. The struct stat variable is also correctly removed since it is no longer used. The two loop condition reorderings (checking i < sizeof(...) before dereferencing the array) are correct safety improvements that prevent potential out-of-bounds reads if the buffer is not null-terminated. All changes are minimal, targeted, and do not break any existing logic or introduce new defects.

c_sourcecode/src/scanner/scanner.c

The fix correctly changes the cap on pnamelen from sizeof(ProcessName) (512) to sizeof(containerName) (256). Since pnamelen is computed as us - containerName where us is a pointer into containerName[256], the correct upper bound is sizeof(containerName) - 1 = 255. The original check against sizeof(ProcessName) (512) was too permissive and could allow pnamelen to exceed the source buffer size in the memcpy. The fix is minimal, correct, and does not break any existing logic.

c_sourcecode/src/archive/archive.c

The fix correctly replaces strcpy(tmp + 1, tmp + pattern_len) with memmove(tmp + 1, tmp + pattern_len, strlen(tmp + pattern_len) + 1). The original code used strcpy on overlapping memory regions (source tmp + pattern_len and destination tmp + 1 both point into the same buffer new_dump_name, with pattern_len = 5, so the source starts 4 bytes ahead of the destination — clearly overlapping). strcpy has undefined behavior on overlapping buffers. memmove is the correct replacement as it explicitly handles overlapping memory regions safely. The byte count strlen(tmp + pattern_len) + 1 correctly copies the tail of the string including the null terminator. The fix is minimal (single line change), does not introduce any new defects, preserves the existing loop logic, and directly addresses the Coverity BUFFER_SIZE / overlapping buffer defect.

c_sourcecode/src/platform/platform.c

The fix correctly addresses the UNUSED_VALUE defect by removing the unused assignment at line 93. The original code assigned the return value of stripinvalidchar() to variable 'i' inside the if block (line 93), but this value was immediately overwritten by another call to stripinvalidchar() at line 96, making the first assignment unused. The fix removes the redundant assignment while preserving the call to stripinvalidchar() at line 96, which is used in the subsequent condition check at line 100. The logic flow remains intact: fgets() is still called, the file is closed, stripinvalidchar() processes the buffer once, and the result is properly used. No new defects are introduced.

c_sourcecode/src/utils/cleanup_batch.c

The fix correctly addresses the defect by changing stat() to lstat() on line 531. This change prevents following symbolic links when checking file status in remove_pending_dumps(), which is the appropriate behavior for a recursive directory cleanup function. The fix is minimal (single function call change), preserves all existing logic, and aligns with the pattern already used in walk_dir_recursive() at line 186. No new defects are introduced, and the change is functionally correct for handling symbolic links safely during directory traversal.

[Copilot]

🟡 Changes recommended

Uptime conversion remains unsafe, and the cleanup traversal still contains an exploitable TOCTOU race.

3 open findings

• TOCTOU enables symlink traversal outside working directory · New
• Handle fgets failure instead of leaving an empty branch · New
• Validate parsed double before conversion to int · New

What changed in this PR

Addresses Coverity findings across crash processing, rate limiting, cleanup, and platform utilities.

Changes:

• Corrects buffer sizing and overlapping copies.
• Reorders bounds checks and removes redundant file existence checks.
• Adds input validation and symlink-aware cleanup.

File	Description
c_sourcecode/​src/​main.c	Corrects archive initialization size.
c_sourcecode/​src/​utils/​prerequisites.c	Adds uptime range validation.
c_sourcecode/​src/​ratelimit/​ratelimit.c	Reorders bounds checks and removes pre-open stat.
c_sourcecode/​src/​scanner/​scanner.c	Corrects container-name copy bound.
c_sourcecode/​src/​archive/​archive.c	Uses overlap-safe memmove.
c_sourcecode/​src/​platform/​platform.c	Removes a redundant sanitization call.
c_sourcecode/​src/​utils/​cleanup_batch.c	Uses lstat during recursive cleanup.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

The previously reported cleanup safety concern needs human validation because its resolution could not be confirmed from the available context.

3 open issues

• TOCTOU enables symlink traversal outside working directory
• Validate parsed double before conversion to int
• Handle fgets failure instead of leaving an empty branch

_{Review effort: Balanced}