search

nasa / cFE

The Core Flight System (cFS) Core Flight Executive (cFE)
Apache License 2.0
402 stars 198 forks source link

Possible PortMessage buffer truncation #2529

Closed chillfig closed 1 week ago

chillfig commented 3 months ago

Is your feature request related to a problem? Please describe. Data truncation is possible in the snprintf() below https://github.com/nasa/cFE/blob/5cffc39eccee4902f71809c8963920ea1469e3cd/modules/evs/fsw/src/cfe_evs_utils.c#L548-L551

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 commented 1 month ago

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 commented 1 month ago

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.