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

Fix #2536, msg api test buffer overrun #2537

Merged

dzbaker merged 1 commit into nasa:main from jphickey:fix-2536-msgapi-buffer

May 21, 2024

Contributor

jphickey commented Mar 26, 2024

Checklist (Please check before submitting)

I reviewed the Contributing Guide.
I signed and emailed the appropriate Contributor License Agreement to [email protected] and copied [email protected].

Describe the contribution
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.

Fixes #2536

Testing performed
Run functional tests

Expected behavior changes
No buffer overrun reports when using -fsanitize=address

System(s) tested on
Debian

Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.

jphickey added the CCB:Ready label

jphickey force-pushed the fix-2536-msgapi-buffer branch from 2c55e70 to e83aa11 Compare

April 18, 2024 15:26

github-advanced-security bot found potential problems

View reviewed changes

modules/cfe_testcase/src/msg_api_test.c

-                  UtAssert_INT32_EQ(CFE_MSG_SetType(CFE_MSG_PTR(cmd2), CFE_MSG_Type_Cmd), CFE_SUCCESS);
+                  /* test generate-checksum on commands */
+                  CFE_Assert_STATUS_STORE(CFE_MSG_ValidateChecksum(CFE_MSG_PTR(cmd), &isValid));
+                  if (CFE_Assert_STATUS_MAY_BE(CFE_SUCCESS))

Check warning

Code scanning / CodeQL-coding-standard

Side effect in a Boolean expression Warning test

This Boolean expression is not side-effect free.

modules/cfe_testcase/src/msg_api_test.c

+                      UtAssert_INT32_EQ(CFE_MSG_ValidateChecksum(CFE_MSG_PTR(cmd), &isValid), CFE_SUCCESS);
+                      UtAssert_BOOL_TRUE(isValid);
+                  }
+                  else if (CFE_Assert_STATUS_MAY_BE(CFE_MSG_WRONG_MSG_TYPE))

Check warning

Code scanning / CodeQL-coding-standard

Side effect in a Boolean expression Warning test

This Boolean expression is not side-effect free.

modules/cfe_testcase/src/msg_api_test.c

+                  /* test generate-checksum on telemetry */
+                  CFE_Assert_STATUS_STORE(CFE_MSG_ValidateChecksum(CFE_MSG_PTR(tlm), &isValid));
+                  if (CFE_Assert_STATUS_MAY_BE(CFE_SUCCESS))

Check warning

Code scanning / CodeQL-coding-standard

Side effect in a Boolean expression Warning test

This Boolean expression is not side-effect free.

modules/cfe_testcase/src/msg_api_test.c Fixed Show fixed Hide fixed

modules/cfe_testcase/src/msg_api_test.c

-                  if (!CFE_Assert_STATUS_MAY_BE(CFE_SUCCESS))
+                  /* Check if GetMsgTime is implemented on commands */
+                  CFE_Assert_STATUS_STORE(CFE_MSG_GetMsgTime(CFE_MSG_PTR(cmd), &msgTime));
+                  if (CFE_Assert_STATUS_MAY_BE(CFE_SUCCESS))

Check warning

Code scanning / CodeQL-coding-standard

Side effect in a Boolean expression Warning test

This Boolean expression is not side-effect free.

modules/cfe_testcase/src/msg_api_test.c

-                  if (!CFE_Assert_STATUS_MAY_BE(CFE_SUCCESS))
+                  /* Check if GetMsgTime is implemented on telemetry */
+                  CFE_Assert_STATUS_STORE(CFE_MSG_GetMsgTime(CFE_MSG_PTR(tlm), &msgTime));
+                  if (CFE_Assert_STATUS_MAY_BE(CFE_SUCCESS))

Check warning

Code scanning / CodeQL-coding-standard

Side effect in a Boolean expression Warning test

This Boolean expression is not side-effect free.


          Fix nasa#2536, rework msg API test for secondary headers

24a2b21

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.

jphickey force-pushed the fix-2536-msgapi-buffer branch from e83aa11 to 24a2b21 Compare

April 18, 2024 15:34

github-advanced-security bot found potential problems

View reviewed changes

modules/cfe_testcase/src/msg_api_test.c

+                      UtAssert_INT32_EQ(CFE_MSG_ValidateChecksum(CFE_MSG_PTR(tlm), &isValid), CFE_SUCCESS);
+                      UtAssert_BOOL_TRUE(isValid);
+                  }
+                  else if (CFE_Assert_STATUS_MAY_BE(CFE_MSG_WRONG_MSG_TYPE))

Check warning

Code scanning / CodeQL-coding-standard

Side effect in a Boolean expression Warning test

This Boolean expression is not side-effect free.

modules/cfe_testcase/src/msg_api_test.c

-                  CFE_Assert_STATUS_STORE(CFE_MSG_SetMsgTime(CFE_MSG_PTR(cmd2), currentTime));
-                  if (!CFE_Assert_STATUS_MAY_BE(CFE_SUCCESS))
+                  else if (CFE_Assert_STATUS_MAY_BE(CFE_MSG_WRONG_MSG_TYPE))

Check warning

Code scanning / CodeQL-coding-standard

Side effect in a Boolean expression Warning test

This Boolean expression is not side-effect free.

modules/cfe_testcase/src/msg_api_test.c

+                      UtAssert_INT32_EQ(CFE_MSG_GetMsgTime(CFE_MSG_PTR(tlm), &msgTime), CFE_SUCCESS);
+                      UtAssert_UINT32_EQ(CFE_TIME_Compare(msgTime, currentTime), CFE_TIME_EQUAL);
+                  }
+                  else if (CFE_Assert_STATUS_MAY_BE(CFE_MSG_WRONG_MSG_TYPE))

Check warning

Code scanning / CodeQL-coding-standard

Side effect in a Boolean expression Warning test

This Boolean expression is not side-effect free.

Contributor Author

jphickey commented Apr 18, 2024

Should be ready now. This now has two separate buffers (cmd and tlm) and allows either operation (checksum, timestamp) to independently work or not work on either buffer.

jphickey requested a review from skliper

April 18, 2024 15:43

skliper approved these changes

View reviewed changes

dzbaker added CCB:Approved and removed CCB:Ready labels

dzbaker added a commit that referenced this pull request


          Merge pull request #2537 from jphickey:fix-2536-msgapi-buffer

ef14d5a

Fix #2536, msg api test buffer overrun

dzbaker mentioned this pull request

cFE Integration candidate: Equuleus-rc1+dev13 #2555

Merged

2 tasks

dzbaker added a commit to nasa/cFS that referenced this pull request


          IC: equuleus-rc1+dev12

770d1e7

*Combines:*

cFE equuleus-rc1+dev141
osal equuleus-rc1+dev66
PSP equuleus-rc1+dev42

**Includes:**

*cFS*
- #751
- #762

*cFE*
- nasa/cFE#2537
- nasa/cFE#2381
- nasa/cFE#2551

*osal*
- nasa/osal#1460

*PSP*
- nasa/PSP#430

Co-authored by: Justin Figueroa <[email protected]>
Co-authored by: Joseph Hickey <[email protected]>

dzbaker mentioned this pull request

IC: equuleus-rc1+dev12 nasa/cFS#763

Merged

2 tasks

dzbaker mentioned this pull request

cFE Integration candidate: Equuleus-rc1+dev12 #2563

Merged

2 tasks

dzbaker added a commit to nasa/cFS that referenced this pull request


          IC: equuleus-rc1+dev12

0d11741

*Combines:*

cFE equuleus-rc1+dev137
osal equuleus-rc1+dev66
PSP equuleus-rc1+dev42
to_lab equuleus-rc1+dev56

**Includes:**

*cFS*
- #751
- #762

*cFE*
- nasa/cFE#2537
- nasa/cFE#2525

*osal*
- nasa/osal#1460

*PSP*
- nasa/PSP#430

*to_lab*
- nasa/to_lab#198

Co-authored by: Justin Figueroa <[email protected]>
Co-authored by: Joseph Hickey <[email protected]>
Co-authored by: Cody Martin <[email protected]>

dzbaker merged commit 19a01d2 into nasa:main

22 checks passed

dzbaker added a commit to nasa/cFS that referenced this pull request


          IC: equuleus-rc1+dev12

76826c7

*Combines:*

cFE equuleus-rc1+dev137
osal equuleus-rc1+dev66
PSP equuleus-rc1+dev42
to_lab equuleus-rc1+dev56

**Includes:**

*cFS*
- #751
- #762

*cFE*
- nasa/cFE#2537
- nasa/cFE#2525

*osal*
- nasa/osal#1460

*PSP*
- nasa/PSP#430

*to_lab*
- nasa/to_lab#198

Co-authored by: Justin Figueroa <[email protected]>
Co-authored by: Joseph Hickey <[email protected]>
Co-authored by: Cody Martin <[email protected]>
Co-authored by: Dan Knutsen <[email protected]>

jphickey deleted the fix-2536-msgapi-buffer branch

May 29, 2024 14:20

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels