🐛 [BUG] - Conformance - SA setARSN and setARSNW Always Return Success Regardless of Precondition Failures

Description

Bug Report: SA setARSN and setARSNW Always Return Success Regardless of Precondition Failures

Summary

Field	Value
Product	NASA CryptoLib (SDLS Protocol Implementation)
Version	1.4.2
Component	src/sa/internal/sa_interface_inmemory.template.c — sa_setARSN, sa_setARSNW
Issue	Both functions unconditionally return CRYPTO_LIB_SUCCESS at the end, regardless of which internal branch executed — including branches that explicitly detect and log a precondition failure
Bug Type	Conformance
Severity	Medium
Impact	Callers cannot distinguish a successful ARSN/ARSNW update from a rejected one under any circumstance; an SDLS-EP peer that relies on the return status to know whether its command was applied has no way to detect a rejected update

---

Description

sa_setARSN and sa_setARSNW (src/sa/internal/sa_interface_inmemory.template.c) each end
with a single, unconditional return CRYPTO_LIB_SUCCESS; statement outside of all their internal
if/else branching. This means the return value carries no information at all about what
actually happened inside the function — even in the cases where the function's own logic
correctly identifies that the request should be rejected.

This is visible most clearly in sa_setARSN, which does implement an authentication-capability
check and does print a diagnostic when it fails — the check runs, the failure is detected, and
the function still reports success to its caller:

if ((sa[spi].est == 1 && sa[spi].ast == 1) || sa[spi].ast == 1)
{ /* accept the update */ }
else
{
#ifdef PDU_DEBUG
    printf("Failed setARSN on SPI %d, ECS %d, ACS %d\n", spi, sa[spi].ecs, sa[spi].acs);
#endif
    // no status assignment here — falls through to the same `return CRYPTO_LIB_SUCCESS` below
}

The same shape recurs for the out-of-range-SPI branch in both functions, and — in
sa_setARSNW's case — for a precondition that isn't even checked at all (see the "SetARSNW has
no length validation" evidence below): whether the function detects a problem, silently misses
one, or never had a check to begin with, the caller gets CRYPTO_LIB_SUCCESS every time.

Affected Code

sa_setARSN (src/sa/internal/sa_interface_inmemory.template.c:1667-1724)

static int32_t sa_setARSN(TC_t *tc_frame)
{
    ...
    if (spi < NUM_SA)
    {
        if ((sa[spi].est == 1 && sa[spi].ast == 1) || sa[spi].ast == 1)
        { /* ... set ARSN, the "eligible" path ... */ }
        else
        {
#ifdef PDU_DEBUG
            printf("Failed setARSN on SPI %d, ECS %d, ACS %d\n", spi, sa[spi].ecs, sa[spi].acs);
#endif
            // no status assignment
        }
    }
    else
    {
        printf("sa_setARSN ERROR: SPI %d does not exist.\n", spi);
    }

    return CRYPTO_LIB_SUCCESS;   // unconditional, regardless of which branch executed
}

sa_setARSNW (src/sa/internal/sa_interface_inmemory.template.c:1730-1775)

static int32_t sa_setARSNW(TC_t *tc_frame)
{
    ...
    if (spi < NUM_SA)
    {
        if (sa[spi].arsnw_len > (ARSN_SIZE)) { sa[spi].arsnw_len = ARSN_SIZE; }
        sa[spi].arsnw = (((uint8_t)sdls_frame.tlv_pdu.data[2]));   // no length field, no validation
    }
    else
    {
#ifdef PDU_DEBUG
        printf("sa_setARSNW ERROR: SPI %d does not exist.\n", spi);
#endif
    }

    return CRYPTO_LIB_SUCCESS;   // unconditional
}

Reproduction Sequence (from the fuzzing logs)

Two independent single-frame reproductions, each showing a different way the return value ends
up wrong — a check that runs and fails (SetARSN), and a check that never exists at all
(SetARSNW):

Case 1 — check runs, detects failure, is still ignored (sa_setARSN, SPI 63 not
authentication-capable):

#	Direction	Hex	Meaning
1	ground → flight	002c10a6000000001104d0003f0c001001020c00000000000000000000000001010080000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002ab0	TC frame, SA Create, SPI 63, est=0/ast=0 (no auth service)
2	ground → flight	002c100e000000001a0010003ffa32	TC frame, sg=0x1, pid=0xa (SA SetARSN), SPI 63

SA SetARSN
Failed setARSN on SPI 63, ECS 2, ACS 1
Status code: 0

sa_setARSN prints its own diagnostic confirming it detected the SA isn't eligible, then
returns Status code: 0 (success) anyway.

Case 2 — no check exists at all (sa_setARSNW, no length field on the wire, no validation of
any kind):

#	Direction	Hex	Meaning
1	ground → flight	(same SA Create as above, SPI 63)	—
2	ground → flight	002c100e00000000150010003f9fcb	TC frame, sg=0x1, pid=0x5 (SA SetARSNW), SPI 63, single raw byte as the window value

SA setARSNW
spi = 63
ARSN set to: 0
Status code: 0

No diagnostic at all — sa_setARSNW has no equivalent check to even attempt, reads a single
fixed wire byte with no length field, and reports success regardless.

Case 3 — out-of-range SPI, same architecture, both functions:

SA SetARSN
sa_setARSN ERROR: SPI 52788 does not exist.
Status code: 0

SA setARSNW
spi = 11356
sa_setARSNW ERROR: SPI 11356 does not exist.
Status code: 0

Both branches print an explicit "does not exist" diagnostic and still return 0.

Suggested Fix

Thread a real status variable through every branch instead of returning a hardcoded constant:

int32_t status = CRYPTO_LIB_SUCCESS;
if (spi < NUM_SA) {
    if (/* authentication-capable */) {
        // ... set ARSN/ARSNW ...
    } else {
        status = CRYPTO_LIB_ERR_SA_AUTH_NOT_ENABLED;  // or a suitably specific new code
    }
} else {
    status = CRYPTO_LIB_ERR_SPI_INDEX_OOB;
}
return status;

This alone fixes Case 1 and Case 3 above (checks that already run correctly, just aren't
propagated). It does not fix Case 2 — sa_setARSNW additionally needs the missing
authentication-capability check and a real length field/validation added before there is
anything to propagate a failure status for.

---

Discovered using the StratoFuzz protocol fuzzing framework.

Branch Name

dev

Reproduction steps

## Reproduction Sequence (from the fuzzing logs)

Two independent single-frame reproductions, each showing a different way the return value ends
up wrong — a check that runs and fails (SetARSN), and a check that never exists at all
(SetARSNW):

**Case 1 — check runs, detects failure, is still ignored** (`sa_setARSN`, SPI 63 not
authentication-capable):

| # | Direction | Hex | Meaning |
|---|-----------|-----|---------|
| 1 | ground → flight | `002c10a6000000001104d0003f0c001001020c00000000000000000000000001010080000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002ab0` | TC frame, SA Create, SPI 63, `est=0/ast=0` (no auth service) |
| 2 | ground → flight | `002c100e000000001a0010003ffa32` | TC frame, `sg=0x1, pid=0xa` (SA SetARSN), SPI 63 |


SA SetARSN
Failed setARSN on SPI 63, ECS 2, ACS 1
Status code: 0

`sa_setARSN` prints its own diagnostic confirming it detected the SA isn't eligible, then
returns `Status code: 0` (success) anyway.

**Case 2 — no check exists at all** (`sa_setARSNW`, no length field on the wire, no validation of
any kind):

| # | Direction | Hex | Meaning |
|---|-----------|-----|---------|
| 1 | ground → flight | (same SA Create as above, SPI 63) | — |
| 2 | ground → flight | `002c100e00000000150010003f9fcb` | TC frame, `sg=0x1, pid=0x5` (SA SetARSNW), SPI 63, single raw byte as the window value |


SA setARSNW
spi = 63
ARSN set to: 0
Status code: 0

No diagnostic at all — `sa_setARSNW` has no equivalent check to even attempt, reads a single
fixed wire byte with no length field, and reports success regardless.

**Case 3 — out-of-range SPI, same architecture, both functions**:


SA SetARSN
sa_setARSN ERROR: SPI 52788 does not exist.
Status code: 0


SA setARSNW
spi = 11356
sa_setARSNW ERROR: SPI 11356 does not exist.
Status code: 0

Both branches print an explicit "does not exist" diagnostic and still return `0`.

Screenshots

Logs

OS

Linux

Start in src/sa/internal/sa_interface_inmemory.template.c at sa_setARSN and sa_setARSNW, then reproduce the three cases described in the issue to observe their status codes. Review nearby status constants and related SA setters; the work is done when rejected authentication and out-of-range SPI cases return failure, and ARSNW validates the required preconditions and input length.