nasa / nasa/CryptoLib

sa_get_from_spi missing shivf_len <= iv_len cross-field check (sibling of existing arsn/shsnf guard)

Open Beginner friendly
#512 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C
Stars
169
Forks
67
PR merge metrics
No merged PRs in 30d

Description

Summary

sa_get_from_spi() in src/sa/internal/sa_interface_inmemory.template.c enforces the cross-field invariant arsn_len >= shsnf_len (line 777, returning CRYPTO_LIB_ERR_ARSN_LT_SHSNF) but does not enforce the parallel invariant iv_len >= shivf_len. A Security Association loaded with shivf_len > iv_len (e.g. via a hand-edited inmemory config or an mariadb row crafted by an operator with SA-table write access) causes every per-frame IV-walk loop to start at a negative index and read up to shivf_len bytes from before sa_ptr->iv inside the SecurityAssociation_t struct — leaking pre-iv struct fields into every outgoing TC/AOS frame.

Locations

The vulnerable loop shape is:

for (i = sa_ptr->iv_len - sa_ptr->shivf_len; i < sa_ptr->iv_len; i++)
{
    *(p_new_enc_frame + index_temp) = *(sa_ptr->iv + i);
    index_temp++;
}

Hits in production code:

  • src/core/crypto_aos.c:318 (IV print path, debug)
  • src/core/crypto_aos.c:351 (Crypto_AOS_ApplySecurity transmitted-IV write)
  • src/core/crypto_aos.c:697 (Crypto_AOS_ProcessSecurity IV reconstruction)
  • src/core/crypto_tc.c:331 (Crypto_TC_ApplySecurity transmitted-IV write)
  • src/core/crypto_tc.c:631 (Crypto_TC_ProcessSecurity IV reconstruction)

The sibling shsnf_len/arsn_len pattern appears at crypto_aos.c:365, 710 and crypto_tc.c:644, 1233 — same shape, same risk. The line-777 check in sa_get_from_spi already guards those; the matching check for IV is missing.

Threat model

Loading a malformed SA requires write access to the SA backend (inmemory config file, mariadb security_associations table, custom backend). On a deployed system that means an operator-tier or upstream-tooling-tier compromise — not a remote attacker. The leak is small (≤ shivf_len bytes per frame, typically 12) but it is steady (every encrypted/authenticated frame the SA emits leaks a few bytes of adjacent SA struct memory into the channel).

sa_verify_data() (line 1822 same file) checks each field against the compile-time max (shivf_len > IV_SIZE, shsnf_len > ARSN_SIZE, etc.) but not against the per-SA iv_len / arsn_len. The two layers of check are complementary.

Fix

PR adds the parallel guard immediately after the existing line-777 arsn_len/shsnf_len check, plus the new CRYPTO_LIB_ERR_SHIVF_LEN_GREATER_THAN_IV_LEN (-86) return code in include/crypto_error.h and its name string in src/core/crypto_error.c.

Notes

The Mariadb backend (src/sa/mariadb/sa_interface_mariadb.template.c) doesn't currently check either invariant on insert — a follow-up could add the same two-line guard there too. I left it out of this PR to keep scope tight.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/sa/internal/sa_interface_inmemory.template.c at the existing arsn_len/shsnf_len guard in sa_get_from_spi() and compare the surrounding validation logic. Then inspect include/crypto_error.h and src/core/crypto_error.c for the matching error definition and name string. Done means malformed SAs with shivf_len greater than iv_len are rejected with the new error while the existing invariant remains unchanged.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
cryptography, security
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.