From e5bfb76e262ba3f1c972a9c2f7ae05035853dc3a Mon Sep 17 00:00:00 2001 From: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com> Date: Thu, 10 Sep 2026 19:15:51 +0100 Subject: [PATCH] Fix #484, reject invalid move-target buffers --- fsw/src/cf_cfdp.c | 6 ++++++ fsw/src/cf_cfdp.h | 4 ++-- unit-test/cf_cfdp_tests.c | 24 ++++++++++++++++++++++++ 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/fsw/src/cf_cfdp.c b/fsw/src/cf_cfdp.c index 8c216dc8..699b995c 100644 --- a/fsw/src/cf_cfdp.c +++ b/fsw/src/cf_cfdp.c @@ -2459,6 +2459,12 @@ const char *CF_CFDP_GetMoveTarget(const char *dest_dir, const char *subject_file const char *filename; int dest_path_len; + /* A returned destination must have space for its string terminator. */ + if (dest_buf == NULL || dest_size == 0) + { + return NULL; + } + result = NULL; if (dest_dir != NULL && dest_dir[0] != 0) { diff --git a/fsw/src/cf_cfdp.h b/fsw/src/cf_cfdp.h index c1eb2d9b..36823313 100644 --- a/fsw/src/cf_cfdp.h +++ b/fsw/src/cf_cfdp.h @@ -840,8 +840,8 @@ CF_CListTraverse_Status_t CF_CFDP_DoTick(CF_CListNode_t *node, void *context); * @param dest_buf Buffer to store result * @param dest_size Size of result buffer * - * @retval NULL if the result is not valid (i.e. dest_dir not set) - * @retval dest_buf if result is valid + * @retval NULL if dest_dir is not set, dest_buf is NULL, or dest_size is zero + * @retval dest_buf if result is a valid NUL-terminated string */ const char *CF_CFDP_GetMoveTarget(const char *dest_dir, const char *subject_file, char *dest_buf, size_t dest_size); diff --git a/unit-test/cf_cfdp_tests.c b/unit-test/cf_cfdp_tests.c index 68504049..32d74d82 100644 --- a/unit-test/cf_cfdp_tests.c +++ b/unit-test/cf_cfdp_tests.c @@ -1920,10 +1920,34 @@ void Test_CF_CFDP_GetMoveTarget(void) */ char FileNameBuf[6]; + /* No destination capacity must not return an uninitialized string. */ + memset(FileNameBuf, 'X', sizeof(FileNameBuf)); + UtAssert_NULL(CF_CFDP_GetMoveTarget("d", "ut", FileNameBuf, 0)); + UtAssert_True(memcmp(FileNameBuf, "XXXXXX", sizeof(FileNameBuf)) == 0, "Zero-capacity output remains untouched"); + /* nominal, no dest dir */ UtAssert_NULL(CF_CFDP_GetMoveTarget(NULL, "ut", FileNameBuf, sizeof(FileNameBuf))); UtAssert_NULL(CF_CFDP_GetMoveTarget("", "ut", FileNameBuf, sizeof(FileNameBuf))); + /* Missing storage is invalid regardless of its reported capacity. */ + UtAssert_NULL(CF_CFDP_GetMoveTarget("d", "ut", NULL, 0)); + UtAssert_NULL(CF_CFDP_GetMoveTarget("d", "ut", NULL, sizeof(FileNameBuf))); + UtAssert_STUB_COUNT(CFE_EVS_SendEvent, 0); + + /* One byte holds the terminator, and must not touch the following bytes. */ + UtAssert_True(CF_CFDP_GetMoveTarget("d", "ut", FileNameBuf, 1) == FileNameBuf, + "One-byte destination returns the supplied buffer"); + UtAssert_ZERO(FileNameBuf[0]); + UtAssert_True(memcmp(FileNameBuf + 1, "XXXXX", sizeof(FileNameBuf) - 1) == 0, "Only the available byte is written"); + UtAssert_STUB_COUNT(CFE_EVS_SendEvent, 0); + + /* An exact fit preserves the complete path and the following byte. */ + UtAssert_True(CF_CFDP_GetMoveTarget("d", "ut", FileNameBuf, 5) == FileNameBuf, + "Exact-fit destination returns the supplied buffer"); + UtAssert_STRINGBUF_EQ(FileNameBuf, 5, "d/ut", -1); + UtAssert_INT32_EQ(FileNameBuf[5], 'X'); + UtAssert_STUB_COUNT(CFE_EVS_SendEvent, 0); + /* nominal with dest dir */ UtAssert_NOT_NULL(CF_CFDP_GetMoveTarget("d", "ut", FileNameBuf, sizeof(FileNameBuf))); UtAssert_STRINGBUF_EQ(FileNameBuf, sizeof(FileNameBuf), "d/ut", -1);