From 5eab599fb1be913627ee5694f8d80a971e618005 Mon Sep 17 00:00:00 2001 From: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com> Date: Wed, 9 Sep 2026 02:23:29 +0100 Subject: [PATCH 1/2] Validate OTAR TLV lengths before parsing --- src/core/crypto.c | 18 ++++++++++++++++-- src/core/crypto_key_mgmt.c | 26 ++++++++++++++++++-------- test/unit/ut_crypto.c | 28 +++++++++++++++++++++------- test/unit/ut_ep_key_mgmt.c | 6 ++++++ 4 files changed, 61 insertions(+), 17 deletions(-) diff --git a/src/core/crypto.c b/src/core/crypto.c index 7c89d157..a06f8c02 100644 --- a/src/core/crypto.c +++ b/src/core/crypto.c @@ -1225,11 +1225,25 @@ int32_t Crypto_Process_Extended_Procedure_Pdu(TC_t *tc_sdls_processed_frame, uin sdls_frame.tlv_pdu.hdr.pid = (tc_sdls_processed_frame->tc_pdu[0] & 0x0F); sdls_frame.tlv_pdu.hdr.pdu_len = (tc_sdls_processed_frame->tc_pdu[1] << 8) | tc_sdls_processed_frame->tc_pdu[2]; - for (int x = 3; x < (3 + tc_sdls_processed_frame->tc_header.fl); x++) + + if (tc_sdls_processed_frame->tc_pdu_len < SDLS_TLV_HDR_SIZE) + { + return CRYPTO_LIB_ERR_BAD_TLV_LENGTH; + } + + uint16_t max_tlv = tc_sdls_processed_frame->tc_pdu_len - SDLS_TLV_HDR_SIZE; + uint16_t declared_tlv_bytes = + (sdls_frame.tlv_pdu.hdr.pdu_len + (BYTE_LEN - 1)) / BYTE_LEN; + if ((declared_tlv_bytes > max_tlv) || (declared_tlv_bytes > TLV_DATA_SIZE)) + { + return CRYPTO_LIB_ERR_BAD_TLV_LENGTH; + } + + for (uint16_t x = 0; x < declared_tlv_bytes; x++) { // Todo - Consider how this behaves with large OTAR PDUs that are larger than 1 TC in size. Most // likely fails. Must consider Uplink Sessions (sequence numbers). - sdls_frame.tlv_pdu.data[x - 3] = tc_sdls_processed_frame->tc_pdu[x]; + sdls_frame.tlv_pdu.data[x] = tc_sdls_processed_frame->tc_pdu[x + SDLS_TLV_HDR_SIZE]; } #ifdef CCSDS_DEBUG diff --git a/src/core/crypto_key_mgmt.c b/src/core/crypto_key_mgmt.c index 139fd547..cd1cd751 100644 --- a/src/core/crypto_key_mgmt.c +++ b/src/core/crypto_key_mgmt.c @@ -54,20 +54,30 @@ int32_t Crypto_Key_OTAR(void) int y; int32_t status = CRYPTO_LIB_SUCCESS; - int pdu_keys = ((sdls_frame.tlv_pdu.hdr.pdu_len / BYTE_LEN) - SDLS_KEYID_LEN - SDLS_IV_LEN - MAC_SIZE) / - (SDLS_KEYID_LEN + SDLS_KEY_LEN); + const int pdu_len_bits = sdls_frame.tlv_pdu.hdr.pdu_len; + const int pdu_len_bytes = pdu_len_bits / BYTE_LEN; + const int fixed_len = SDLS_KEYID_LEN + SDLS_IV_LEN + MAC_SIZE; + const int key_block_len = SDLS_KEYID_LEN + SDLS_KEY_LEN; + + if ((pdu_len_bits % BYTE_LEN != 0) || (pdu_len_bytes < fixed_len) || (pdu_len_bytes > TLV_DATA_SIZE) || + ((pdu_len_bytes - fixed_len) % key_block_len != 0)) + { + return CRYPTO_LIB_ERR_OTAR_BAD_TLV_LENGTH; + } + + int pdu_keys = (pdu_len_bytes - fixed_len) / key_block_len; + if (pdu_keys > SDLS_EKB_LEN) + { + return CRYPTO_LIB_ERR_OTAR_BAD_TLV_LENGTH; + } + int w; crypto_key_t *ekp = NULL; #ifdef DEBUG - int expected_pdu_len = SDLS_KEYID_LEN + SDLS_IV_LEN + ((SDLS_KEYID_LEN + SDLS_KEY_LEN) * pdu_keys) + MAC_SIZE; + int expected_pdu_len = fixed_len + (key_block_len * pdu_keys); printf("Expected PDU Length: %d (%d keys)\n", expected_pdu_len, pdu_keys); #endif - if ((sdls_frame.tlv_pdu.hdr.pdu_len / BYTE_LEN) < - SDLS_KEYID_LEN + SDLS_IV_LEN + ((SDLS_KEYID_LEN + SDLS_KEY_LEN) * pdu_keys) + MAC_SIZE) - { - return CRYPTO_LIB_ERR_OTAR_BAD_TLV_LENGTH; - } // Master Key ID packet.mkid = (sdls_frame.tlv_pdu.data[0] << BYTE_LEN) | (sdls_frame.tlv_pdu.data[1]); diff --git a/test/unit/ut_crypto.c b/test/unit/ut_crypto.c index 7493a473..d66047b2 100644 --- a/test/unit/ut_crypto.c +++ b/test/unit/ut_crypto.c @@ -269,19 +269,33 @@ UTEST(CRYPTO_C, PDU_SWITCH) /** * @brief Unit Test: Crypto Extended Procedures PDU Test **/ +UTEST(CRYPTO_C, EXT_PROC_PDU_REJECTS_OVERSIZED_DECLARED_TLV) +{ + TC_t tc_frame = {0}; + + tc_frame.tc_sec_header.spi = SPI_MIN; + tc_frame.tc_header.vcid = TC_SDLS_EP_VCID; + tc_frame.tc_pdu_len = SDLS_TLV_HDR_SIZE + 4; + tc_frame.tc_pdu[0] = PID_OTAR; + tc_frame.tc_pdu[1] = 0; + tc_frame.tc_pdu[2] = 64; // Declares eight bytes, but only four are present. + + int32_t status = Crypto_Process_Extended_Procedure_Pdu(&tc_frame, NULL, tc_frame.tc_pdu_len); + ASSERT_EQ(CRYPTO_LIB_ERR_BAD_TLV_LENGTH, status); +} + UTEST(CRYPTO_C, EXT_PROC_PDU) { remove("sa_save_file.bin"); uint8_t *ingest = NULL; - TC_t *tc_frame = NULL; - tc_frame = malloc(sizeof(uint8_t) * TC_SIZE); - int32_t status = CRYPTO_LIB_ERROR; + TC_t tc_frame = {0}; + int32_t status = CRYPTO_LIB_ERROR; crypto_config_tc.has_pus_hdr = TC_NO_PUS_HDR; - tc_frame->tc_header.vcid = TC_SDLS_EP_VCID; - tc_frame->tc_header.fl = 1; + tc_frame.tc_sec_header.spi = SPI_MIN; + tc_frame.tc_header.vcid = TC_SDLS_EP_VCID; + tc_frame.tc_pdu_len = SDLS_TLV_HDR_SIZE; - status = Crypto_Process_Extended_Procedure_Pdu(tc_frame, ingest, TC_SIZE); - free(tc_frame); + status = Crypto_Process_Extended_Procedure_Pdu(&tc_frame, ingest, TC_SIZE); ASSERT_EQ(status, CRYPTO_LIB_SUCCESS); } #endif // CRYPTO_EPROC diff --git a/test/unit/ut_ep_key_mgmt.c b/test/unit/ut_ep_key_mgmt.c index a730ff0e..a6754bb7 100644 --- a/test/unit/ut_ep_key_mgmt.c +++ b/test/unit/ut_ep_key_mgmt.c @@ -4,6 +4,12 @@ #include "sa_interface.h" #include "utest.h" +UTEST(EP_KEY_MGMT, OTAR_REJECTS_OVERSIZED_TLV) +{ + sdls_frame.tlv_pdu.hdr.pdu_len = (TLV_DATA_SIZE + 1) * BYTE_LEN; + ASSERT_EQ(CRYPTO_LIB_ERR_OTAR_BAD_TLV_LENGTH, Crypto_Key_OTAR()); +} + UTEST(EP_KEY_MGMT, OTAR_0_140_142) { remove("sa_save_file.bin"); From b668bdcd81f5e4f49e2c61fe452eb599fcd4a766 Mon Sep 17 00:00:00 2001 From: Sylvester Kaczmarek <16242628+sylvesterkaczmarek@users.noreply.github.com> Date: Wed, 9 Sep 2026 08:16:53 +0100 Subject: [PATCH 2/2] Parse no-packet Extended Procedure reply type --- src/core/crypto.c | 2 +- test/unit/ut_crypto.c | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/core/crypto.c b/src/core/crypto.c index a06f8c02..e0354111 100644 --- a/src/core/crypto.c +++ b/src/core/crypto.c @@ -1219,7 +1219,7 @@ int32_t Crypto_Process_Extended_Procedure_Pdu(TC_t *tc_sdls_processed_frame, uin #endif // No Packet HDR or PUS in these frames // SDLS TLV PDU - sdls_frame.hdr.type = (tc_sdls_processed_frame->tc_pdu[0] & 0x80) >> 7; + sdls_frame.tlv_pdu.hdr.type = (tc_sdls_processed_frame->tc_pdu[0] & 0x80) >> 7; sdls_frame.tlv_pdu.hdr.uf = (tc_sdls_processed_frame->tc_pdu[0] & 0x40) >> 6; sdls_frame.tlv_pdu.hdr.sg = (tc_sdls_processed_frame->tc_pdu[0] & 0x30) >> 4; sdls_frame.tlv_pdu.hdr.pid = (tc_sdls_processed_frame->tc_pdu[0] & 0x0F); diff --git a/test/unit/ut_crypto.c b/test/unit/ut_crypto.c index d66047b2..4df84aee 100644 --- a/test/unit/ut_crypto.c +++ b/test/unit/ut_crypto.c @@ -269,6 +269,25 @@ UTEST(CRYPTO_C, PDU_SWITCH) /** * @brief Unit Test: Crypto Extended Procedures PDU Test **/ +/** + * @brief Unit Test: No-packet EP reply type is parsed into the TLV header + **/ +UTEST(CRYPTO_C, EXT_PROC_PDU_NO_PACKET_REPLY_TYPE) +{ + TC_t tc_frame = {0}; + + tc_frame.tc_sec_header.spi = SPI_MIN; + tc_frame.tc_header.vcid = TC_SDLS_EP_VCID; + tc_frame.tc_pdu_len = SDLS_TLV_HDR_SIZE; + tc_frame.tc_pdu[0] = 0x80; // Procedure type = reply + + sdls_frame.tlv_pdu.hdr.type = PDU_TYPE_COMMAND; + + int32_t status = Crypto_Process_Extended_Procedure_Pdu(&tc_frame, NULL, 0); + ASSERT_EQ(CRYPTO_LIB_SUCCESS, status); + ASSERT_EQ(PDU_TYPE_REPLY, (uint8_t)sdls_frame.tlv_pdu.hdr.type); +} + UTEST(CRYPTO_C, EXT_PROC_PDU_REJECTS_OVERSIZED_DECLARED_TLV) { TC_t tc_frame = {0};