Checklist (Please check before submitting)
Describe the bug
CF_CFDP_GetMoveTarget() may return dest_buf even when dest_size == 0.
In that case, the function calls snprintf(dest_buf, dest_size, "%s/%s", ...), but with a zero-sized destination buffer no bytes are written, so dest_buf is not guaranteed to contain a terminating '\0'. The function still returns dest_buf as though it were a valid C string.
If a caller later treats the returned pointer as a normal string (for example with %s, strlen, logging helpers, or path/file APIs), this can trigger undefined behavior and may result in an out-of-bounds read.
This appears to be a boundary-condition / robustness issue with potential security relevance, rather than a confirmed high-severity vulnerability by itself.
To Reproduce
- Use logic equivalent to
CF_CFDP_GetMoveTarget() with a non-empty dest_dir.
- Pass a valid
dest_buf pointer with dest_size == 0.
- Observe that the function returns
dest_buf.
- Observe that no bytes were written to the buffer, so it is not guaranteed to be a NUL-terminated C string.
- Pass the returned pointer to a string-consuming API such as
printf("%s", ...), strlen(), or similar.
- Depending on surrounding memory contents and runtime environment, this can trigger undefined behavior and may read past the object boundary.
A minimal reproduction is shown below:
#include <stdio.h>
#include <string.h>
#include <stddef.h>
/* minimal reproduction of logic in CF_CFDP_GetMoveTarget() */
const char *GetMoveTarget(const char *dest_dir, const char *subject_file, char *dest_buf, size_t dest_size)
{
const char *result;
const char *filename;
int dest_path_len;
result = NULL;
if (dest_dir != NULL && dest_dir[0] != 0)
{
filename = strrchr(subject_file, '/');
if (filename == NULL)
{
filename = subject_file;
}
else
{
++filename;
}
dest_path_len = snprintf(dest_buf, dest_size, "%s/%s", dest_dir, filename);
if (dest_path_len >= (int)dest_size && dest_size > 2)
{
dest_buf[dest_size - 2] = '$';
}
result = dest_buf;
}
return result;
}
int main(void)
{
char buf0[1] = { 'X' };
const char *r0 = GetMoveTarget("d", "longname", buf0, 0);
/* r0 is returned, but buf0 was not written and is not guaranteed
to be a valid NUL-terminated string */
printf("returned=%p\n", (void *)r0);
printf("bounded print: %.8s\n", r0); /* still undefined behavior */
return 0;
}
Expected behavior
The function should not return a pointer that is not guaranteed to reference a valid NUL-terminated C string.
Safer behavior would be one of:
- return NULL when dest_buf == NULL or dest_size == 0, or
- ensure that any non-NULL returned buffer is always initialized as a valid empty string when dest_size > 0.
For example, a defensive fix could:
- return NULL if dest_buf == NULL or dest_size == 0
- initialize dest_buf[0] = '\0' before formatting when dest_size
Code Analysis
Location 1: fsw/src/cf_cfdp.c (lines 2209-2242)
CF_CFDP_GetMoveTarget() returns dest_buf even when dest_size is 0, which may yield a non-NUL-terminated string
Suggested Fix:
- const char *CF_CFDP_GetMoveTarget(const char *dest_dir, const char *subject_file, char *dest_buf, size_t dest_size)
- {
- const char *result;
- const char *filename;
- int dest_path_len;
-
- result = NULL;
- if (dest_dir != NULL && dest_dir[0] != 0)
- {
- filename = strrchr(subject_file, '/');
- if (filename == NULL)
- {
- filename = subject_file; /* not in a dir */
- }
- else
- {
- ++filename;
- }
-
- dest_path_len = snprintf(dest_buf, dest_size, "%s/%s", dest_dir, filename);
- if (dest_path_len >= dest_size && dest_size > 2)
- {
- /* Mark character before zero terminator to indicate truncation */
- dest_buf[dest_size - 2] = CF_FILENAME_TRUNCATED;
-
- /* Send event describing that the path would be truncated */
- CFE_EVS_SendEvent(CF_EID_INF_CFDP_BUF_EXCEED, CFE_EVS_EventType_INFORMATION,
- "CF: destination has been truncated to %s", dest_buf);
- }
-
- result = dest_buf;
- }
-
- return result;
- }
+ const char *CF_CFDP_GetMoveTarget(const char *dest_dir, const char *subject_file, char *dest_buf, size_t dest_size)
+ {
+ const char *result;
+ const char *filename;
+ int dest_path_len;
+
+ result = NULL;
+
+ /* dest_buf must be a writable buffer with space for a NUL terminator */
+ if (dest_buf == NULL || dest_size == 0)
+ {
+ return NULL;
+ }
+
+ /* Ensure a safe empty string regardless of snprintf outcome */
+ dest_buf[0] = 0;
+
+ if (dest_dir != NULL && dest_dir[0] != 0)
+ {
+ filename = strrchr(subject_file, '/');
+ if (filename == NULL)
+ {
+ filename = subject_file; /* not in a dir */
+ }
+ else
+ {
+ ++filename;
+ }
+
+ dest_path_len = snprintf(dest_buf, dest_size, "%s/%s", dest_dir, filename);
+ if (dest_path_len >= (int)dest_size && dest_size > 2)
+ {
+ /* Mark character before zero terminator to indicate truncation */
+ dest_buf[dest_size - 2] = CF_FILENAME_TRUNCATED;
+
+ /* Send event describing that the path would be truncated */
+ CFE_EVS_SendEvent(CF_EID_INF_CFDP_BUF_EXCEED, CFE_EVS_EventType_INFORMATION,
+ "CF: destination has been truncated to %s", dest_buf);
+ }
+
+ result = dest_buf;
+ }
+
+ return result;
+ }
Checklist (Please check before submitting)
Describe the bug
CF_CFDP_GetMoveTarget()may returndest_bufeven whendest_size == 0.In that case, the function calls
snprintf(dest_buf, dest_size, "%s/%s", ...), but with a zero-sized destination buffer no bytes are written, sodest_bufis not guaranteed to contain a terminating'\0'. The function still returnsdest_bufas though it were a valid C string.If a caller later treats the returned pointer as a normal string (for example with
%s,strlen, logging helpers, or path/file APIs), this can trigger undefined behavior and may result in an out-of-bounds read.This appears to be a boundary-condition / robustness issue with potential security relevance, rather than a confirmed high-severity vulnerability by itself.
To Reproduce
CF_CFDP_GetMoveTarget()with a non-emptydest_dir.dest_bufpointer withdest_size == 0.dest_buf.printf("%s", ...),strlen(), or similar.A minimal reproduction is shown below:
Expected behavior
The function should not return a pointer that is not guaranteed to reference a valid NUL-terminated C string.
Safer behavior would be one of:
For example, a defensive fix could:
Code Analysis
Location 1:
fsw/src/cf_cfdp.c(lines 2209-2242)CF_CFDP_GetMoveTarget() returns dest_buf even when dest_size is 0, which may yield a non-NUL-terminated string
Suggested Fix: