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

Don't try to memcpy() to a NULL destination. #637

Merged
merged 1 commit into from
Nov 12, 2023

Conversation

icculus
Copy link
Contributor

@icculus icculus commented Nov 7, 2023

This function is called with a literal NULL for that pointer later in the file.

We had a user run into this in SDL; checking for NULL before memcpy()'ing resolved it.

This function is called with a literal NULL for that pointer later in
the file.
Copy link
Member

@Youw Youw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good one, thanks.
I wounder why Coverity didn't catch this.
It is clear (and I know it only now, when I checked explicitly for that) that it is an undefined behavior if either of src or dest of memcpy is a null pointer even of the size is 0.

@Youw Youw added bug Something isn't working macOS Related to macOS backend labels Nov 7, 2023
@Youw
Copy link
Member

Youw commented Nov 7, 2023

We had a user run into this in SDL

I wounder what's your user's environment. Clearly that check wasn't required for years, otherwise someone else would hit it by now.
My guess that the implementation of C-runtime library on macOS wasn't as strict as The Standard states.

UPD: OK, I see a discussion under libsdl-org/SDL#8428 so this does look like a fix for a potential (theoretical) issue.
Good to have it anyway, thanks.

@Youw Youw merged commit eea8cac into libusb:master Nov 12, 2023
10 of 11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working macOS Related to macOS backend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants