🐛 [BUG] - Key Inventory Missing Range Validation

[VissaMoutafis]

Description

Bug Report: Key Inventory Missing Range Validation

Summary

Field	Value
Product	NASA CryptoLib (SDLS Protocol Implementation)
Version	1.4.2
Component	src/core/crypto_key_mgmt.c — Crypto_Key_inventory
Issue	No validation that kid_first <= kid_last, causing uint16_t underflow
Impact	Returns an incorrect (potentially very large) range of key states

---

Description

The Crypto_Key_inventory function (line 384) reads a kid_first and kid_last pair from the PDU to define the range of keys to report. The range is computed as:

range = packet.kid_last - packet.kid_first + 1;  // line 409

There is no check that kid_first <= kid_last. When kid_first > kid_last, the subtraction underflows the uint16_t value, producing a very large range. This large range is then used to:

1. Set sdls_frame.tlv_pdu.hdr.pdu_len (line 410) — overflowing the PDU length field.
2. Set sdls_frame.hdr.pkt_length (line 411) — corrupting the packet length.
3. Drive the loop for (x = packet.kid_first; x <= packet.kid_last; x++) (line 418) — which, due to uint16_t wraparound, iterates through nearly the entire key space.

The loop writes key IDs and key states into sdls_ep_reply via an uint8_t count variable (line 389) that will wrap around at 255, causing writes to overwrite earlier entries in the reply buffer. Additionally, since the count variable is uint8_t, it wraps silently and the function returns without error.

Affected Code

src/core/crypto_key_mgmt.c, lines 400–431:

// Read in PDU
packet.kid_first =
    ((uint8_t)sdls_frame.tlv_pdu.data[count] << BYTE_LEN) | ((uint8_t)sdls_frame.tlv_pdu.data[count + 1]);
count = count + 2;
packet.kid_last =
    ((uint8_t)sdls_frame.tlv_pdu.data[count] << BYTE_LEN) | ((uint8_t)sdls_frame.tlv_pdu.data[count + 1]);
count = count + 2;

// Prepare for Reply
range = packet.kid_last - packet.kid_first + 1;  // *** No check that kid_first <= kid_last ***
sdls_frame.tlv_pdu.hdr.pdu_len = (SDLS_KEY_INVENTORY_RPLY_SIZE * (range)) * BYTE_LEN;
// ...

for (x = packet.kid_first; x <= packet.kid_last; x++)
{
    sdls_ep_reply[count++] = ((x & 0xFF00) >> BYTE_LEN);  // *** count wraps at 255 ***
    sdls_ep_reply[count++] = (x & 0x00FF);
    // ...
    sdls_ep_reply[count++] = ekp->key_state;
}

PoC

The following 8-packet sequence triggers this bug. Packet 8 contains a Key Inventory command where FirstKeyId (0xF157 = 61783) is greater than LastKeyId (0x03FF = 1023):

Packet 1/8: 002c1046000000001101d0003fcc001001010c0000000000000000000000000101002000000000000000000000000000000000000000000000000000000000000000000000f2d4
Packet 2/8: 002c104c000000000102000000000000000000000000000000719405af7fcd82f5c1073e5c81455b28e37b2e8b4cada60ec40bd3eea8585e2fb2e22726654fed87b83a4a912feffdf35b9c1422
Packet 3/8: 002c100e0000000002001000828a33
Packet 4/8: 002c101e00000000160090003f00820082000000000000000000000000551f
Packet 5/8: 002c100e0000000003001000822062
Packet 6/8: 002c1012000000001b0030003f00002c040ac0
Packet 7/8: 002c100e0000000006001000820335
Packet 8/8: 002c10100000000007fa10f15703ff9782

Expected behavior: Return an error indicating the range is invalid (first key ID must be less than or equal to last key ID).

Actual behavior: Returns CRYPTO_LIB_SUCCESS with a corrupted reply buffer and incorrect PDU/packet length fields.

Suggested Fix

Add a range validation check before computing the range:

if (packet.kid_first > packet.kid_last)
{
    return CRYPTO_LIB_ERROR;  // Or a more specific error code
}

range = packet.kid_last - packet.kid_first + 1;

Additionally, the count variable should be widened from uint8_t to uint16_t to avoid silent wraparound when iterating over large key ranges.

---

Discovered using the StratoFuzz protocol fuzzing framework.

Branch Name

No response

Reproduction steps

1. Go to '...'
2. Click on '....'
3. Scroll down to '....'
4. See error

Screenshots

![DESCRIPTION](LINK.png)

Logs

OS

Linux