Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible PortMessage buffer truncation #2529

Closed
chillfig opened this issue Mar 12, 2024 · 2 comments · Fixed by #2566
Closed

Possible PortMessage buffer truncation #2529

chillfig opened this issue Mar 12, 2024 · 2 comments · Fixed by #2566

Comments

@chillfig
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Data truncation is possible in the snprintf() below

snprintf(PortMessage, sizeof(PortMessage), "%s %u/%u/%s %u: %s", TimeBuffer,
(unsigned int)EVS_PktPtr->Payload.PacketID.SpacecraftID,
(unsigned int)EVS_PktPtr->Payload.PacketID.ProcessorID, EVS_PktPtr->Payload.PacketID.AppName,
(unsigned int)EVS_PktPtr->Payload.PacketID.EventID, EVS_PktPtr->Payload.Message);

Describe the solution you'd like
Appropriately allocate size for the message.

Describe alternatives you've considered
Leave as is. The possibility of truncating the message may be low given that AppName, for example, will not likely occupy the max 20 characters that it is allotted.

Additional context
This issue was flagged in JSC 2.1 static analysis.

TimeBuffer: 23 characters (excluding the null terminator)
Spacecraft ID: 10 characters (maximum length for uint32)
Processor ID: 10 characters (maximum length for uint32)
AppName: 20 characters (maximum given size)
Event ID: 5 characters (maximum length for uint16)
Message: 122 characters (maximum given size)
Exact non-data characters: 8 characters
The total exact maximum length is 23 + 10 + 10 + 20 + 5 + 122 + 8 = 198 characters needed, not counting the null terminator.

The defined length for CFE_EVS_MAX_PORT_MSG_LENGTH of 172

Requester Info
Justin Figueroa, Vantage

@skliper
Copy link
Contributor

skliper commented May 20, 2024

This is actively happening in the bundle, see https://github.com/nasa/cFS/actions/runs/8971391786/artifacts/1476886717 for sample app, ci lab and to lab initialization messages. This ends up truncating the new line and makes the log harder to parse with long/clobbered lines:

EVS Port1 1980-012-14:03:20.55226 66/1/SAMPLE_APP 1: Sample App Initialized.Sample App Development Build equuleus-rc1+dev46 (Codename Equuleus), Last Official Release: Sam1980-012-14:03:20.55236 CI_LAB listening on UDP port: 1234
EVS Port1 1980-012-14:03:20.55236 66/1/CI_LAB_APP 3: CI Lab Initialized.CI Lab App Development Build equuleus-rc1+dev61 (Codename Equuleus), Last Official Release: CI Lab EVS Port1 1980-012-14:03:20.55256 66/1/TO_LAB_APP 1: TO Lab Initialized.TO Lab Development Build equuleus-rc1+dev52 (Codename Equuleus), Last Official Release: TO Lab v2.3SCH Lab Initialized.SCH Lab Development Build equuleus-rc1+dev29 (Codename Equuleus), Last Official Release: SCH Lab v2.3.0)

I suggest checking for and replacing last two chars w/ */n or whatever truncation character folks want.

Could also update the lab apps to not put out such a long msg, or increase the limit so the current messages fit.

@skliper
Copy link
Contributor

skliper commented May 20, 2024

Marking as a bug due to it missing the new line, so the next message gets added on in the output which could be lost by automatic parsing tools that might expect events/sys log output to be on it's own line. It's also harder to skim visually.

@skliper skliper added the bug label May 20, 2024
chillfig pushed a commit to chillfig/cFE that referenced this issue Jun 14, 2024
dzbaker added a commit that referenced this issue Jul 1, 2024
Fix #2529, Reduces CFE_EVS_MAX_PORT_MSG_LENGTH to prevent new line character truncation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants