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

Fix #808, length-limited string length checks #810

Merged
merged 1 commit into from
Feb 12, 2021

Conversation

jphickey
Copy link
Contributor

Describe the contribution

Create a wrapper around memchr() that mimics the non-C99 function strnlen() which is defined in POSIX-2008.

Use this instead of strlen() whenever the string being checked either originates in or will be copied into a fixed-length array buffer.

Fixes #808

Testing performed
Build and sanity check CFE
Run all unit tests on both native and RTEMS

Expected behavior changes
No behavior changes except if a bug causes strings to be unterminated.

System(s) tested on
Ubuntu 20.04 (native)
RTEMS 4.11.3 (qemu)

Additional context
Worth noting that in most cases this was testing the length of a string in the internal OSAL table entry, which was already length-checked when created. So this check is somewhat redundant, the only way this could catch something is if there is memory corruption of some sort.

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

Create a wrapper around memchr() that mimics the non-C99 function
"strnlen()" which is in POSIX-2008.

Use this instead of strlen() whenever the string being checked
either originates in or will be copied into a fixed-length array buffer.
@jphickey jphickey added the CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) label Feb 12, 2021
@jphickey
Copy link
Contributor Author

Note - in places where it doesn't actually care what the length is and only checking it is terminated within the limit are replaced with a direct call to memchr() rather than going through the OS_strnlen() wrapper. Places where the length is actually used afterward should use the wrapper.

@skliper skliper added this to the 6.0.0 milestone Feb 12, 2021
@skliper
Copy link
Contributor

skliper commented Feb 12, 2021

@astrogeco requesting fasttrack, no behavior changes.

@astrogeco astrogeco changed the base branch from main to integration-candidate February 12, 2021 21:32
@astrogeco astrogeco merged commit 842fe88 into nasa:integration-candidate Feb 12, 2021
@astrogeco astrogeco added CCB:2021-02-17 and removed CCB:Ready Pull request is ready for discussion at the Configuration Control Board (CCB) labels Feb 17, 2021
@jphickey jphickey deleted the fix-808-strnlen branch April 28, 2021 18:58
jphickey pushed a commit to jphickey/osal that referenced this pull request Aug 10, 2022
jphickey pushed a commit to jphickey/osal that referenced this pull request Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Convert from strlen to strnlen where appropriate
3 participants