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

UtAssert_StringBufCompare SEGV when -1 for size passed in (on CentOS 7) #1237

Closed
skliper opened this issue Mar 25, 2022 · 0 comments · Fixed by #1238 or #1239
Closed

UtAssert_StringBufCompare SEGV when -1 for size passed in (on CentOS 7) #1237

skliper opened this issue Mar 25, 2022 · 0 comments · Fixed by #1238 or #1239
Assignees
Labels
draco-rc1 unit-test Tickets related to the OSAL unit testing (functional and/or coverage)
Milestone

Comments

@skliper
Copy link
Contributor

skliper commented Mar 25, 2022

Describe the bug
memchr(x, x, -1) returns NULL on CentOS 7 (at least in one case), which within UtAssert_StringBufCompare causes the memcmp to use the max size_t which causes a SEGV.

bool UtAssert_StringBufCompare(const char *String1, size_t String1Max, const char *String2, size_t String2Max,
UtAssert_Compare_t CompareType, const char *File, uint32 Line)
{
char ScrubbedString1[256];
char ScrubbedString2[256];
const char *EndPtr1;
const char *EndPtr2;
size_t FormatLen1;
size_t FormatLen2;
bool Result;
int Compare;
/* Locate the actual end of both strings */
if (String1 == NULL)
{
EndPtr1 = NULL;
}
else
{
EndPtr1 = memchr(String1, 0, String1Max);
}
if (EndPtr1 != NULL)
{
FormatLen1 = EndPtr1 - String1;
}
else
{
FormatLen1 = String1Max;
}
if (String2 == NULL)
{
EndPtr2 = NULL;
}
else
{
EndPtr2 = memchr(String2, 0, String2Max);
}
if (EndPtr2 != NULL)
{
FormatLen2 = EndPtr2 - String2;
}
else
{
FormatLen2 = String2Max;
}
if (FormatLen1 == 0 && FormatLen2 == 0)
{
/* Two empty strings are considered equal */
Compare = 0;
}
else

To Reproduce
Build and run this test case (others also use this pattern):
https://github.com/nasa/cFE/blob/a39b0a65fb2724692469ff492484b523fb4fa7e6/modules/config/ut-coverage/test_cfe_config.c#L138-L146

Expected behavior
No SEGV

Code snips
See above

System observed on:

  • Hardware: unknown
  • OS: CentOS 7
  • Versions: Bundle main

Additional context
Add any other context about the problem here.

Reporter Info
Jacob Hageman - NASA/GSFC

@skliper skliper added the unit-test Tickets related to the OSAL unit testing (functional and/or coverage) label Mar 25, 2022
@skliper skliper added this to the Draco milestone Mar 25, 2022
jphickey added a commit to jphickey/osal that referenced this issue Mar 25, 2022
In some systems, passing a large size value to memchr() causes it to
return NULL, even if the char is guaranteed to be found within the
actual valid buffer memory.

This modifies the string buffer comparison function to actively
check for this sentinel value and use "strlen()" instead.
jphickey added a commit to jphickey/osal that referenced this issue Mar 25, 2022
In some systems, passing a large size value to memchr() causes it to
return NULL, even if the char is guaranteed to be found within the
actual valid buffer memory.

This modifies the string buffer comparison function to actively
check for this sentinel value and use "strlen()" instead.
astrogeco added a commit that referenced this issue Mar 25, 2022
Fix #1237, avoid calling memchr() with unknown size buffer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
draco-rc1 unit-test Tickets related to the OSAL unit testing (functional and/or coverage)
Projects
None yet
2 participants