Skip to content

GetSectionHeader: bound the section-name read loop (matches GetSymbol fix from #32)#164

Open
nurdymuny wants to merge 5 commits into
nasa:mainfrom
nurdymuny:fix/section-name-overflow
Open

GetSectionHeader: bound the section-name read loop (matches GetSymbol fix from #32)#164
nurdymuny wants to merge 5 commits into
nasa:mainfrom
nurdymuny:fix/section-name-overflow

Conversation

@nurdymuny

Copy link
Copy Markdown

Summary

Adds the i < sizeof(VerboseStr) bound to the section-name fgetc loop in
GetSectionHeader() and appends an explicit NUL terminator after the loop.

This matches the fix that was applied to the identical loop in GetSymbol()
in commit 75844867 (Fix #32, May 2020) — that pass touched the symbol-name
loop but missed the sibling section-name loop one screen up.

Without this bound, an ELF input whose section header string table contains
a string of 60+ non-NUL bytes overflows the fixed 60-byte stack buffer
VerboseStr[60].

Diff

-        while ((VerboseStr[i] = fgetc(SrcFileDesc)) != '\0')
+        while ((i < sizeof(VerboseStr)) && ((VerboseStr[i] = fgetc(SrcFileDesc)) != '\0'))
         {
             i++;
         }
+        VerboseStr[i % sizeof(VerboseStr)] = '\0'; /* nul-terminate; see the GetSymbol() loop for the same fix shape */

Tests

  • Verified by inspection against the GetSymbol() loop at line 1971 which
    uses the same buffer and same fix shape.
  • Static analysis baseline (CodeQL / Cppcheck) in the existing CI should
    remain clean; this change closes one finding rather than introducing one.

Linked

Closes #163.

jphickey and others added 5 commits May 13, 2026 09:47
Reset MISSION_REV to 0xFF and build number to 1.
…use-updated-static-analysis-workflow

Part cFS/workflows#129, Use Updated Static Analysis Workflow
The fgetc loop that reads a section name from the ELF section header
string table into the fixed 60-byte VerboseStr[60] buffer had no upper
bound on the loop index. An input ELF whose string table contains a
section-name string longer than 60 bytes (no early '\0') overflows the
stack buffer.

The sibling loop in GetSymbol() (line 1971) reads a symbol name into
the same buffer using the correct '(i < sizeof(VerboseStr))' bound;
that bound was added in 7584486 (Fix nasa#32, May 2020) but the same
fix was never applied to the section-name loop a screen up.

This applies the same fix shape — bound the loop, then explicitly
nul-terminate after.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants