This specific manifestation of "C is terrible for shared code APIs" is rare.
But issues caused by folks forgetting to check errno, miscomparing NULL to NULL when calling multiple APIs, dereferencing external-API-returned NULL due to missed error checking, mishandling inherited errno after successful action, or--the most common--mishandling negative integer return sentinels meant to indicate errors? Those are incredibly common.
This specific library call is quite poorly designed.
Standard C api calls return an integer, which is 0 on success or nonzero on error. Ignoring it is an intentional choice. `errno` is the hack that shouldn't really exist because it's trivially easy to forget to check that and also has issues with threading (patched by making it a thread local) and more recently, coroutines (which admit no general purpose patch because they're a userspace construct).
Another reason why this is terrible is that it's unclear what the returned pointer's lifetime is. Is it until the next call to crypt? If so, is it thread-local like errno? If it's passed on to the user, is it safe to call free() on it, or is there a crypt_free function that should be called instead?
But all of this, and I mean all of it pales in comparison to returning '*' on error.
> But all of this, and I mean all of it pales in comparison to returning '*' on error.
100% agree. This is an egregious failure. But C's error-handling story would be bad even without this, and the badness of C's error handling is what motivated the (bad) decision to return *.
It'd be great if we wrote systems software in a language that supported advanced features like...
checks notes
...returning multiple values.
Hell, even in VBA you had to at least spend the keystrokes to type "on error resume next".
It's not a C thing, it's a Linux thing. The asterisk is relevant to how crypt() is used for /etc/shadow.
If the password field contains some string that is not a valid result of crypt(3), for instance ! or *, the user will not
be able to use a unix password to log in (but the user may log in the system by other means).
No, it's a C thing. They previously returned NULL for invalid passwd lookups, but because there's no way in C to force API users to decide what to do if an error occurrs, that was causing program crashes due to NULL dereferences.
So they made the (very bad, but motivated by C's broken design/conventions) decision to return a magic string instead.
> but because there's no way in C to force API users to decide what to do if an error occurrs,
0. There are tons of libc functions that return NULL on failure, it is conventional way in C to indicate failure.
1. There's no much to decide, just fail to login if crypt() fails with NULL.
2. errno is standard way in C to check error code of a failing function, but in case of login you hardly need this, just fail!
I totally agree that fixing this issue in musl was a pretty dubious solution. Don't fix what's not broken, fix broken application instead. Publish a CVE if you cannot fix it.
I looked through FreeBSD source code for how they use crypt(), it's checked for NULL everywhere!
Returning NULL on error is actually an excellent choice because most programs will crash if they try to use it without checking. A program, any program, crashing is better than this bug. Of course, modern programming languages have better constructs, but this was rare to nonexistent when C was started and if you think C is going away anytime soon you're delirious. musl's decision to change the return value to "*" is ununderstandable and likely intentionally evil.
This specific manifestation of "C is terrible for shared code APIs" is rare.
But issues caused by folks forgetting to check errno, miscomparing NULL to NULL when calling multiple APIs, dereferencing external-API-returned NULL due to missed error checking, mishandling inherited errno after successful action, or--the most common--mishandling negative integer return sentinels meant to indicate errors? Those are incredibly common.
This specific library call is quite poorly designed.
Standard C api calls return an integer, which is 0 on success or nonzero on error. Ignoring it is an intentional choice. `errno` is the hack that shouldn't really exist because it's trivially easy to forget to check that and also has issues with threading (patched by making it a thread local) and more recently, coroutines (which admit no general purpose patch because they're a userspace construct).
Another reason why this is terrible is that it's unclear what the returned pointer's lifetime is. Is it until the next call to crypt? If so, is it thread-local like errno? If it's passed on to the user, is it safe to call free() on it, or is there a crypt_free function that should be called instead?
But all of this, and I mean all of it pales in comparison to returning '*' on error.
> But all of this, and I mean all of it pales in comparison to returning '*' on error.
100% agree. This is an egregious failure. But C's error-handling story would be bad even without this, and the badness of C's error handling is what motivated the (bad) decision to return *.
It'd be great if we wrote systems software in a language that supported advanced features like...
checks notes
...returning multiple values.
Hell, even in VBA you had to at least spend the keystrokes to type "on error resume next".
1 reply →
It's not a C thing, it's a Linux thing. The asterisk is relevant to how crypt() is used for /etc/shadow.
No, it's a C thing. They previously returned NULL for invalid passwd lookups, but because there's no way in C to force API users to decide what to do if an error occurrs, that was causing program crashes due to NULL dereferences.
So they made the (very bad, but motivated by C's broken design/conventions) decision to return a magic string instead.
> but because there's no way in C to force API users to decide what to do if an error occurrs,
0. There are tons of libc functions that return NULL on failure, it is conventional way in C to indicate failure.
1. There's no much to decide, just fail to login if crypt() fails with NULL.
2. errno is standard way in C to check error code of a failing function, but in case of login you hardly need this, just fail!
I totally agree that fixing this issue in musl was a pretty dubious solution. Don't fix what's not broken, fix broken application instead. Publish a CVE if you cannot fix it.
I looked through FreeBSD source code for how they use crypt(), it's checked for NULL everywhere!
UPD: Except libpam. :(
1 reply →
Returning NULL on error is actually an excellent choice because most programs will crash if they try to use it without checking. A program, any program, crashing is better than this bug. Of course, modern programming languages have better constructs, but this was rare to nonexistent when C was started and if you think C is going away anytime soon you're delirious. musl's decision to change the return value to "*" is ununderstandable and likely intentionally evil.
4 replies →
Now imagine every other C API that can potentially return NULL did the same thing. Is that in any way sensible?
You can argue that C has broken design/conventions, but the musl decision was really really awkward.
This sounds like a program problem not a library problem. Why did the library try to solve it?
Oh, it was magic alright.