Skip to content

πŸ› [BUG] - Missing Control SPI Check in sa_startΒ #519

Description

@VissaMoutafis

Description

Bug Report: Missing Control SPI Check in sa_start

Summary

Field Value
Product NASA CryptoLib (SDLS Protocol Implementation)
Version 1.4.2
Component src/sa/internal/sa_interface_inmemory.template.c β€” sa_start
Issue sa_start does not check if the target SPI matches the control (in-use) SPI
Impact Allows modifying the SA currently in use for the control channel, violating SDLS-EP protocol safety constraints

Description

The sa_start function (line 1023) does not verify that the target SPI is different from the control SPI (the SPI of the SA currently being used to protect the TC channel carrying the EP command). This check is present in all other SA management commands:

Function Control SPI check Line
sa_stop Yes 1191
sa_rekey Yes 1275
sa_expire Yes 1372
sa_create Yes 1437
sa_delete Yes (present)
sa_setARSN Yes (present)
sa_setARSNW Yes (present)
sa_start No Missing

The SDLS-EP specification (CCSDS 355.1-B-1) requires that the SA used to carry EP commands cannot be modified by those same commands, as this could disrupt the security of the control channel itself.

Affected Code

src/sa/internal/sa_interface_inmemory.template.c, sa_start function starting at line 1023:

static int32_t sa_start(TC_t *tc_frame)
{
    uint8_t        count = 0;
    uint16_t       spi   = 0x0000;
    // ...

    spi = ((uint8_t)sdls_frame.tlv_pdu.data[0] << 8) | (uint8_t)sdls_frame.tlv_pdu.data[1];

    // *** Missing check: spi == tc_frame->tc_sec_header.spi ***

    if (spi < NUM_SA)
    {
        // ... proceeds to modify SA without checking control SPI ...
    }
    // ...
}

Compare with sa_stop (line 1174), which correctly includes:

static int32_t sa_stop(TC_t *tc_frame)
{
    // ...
    spi         = ((uint8_t)sdls_frame.tlv_pdu.data[0] << BYTE_LEN) | (uint8_t)sdls_frame.tlv_pdu.data[1];
    control_spi = tc_frame->tc_sec_header.spi;

    if (spi == control_spi)
    {
        status = CRYPTO_LIB_ERR_SDLS_EP_WRONG_SPI;
        return status;
    }
    // ...
}

Suggested Fix

Add the control SPI check at the beginning of sa_start, consistent with all other SA management functions:

static int32_t sa_start(TC_t *tc_frame)
{
    // ...
    spi = ((uint8_t)sdls_frame.tlv_pdu.data[0] << 8) | (uint8_t)sdls_frame.tlv_pdu.data[1];

    control_spi = tc_frame->tc_sec_header.spi;
    if (spi == control_spi)
    {
        status = CRYPTO_LIB_ERR_SDLS_EP_WRONG_SPI;
        return status;
    }
    // ...
}

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions