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 out-of-bounds string manipulation #167

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

mcobbice
Copy link

The string postbackUri contains the string "libredfish" (10 characters, + 1 null byte), the address 'postbackUri+11' actually points past the end of the string (inc. null byte). This pointer is eventually passed as an argument to strstr().

This out of bounds access causes a segfault on architectures which enforce pointer bounds, for example, CHERI architectures such as ARM Morello.

@pboyd04
Copy link
Contributor

pboyd04 commented Sep 17, 2024

This isn't correct. The string should not just be "libredfish" it should be something like "libredfish:eth0:ipv4" to get a listener for eth0 interface that listens to events via IPv4. Or "libredfish:vlan.4003:ipv6" to get a listener for VLAN 4003 that listens to events via IPv6.

@mcobbice
Copy link
Author

This isn't correct. The string should not just be "libredfish" it should be something like "libredfish:eth0:ipv4" to get a listener for eth0 interface that listens to events via IPv4. Or "libredfish:vlan.4003:ipv6" to get a listener for VLAN 4003 that listens to events via IPv6.

I see, thank you for the clarification. let me take another look at this

If registerForEvents() is called with "libredfish" for the postbackUri
parameter, the function accepts this string and passes a pointer to
'postbackUri+11' to getDestinationAddress. In this case, the pointer actually
points past the end of the string's null byte.
On CHERI architectures, such as ARM Morello, pointer bounds are enforced in
hardware and attempting to dereference the pointer passed to
getDestinationAddress() causes a segfault.

Valid values for postbackUri should include a colon after "libredfish",
checking for this as part of the strncmp call rejects the invalid string
"libredfish" and this also means that getDestinationAddress() is not passed
an invalid pointer. This prevents a segfault on CHERI and prevents undefined
behavior on other architectures.

Signed-off-by: Michael Cobb <[email protected]>
@mcobbice
Copy link
Author

@pboyd04 If my understanding is correct then we should actually include a check for a colon after "libredfish", and passing "libredfish" as the postbackUri is invalid?
Please see if 6ab1332 is correct

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants