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

fd_set coversions in select impl can read beyond the end of OS_impl_filehandle_table #919

Closed
jphickey opened this issue Mar 22, 2021 · 0 comments · Fixed by #920 or #917
Closed

fd_set coversions in select impl can read beyond the end of OS_impl_filehandle_table #919

jphickey opened this issue Mar 22, 2021 · 0 comments · Fixed by #920 or #917
Assignees
Labels
Milestone

Comments

@jphickey
Copy link
Contributor

Describe the bug
The loop inside OS_FdSet_ConvertIn_Impl and OS_FdSet_ConvertOut_Impl is limited by sizeof(OS_FdSet), which itself is sized to accommodate OS_MAX_NUM_OPEN_FILES as a bit mask.

The problem is that the size is (necessarily) padded up to a multiple of 8 bits. If OS_MAX_NUM_OPEN_FILES was not a multiple of 8, and some of these "padding" bits are set as 1, these functions will attempt to read entries beyond the end of OS_impl_filehandle_table.

To Reproduce
In normal use cases where the correct API is used (e.g. OS_SelectFdAdd()) it is not possible to set these extra bits - as the OS_SelectFdAdd() checks if the filehandle is valid before setting the bit.

But in coverage tests, the structure is memset() to all ones (0xFF) which causes undefined behavior as it will end up reading beyond the end of the array.

Expected behavior
Must not read beyond the end of the array even if extra bits are set.

System observed on:
Ubuntu 20.04

Additional context
Observed as failure in #917. This issue was not introduced by those merges, it just so happens that it changed the preconditions such this became exposed.

Reporter Info
Joseph Hickey, Vantage Systems, Inc.

@jphickey jphickey self-assigned this Mar 22, 2021
@jphickey jphickey added the bug label Mar 22, 2021
jphickey added a commit to jphickey/osal that referenced this issue Mar 22, 2021
Add an extra limit check for the index, as it is possible
due to padding that this goes beyond the end of the array.
astrogeco added a commit that referenced this issue Mar 22, 2021
Fix #919, check index inside fdset conversions
@astrogeco astrogeco added this to the 6.0.0 milestone Mar 22, 2021
jphickey pushed a commit to jphickey/osal that referenced this issue Aug 10, 2022
Fix nasa#888, Add typedef for function return status codes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants