From 2f0bf25670f8934a554d0aa014a4edb06e0d6135 Mon Sep 17 00:00:00 2001 From: Walker Date: Wed, 13 May 2026 13:47:37 -0500 Subject: [PATCH 1/3] Part cFS/workflows#129, Use Updated Static Analysis Workflow --- .github/workflows/static-analysis.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/static-analysis.yml b/.github/workflows/static-analysis.yml index 9c279a0..85dccf2 100644 --- a/.github/workflows/static-analysis.yml +++ b/.github/workflows/static-analysis.yml @@ -1,6 +1,5 @@ name: Static Analysis -# Run on all push and pull requests on: push: branches: @@ -16,4 +15,6 @@ on: jobs: static-analysis: name: Static Analysis - uses: nasa/cFS/.github/workflows/app-static-analysis-reusable.yml@dev \ No newline at end of file + uses: nasa/cFS/.github/workflows/static-analysis-reusable.yml@dev + with: + source-dir: 'source/fsw' \ No newline at end of file From d498d8751d2c48c238a80c702b191f620aaae250 Mon Sep 17 00:00:00 2001 From: Bee Rosa Davis Date: Tue, 9 Jun 2026 18:00:55 -0700 Subject: [PATCH 2/3] mm_load.c: validate DataSize locally in MM_PokeMem before dispatch MM_PokeMem() switches on CmdPtr->Payload.DataSize to choose between CFE_PSP_MemWrite8(), CFE_PSP_MemWrite16(), and CFE_PSP_MemWrite32(). The function relies on MM_VerifyPeekPokeParams() having already rejected any DataSize value that is not one of MM_INTERNAL_BYTE_BIT_WIDTH, MM_INTERNAL_WORD_BIT_WIDTH, or MM_INTERNAL_DWORD_BIT_WIDTH, as the comment above the empty default case explicitly states. This is correct under the current MM_PokeCmd dispatch path, which always calls MM_VerifyPeekPokeParams before MM_PokeMem, but the local function is the one that actually issues the PSP write calls and should not depend on a cross-translation-unit invariant for correctness. Any future code path that reaches MM_PokeMem without going through MM_VerifyPeekPokeParams --- a refactor of MM_PokeCmd dispatch, a new ground-command entry that delegates to MM_PokeMem, a test harness, or an EDS-generated wrapper --- would drop straight into the switch with PSP_Status left at its initialised value of CFE_PSP_ERROR_NOT_IMPLEMENTED and the command counter unchanged but no diagnostic event sent. The failure would be silent. Add an explicit DataSize check at function entry, returning CFE_PSP_ERROR and raising MM_DATA_SIZE_BITS_ERR_EID on the new error path. Matches the defensive style MM_VerifyPeekPokeParams already uses for the same field, and the defense-in-depth template nasa/fprime PR 5262 used for the same kind of brick-wall reliance on an upstream type narrowing. Found by the Davis Manifold geometric vulnerability detector (scj-hunt) while ranking MM functions by attacker-input-handling density. MM_PokeMem ranked top of the MM bundle under the psp_memwrite_unguarded compound rule. The same defensive pattern applies to MM_PokeEeprom, MM_LoadMem variants, MM_DumpMem variants, and the MM_FillMem family. Happy to scope a follow-up sweep covering those once this template is accepted. Signed-off-by: Bee Rosa Davis --- fsw/src/mm_load.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/fsw/src/mm_load.c b/fsw/src/mm_load.c index 49e8e0a..f6ec397 100644 --- a/fsw/src/mm_load.c +++ b/fsw/src/mm_load.c @@ -56,6 +56,34 @@ CFE_Status_t MM_PokeMem(const MM_PokeCmd_t *CmdPtr, cpuaddr DestAddress) size_t DataSize = 0; /* only used for giving MEM type/size in events */ uint32 EventID = 0; + /* + * Defense-in-depth: re-validate the DataSize field before + * dispatching to the platform write primitives. Today the + * validity of this field is also enforced upstream in + * MM_VerifyPeekPokeParams() before MM_PokeMem() is called, + * but this function is the one that holds the + * CFE_PSP_MemWrite{8,16,32}() calls and should not depend on + * a cross-translation-unit invariant for memory safety. Any + * future code path that reaches MM_PokeMem() without going + * through MM_VerifyPeekPokeParams() (refactor of MM_PokeCmd + * dispatch, new ground-command entry that delegates to + * MM_PokeMem, etc.) would otherwise drop straight into the + * switch with PSP_Status left at its initialised value of + * CFE_PSP_ERROR_NOT_IMPLEMENTED and the command counter + * unchanged, but no diagnostic event sent --- silently + * failing to update HK and silently failing to log. + */ + if (CmdPtr->Payload.DataSize != MM_INTERNAL_BYTE_BIT_WIDTH && + CmdPtr->Payload.DataSize != MM_INTERNAL_WORD_BIT_WIDTH && + CmdPtr->Payload.DataSize != MM_INTERNAL_DWORD_BIT_WIDTH) + { + CFE_EVS_SendEvent(MM_DATA_SIZE_BITS_ERR_EID, + CFE_EVS_EventType_ERROR, + "MM_PokeMem invalid data size: %u", + (unsigned int)CmdPtr->Payload.DataSize); + return CFE_PSP_ERROR; + } + /* Write input number of bits to destination address */ switch (CmdPtr->Payload.DataSize) { From 4eeced83eff0e7712124ffd139d5c0d5b9bb5968 Mon Sep 17 00:00:00 2001 From: Bee Rosa Davis Date: Wed, 10 Jun 2026 19:19:55 -0700 Subject: [PATCH 3/3] mm_load.c: validate DataSize locally in MM_PokeEeprom before dispatch Apply the same defense-in-depth template to MM_PokeEeprom() that the preceding commit applied to MM_PokeMem(). MM_PokeEeprom() switches on CmdPtr->Payload.DataSize to choose between CFE_PSP_EepromWrite8(), CFE_PSP_EepromWrite16(), and CFE_PSP_EepromWrite32(), and relies on MM_VerifyPeekPokeParams() having already rejected any invalid DataSize value, as the identical brick-wall comment block above the empty default case explicitly states. This is correct under the current MM_PokeCmd dispatch path but the local function is the one that actually issues the PSP EEPROM write calls, and EEPROM writes are particularly sensitive because they persist across power cycles. A silent failure here --- silent because PSP_Status is left at its initialised value CFE_PSP_ERROR_NOT_IMPLEMENTED and no diagnostic event is sent --- could leave the spacecraft in an unintended persistent state across reboots, with the perf-log entry/exit pair unbalanced relative to the actual work performed. Add the same explicit DataSize check at function entry, returning CFE_PSP_ERROR and raising MM_DATA_SIZE_BITS_ERR_EID on the new error path. Matches the MM_PokeMem hardening template added in the preceding commit on this PR. Found by scj-hunt's ATTEND brain primitive (v0.15 Phase A1) during a patch-twin sweep with MM_PokeMem as the seed: $ scj-hunt twins --bundle cfs_apps_mm_v01 --of MM_PokeMem rank 12: MM_PokeEeprom score 5.5 The patch-twin discovery exposes a gap in the Sudoku Principle catalog: the `psp_memwrite_unguarded` compound rule that flagged MM_PokeMem at score 10.0 keys off CFE_PSP_MemWrite{8,16,32} sinks and does not include CFE_PSP_EepromWrite{8,16,32}. ATTEND clustered the two functions together anyway because they share the same structural shape (3-way switch on DataSize, brick-wall reliance on MM_VerifyPeekPokeParams, identical doc-comment above default case) --- exactly the geometric patch-twin signal the brain primitive is designed to surface. The follow-up sweep on MM_LoadMem{8,16,32}FromFile, MM_DumpMemToFile, and the MM_FillMem family is still open; ATTEND ranks MM_LoadMemWIDCmd (the WID dispatcher for the LoadMem family) at rank 3 above this candidate. Signed-off-by: Bee Rosa Davis --- fsw/src/mm_load.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/fsw/src/mm_load.c b/fsw/src/mm_load.c index f6ec397..caa2ddf 100644 --- a/fsw/src/mm_load.c +++ b/fsw/src/mm_load.c @@ -169,6 +169,37 @@ CFE_Status_t MM_PokeEeprom(const MM_PokeCmd_t *CmdPtr, cpuaddr DestAddress) uint32 DataValue = 0; size_t BytesProcessed = 0; + /* + * Defense-in-depth: re-validate the DataSize field before + * dispatching to the platform write primitives. Today the + * validity of this field is also enforced upstream in + * MM_VerifyPeekPokeParams() before MM_PokeEeprom() is called, + * but this function is the one that holds the + * CFE_PSP_EepromWrite{8,16,32}() calls and should not depend + * on a cross-translation-unit invariant for memory safety. + * EEPROM writes are particularly sensitive because they + * persist across power cycles --- a silent failure here can + * leave the spacecraft in an unintended persistent state + * across reboots. Any future code path that reaches + * MM_PokeEeprom() without going through MM_VerifyPeekPokeParams() + * (refactor of MM_PokeCmd dispatch, new ground-command entry + * that delegates to MM_PokeEeprom, etc.) would otherwise drop + * straight into the switch with PSP_Status left at its + * initialised value of CFE_PSP_ERROR_NOT_IMPLEMENTED, no + * diagnostic event sent, and the perf-log entry/exit pair + * unbalanced relative to the actual work performed. + */ + if (CmdPtr->Payload.DataSize != MM_INTERNAL_BYTE_BIT_WIDTH && + CmdPtr->Payload.DataSize != MM_INTERNAL_WORD_BIT_WIDTH && + CmdPtr->Payload.DataSize != MM_INTERNAL_DWORD_BIT_WIDTH) + { + CFE_EVS_SendEvent(MM_DATA_SIZE_BITS_ERR_EID, + CFE_EVS_EventType_ERROR, + "MM_PokeEeprom invalid data size: %u", + (unsigned int)CmdPtr->Payload.DataSize); + return CFE_PSP_ERROR; + } + CFE_ES_PerfLogEntry(MM_EEPROM_POKE_PERF_ID); /* Write input number of bits to destination address */