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.

Sign up for GitHub

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

Jump to bottom

Improper stack object access in the msg api functional test #2536

Closed
jphickey opened this issue Mar 26, 2024 · 0 comments · Fixed by #2537
Closed

Improper stack object access in the msg api functional test #2536

jphickey opened this issue Mar 26, 2024 · 0 comments · Fixed by #2537
Assignees
@jphickey

Comments

@jphickey
Copy link
Contributor

Describe the bug
In the msg_api_test.c functional test (part of cfe_testcase module), this allocates a CFE_MSG_CommandHeader_t object on the stack, and then proceeds to access this object with all header access APIs.

Importantly ... this includes header accessors that are intended for TLM, which is a slightly bigger header than CMD in the default config. As the message type wasn't set to CMD, the TLM header accessors proceed, and this results in overflowing the buffer.

To Reproduce
Execute CFE test cases, in particular this one:

cFE/modules/cfe_testcase/src/msg_api_test.c

Line 258 in ed1faf4

UtAssert_INT32_EQ(CFE_MSG_SetMsgTime(CFE_MSG_PTR(cmd), currentTime), CFE_SUCCESS);

This attempts to set the message time in a CMD packet, which is a field that is only in TLM (at least in the default header impl).

Expected behavior
The stack buffer must be allocated to allow access as CMD or TLM.

Code snips
See above

System observed on:
Debian with -fsanitize=address switch enabled

Additional context
Discovered using address sanitizer

Reporter Info
Joseph Hickey, Vantage Systems, Inc.
@jphickey jphickey self-assigned this Mar 26, 2024
jphickey added a commit to jphickey/cFE that referenced this issue Mar 26, 2024
@jphickey
Fix nasa#2536, msg api test buffer overrun
2c55e70 
Corrects the buffer type used in the msg api tests, to be sized appropriately
for either command or telemetry.  When the secondary header accessor from the
other type is used, it will not overrun the buffer.
@jphickey jphickey mentioned this issue Mar 26, 2024
Merged
2 tasks
jphickey added a commit to jphickey/cFE that referenced this issue Apr 18, 2024
@jphickey
Fix nasa#2536, rework msg API test for secondary headers
e83aa11 
For secondary header checks, different functions may (or may not) be
implemented on commands vs telemetry.  The functional test must not
assume one or the other is implemented.  It should dynamically check
both checksum and timestamp based on the return value of the API call.
dzbaker added a commit that referenced this issue May 6, 2024
@dzbaker
Merge pull request #2537 from jphickey:fix-2536-msgapi-buffer
ef14d5a 
Fix #2536, msg api test buffer overrun
@dzbaker dzbaker mentioned this issue May 6, 2024
Merged
2 tasks
dzbaker added a commit that referenced this issue May 21, 2024
@dzbaker
Merge pull request #2537 from jphickey:fix-2536-msgapi-buffer
19a01d2 
Fix #2536, msg api test buffer overrun
@dzbaker dzbaker mentioned this issue May 21, 2024
Merged
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jphickey jphickey

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix #2536, msg api test buffer overrun jphickey/cFE
1 participant
@jphickey