From 305793e69ed8a318528ddf224180c16ef99259ab Mon Sep 17 00:00:00 2001 From: stark256-spec Date: Tue, 2 Jun 2026 14:34:36 -0500 Subject: [PATCH] fix: guard against NULL data_ptr in CF_CFDP_CopyStringFromLV CF_CFDP_DoDecodeChunk() sets data_ptr to NULL when the attacker-supplied LV length field exceeds the remaining PDU bytes. Without an explicit NULL check, CF_CFDP_CopyStringFromLV() passes NULL directly to memcpy(), which is undefined behaviour and causes a NULL-pointer dereference on most platforms (crash / denial of service via crafted CFDP Metadata PDU). Add a data_ptr != NULL guard before the memcpy so the function returns CF_ERROR consistently whether the decode ran out of PDU data (NULL ptr) or the string was longer than the destination buffer. Fixes: nasa/CF#491 --- fsw/src/cf_cfdp.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/fsw/src/cf_cfdp.c b/fsw/src/cf_cfdp.c index 5433fe7d..25342c1b 100644 --- a/fsw/src/cf_cfdp.c +++ b/fsw/src/cf_cfdp.c @@ -2237,7 +2237,11 @@ void CF_CFDP_SendEotPkt(CF_Transaction_t *txn) *-----------------------------------------------------------------*/ int CF_CFDP_CopyStringFromLV(char *buf, size_t buf_maxsz, const CF_Logical_Lv_t *src_lv) { - if (src_lv->length < buf_maxsz) + /* Guard against a NULL data_ptr, which CF_CFDP_DoDecodeChunk sets when the + * claimed LV length exceeds the remaining PDU bytes. Without this check a + * crafted PDU can trigger memcpy(buf, NULL, n) — undefined behaviour that + * causes a NULL-pointer dereference on most platforms. */ + if (src_lv->data_ptr != NULL && src_lv->length < buf_maxsz) { memcpy(buf, src_lv->data_ptr, src_lv->length); buf[src_lv->length] = 0; @@ -2246,7 +2250,7 @@ int CF_CFDP_CopyStringFromLV(char *buf, size_t buf_maxsz, const CF_Logical_Lv_t /* ensure output is empty */ buf[0] = 0; - return CF_ERROR; /* invalid len in lv? */ + return CF_ERROR; } /*----------------------------------------------------------------