In this particular case, it is type safe. NULL, by the C standard does not refer to an object. It means that every pointer in C is effectively an option type, which mean that a function that returns a pointer can return NULL for "no result", which, in the case of crypt() is perfectly adequate.
It is the opposite that is unsafe, that is, when the returned value can't be NULL, but it stills returns a pointer. C++ has references in an attempt to fix this problem.
Because C tends to prioritize performance over safety, a NULL pointer dereference is considered undefined behavior and the compiler can invoke nasal demons if it makes the code go faster, but with the right options, you can have it checked and invoke what in other languages would call a panic handler, but on UNIX is the SIGSEGV signal. You can then attach a debugger or read the core file and see what went wrong.
Note: I am not talking about "char *" for strings, that one is unsafe, but NULL is not. It is just a special case of an option type, or a result type, or an error union. Modern languages just extended this idea to objects other than pointers, and with more than one type of error.
The implicitness is exactly the problem. The workaround that caused this vulnerability was done because people assumed that crypt() would never return null.
You don't even need a fancy result type. A simple Option<String> would've forced downstream users to consider the failure case, avoiding this situation in the first place.
The workaround was correctly designed: two error string tokens are used so that one can always be chosen that is not equal to the second argument.
Someone coded a defense against long inputs which ignored this important principle.
What turned the bug into a security disaster is that Musl stupidly chose two symbols such that one of them is exactly the same as a convention for disabled logins. So that the moment someone makes a change to the function which allows crypt("something", "*") to return "*", you have an incident.
If the same bug were made in Glibc, it would have limited impact because crypt("long .... string", "*0") returning "*0", while incorrect, wouldn't do anything in a system that doesn't use *0 in password files. *0 does not correspond to any widespread password field convention.
In theory, having newtons and pounds force be represented as `float` is type safe. In practice, it destroyed a multi-million-dollar NASA spacecraft because of human error.
If NASA were to use the equivalent of `pub struct Newtons(f32); pub struct PoundsForce(f32);`, where the types Newton and PoundsForce were not interchangeable, this would never have happened[1].
[1] I'm aware that that story is a bit more complicated than that, that CSV files were involved (which are, again, not type safe), and that, in that particular scenario, more would have to be done to preserve type safety across formats, vendors and codebases, but my point still stands.
Exactly, whoever program in C knows that every function that returns a pointer may return NULL (even if not documented) and thus every pointer must be checked.
However a better API (to me) that I would have written was:
That way btw the function doesn't need to allocate memory inside, which is (being a mostly embedded programmer) something I don't like at all. Plus returning an error enum makes it clear instead of having to look at errno.
But POSIX API is not something well designed, it dates back to the '80 so... probably today would be written completely differently
We have that better API to some extent; it is called crypt_r. It originated in glibc, evidently, but hasn't made its way to POSIX. It fixes the reentrancy problem of crypt.
It is not immune to the present issue. If our reentrant crypt is given a long password and the salt "", and it decides this is an error, placing the output "" into the result buffer, then it has the problem. It has the problem in a thread-safe way, but it has the problem. :)
> It is just a special case of an option type, or a result type, or an error union.
These are not the same, and your next sentence suggests why. Option has but a single error type, but result types and error unions allow arbitrary types of error.
That depends on how you interpret the pointer type, but in any case there are multiple interpretation and just the fact that these exist will create confusion and type unsafety.
> “When the compiler encounters [a given undefined construct] it is legal for it to make demons fly out of your nose” (the implication is that the compiler may choose any arbitrarily bizarre way to interpret the code without violating the ANSI C standard)
However this is limited by the compiler's actual abilities and code. The C standard doesn't forbid nasal demons, but the compiler isn't programmed to invoke nasal demons and isn't running on hardware with that capability, so it won't happen.
I've heard nasal demons invoked as a literal possibility. No they are not a literal possibility.
You might consider it type safe according to C's rules, but it is not type safe according to contemporary opinion because the success type and error type are the same.
> [...] because the success type and error type are the same.
That's a bit weird? In Rust, if you have an thing that returns Result<String, String> that's still type safe, even though success and error types are the same.
I think I understand what you want to express, maybe. You might say it's in-band vs out-of-band error signalling?
The idea of trying to provide a workaround for buggy code calling crypt() is terrifying. For sure, trying to grease the skids for buggy code calling printf(), go for it. But it is hard to imagine cases where slightly-incorrect usage of crypt() is a good idea.
I don't think so. Even in a language with a good type system like Rust, somebody could have made this API:
/// Hash the phrase with the given setting.
fn crypt(phrase: &[u8], setting: &[u8]) -> [u8; CRYPT_OUTPUT_SIZE];
Then, in a later version, `crypt` needs to become fallible, so they need to signal errors somehow. But because they really want to preserve backward compatibility, they can't change the output type to `Result<..., ...>`. And so they end up doing the same thing that was done in libcrypt - they output some error value and hackily make sure it's not equal to the `setting` string, under the mistaken assumption that the output of `crypt` would only ever be compared to the `setting` string. And then someone creates the exact same bug.
Maybe I'm wrong, but it looks to me like the exact same incentives and means to screw things up exist in C and in Rust for this particular bug.
Specifically, that the lack of tagged unions (or really, structured return types in general; you can pretend those are a tagged union if you believe hard enough) in C library interop conventions is a terrible foundation on which to build widely-used APIs.
And that just ends up indicting the C language itself. Can't send errors down a separate channel; folks might not inspect errno. Can't return NULL on error, folks might compare it blindly to another result. No tagged unions, so can't return one. Can't crash on error because there's no scoped error handling. Can't return a struct for checking .error/.result separately, because alignment and equality (and tearing and accidental clobbering and more) make that brittle as hell for widely-used APIs.
C is not a good language for library APIs. Like, really not. It has its uses/strengths, sure, but without even getting into the typical gripes people have about C, libraries--one of the things that it is used for the most--are one of the things for which it is the least well-suited.
Another example that C programmers love: using 0 to mean infinity. Eg when you have a timeout setting, and 10 means ten seconds, but 0 means infinity seconds instead of zero seconds.
In general it's a problem of sentinel values. Some languages let you abstract this - I think in Rust and C++ you can define an "intrusive optional" which is like optional but uses an invalid value pattern to store the not-present state. In memory it's the same as a sentinel value but the compiler forces you to unwrap it like an optional.
Amusingly, Win API of all places generally evades this pattern: 0 timeout means "no waiting", and to wait indefinitely, you use timeout of INFINITE which is actually UINT_MAX or, equivalently, -1 (in Win API, timeouts are unsigned, not time_t).
That’s just a very bad API. You could do the same in Rust? There are nice C APIs that would use a nice struct that describes the timeout you want… eg GCD.
In this particular case, it is type safe. NULL, by the C standard does not refer to an object. It means that every pointer in C is effectively an option type, which mean that a function that returns a pointer can return NULL for "no result", which, in the case of crypt() is perfectly adequate.
It is the opposite that is unsafe, that is, when the returned value can't be NULL, but it stills returns a pointer. C++ has references in an attempt to fix this problem.
Because C tends to prioritize performance over safety, a NULL pointer dereference is considered undefined behavior and the compiler can invoke nasal demons if it makes the code go faster, but with the right options, you can have it checked and invoke what in other languages would call a panic handler, but on UNIX is the SIGSEGV signal. You can then attach a debugger or read the core file and see what went wrong.
Note: I am not talking about "char *" for strings, that one is unsafe, but NULL is not. It is just a special case of an option type, or a result type, or an error union. Modern languages just extended this idea to objects other than pointers, and with more than one type of error.
The implicitness is exactly the problem. The workaround that caused this vulnerability was done because people assumed that crypt() would never return null.
You don't even need a fancy result type. A simple Option<String> would've forced downstream users to consider the failure case, avoiding this situation in the first place.
The workaround was correctly designed: two error string tokens are used so that one can always be chosen that is not equal to the second argument.
Someone coded a defense against long inputs which ignored this important principle.
What turned the bug into a security disaster is that Musl stupidly chose two symbols such that one of them is exactly the same as a convention for disabled logins. So that the moment someone makes a change to the function which allows crypt("something", "*") to return "*", you have an incident.
If the same bug were made in Glibc, it would have limited impact because crypt("long .... string", "*0") returning "*0", while incorrect, wouldn't do anything in a system that doesn't use *0 in password files. *0 does not correspond to any widespread password field convention.
.unwrap()
Now what?
7 replies →
In theory, having newtons and pounds force be represented as `float` is type safe. In practice, it destroyed a multi-million-dollar NASA spacecraft because of human error.
If NASA were to use the equivalent of `pub struct Newtons(f32); pub struct PoundsForce(f32);`, where the types Newton and PoundsForce were not interchangeable, this would never have happened[1].
[1] I'm aware that that story is a bit more complicated than that, that CSV files were involved (which are, again, not type safe), and that, in that particular scenario, more would have to be done to preserve type safety across formats, vendors and codebases, but my point still stands.
I think related to your caveat is:
https://lexi-lambda.github.io/blog/2020/11/01/names-are-not-...
> The newtype is opaque, so the only way to observe it is to convert it back to a [f32].
Exactly, whoever program in C knows that every function that returns a pointer may return NULL (even if not documented) and thus every pointer must be checked.
However a better API (to me) that I would have written was:
That way btw the function doesn't need to allocate memory inside, which is (being a mostly embedded programmer) something I don't like at all. Plus returning an error enum makes it clear instead of having to look at errno.
But POSIX API is not something well designed, it dates back to the '80 so... probably today would be written completely differently
We have that better API to some extent; it is called crypt_r. It originated in glibc, evidently, but hasn't made its way to POSIX. It fixes the reentrancy problem of crypt.
It is not immune to the present issue. If our reentrant crypt is given a long password and the salt "", and it decides this is an error, placing the output "" into the result buffer, then it has the problem. It has the problem in a thread-safe way, but it has the problem. :)
> It is just a special case of an option type, or a result type, or an error union.
These are not the same, and your next sentence suggests why. Option has but a single error type, but result types and error unions allow arbitrary types of error.
> In this particular case, it is type safe
That depends on how you interpret the pointer type, but in any case there are multiple interpretation and just the fact that these exist will create confusion and type unsafety.
> the compiler can invoke nasal demons if it makes the code go faster
I had never heard that phrase, and was delighted at the answer when I googled it its origin, thank you:
http://www.catb.org/jargon/html/N/nasal-demons.html
> “When the compiler encounters [a given undefined construct] it is legal for it to make demons fly out of your nose” (the implication is that the compiler may choose any arbitrarily bizarre way to interpret the code without violating the ANSI C standard)
However this is limited by the compiler's actual abilities and code. The C standard doesn't forbid nasal demons, but the compiler isn't programmed to invoke nasal demons and isn't running on hardware with that capability, so it won't happen.
I've heard nasal demons invoked as a literal possibility. No they are not a literal possibility.
6 replies →
You might consider it type safe according to C's rules, but it is not type safe according to contemporary opinion because the success type and error type are the same.
> [...] because the success type and error type are the same.
That's a bit weird? In Rust, if you have an thing that returns Result<String, String> that's still type safe, even though success and error types are the same.
I think I understand what you want to express, maybe. You might say it's in-band vs out-of-band error signalling?
8 replies →
Contemporary opinion among who? Words mean things, and "type safe" simply does not mean "agrees with my preferred style of error reporting"
9 replies →
My thoughts exactly.
"Make invalid states unrepresentable" is such a useful adage when designing even basic systems, much less the gates to control your server.
My conclusion is that an old tradition of not returning NULL in crypt as a workaround for buggy code which doesn't handle errors was a bad idea.
The idea of trying to provide a workaround for buggy code calling crypt() is terrifying. For sure, trying to grease the skids for buggy code calling printf(), go for it. But it is hard to imagine cases where slightly-incorrect usage of crypt() is a good idea.
This is still a consequence of a poor type system.
I don't think so. Even in a language with a good type system like Rust, somebody could have made this API:
Then, in a later version, `crypt` needs to become fallible, so they need to signal errors somehow. But because they really want to preserve backward compatibility, they can't change the output type to `Result<..., ...>`. And so they end up doing the same thing that was done in libcrypt - they output some error value and hackily make sure it's not equal to the `setting` string, under the mistaken assumption that the output of `crypt` would only ever be compared to the `setting` string. And then someone creates the exact same bug.
Maybe I'm wrong, but it looks to me like the exact same incentives and means to screw things up exist in C and in Rust for this particular bug.
14 replies →
Specifically, that the lack of tagged unions (or really, structured return types in general; you can pretend those are a tagged union if you believe hard enough) in C library interop conventions is a terrible foundation on which to build widely-used APIs.
And that just ends up indicting the C language itself. Can't send errors down a separate channel; folks might not inspect errno. Can't return NULL on error, folks might compare it blindly to another result. No tagged unions, so can't return one. Can't crash on error because there's no scoped error handling. Can't return a struct for checking .error/.result separately, because alignment and equality (and tearing and accidental clobbering and more) make that brittle as hell for widely-used APIs.
C is not a good language for library APIs. Like, really not. It has its uses/strengths, sure, but without even getting into the typical gripes people have about C, libraries--one of the things that it is used for the most--are one of the things for which it is the least well-suited.
"But it has a stable ABI!" So does a land mine.
you can return a struct just fine... most API's don't though.
Historically it didn't work well between different compilers. Maybe it still doesn't.
2 replies →
That and in-band signalling is error-prone.
Another example that C programmers love: using 0 to mean infinity. Eg when you have a timeout setting, and 10 means ten seconds, but 0 means infinity seconds instead of zero seconds.
In general it's a problem of sentinel values. Some languages let you abstract this - I think in Rust and C++ you can define an "intrusive optional" which is like optional but uses an invalid value pattern to store the not-present state. In memory it's the same as a sentinel value but the compiler forces you to unwrap it like an optional.
I'm not sure how well the 'forcing' works for Rust and C++. You can definitely do this kind of thing in dependently typed languages.
2 replies →
Amusingly, Win API of all places generally evades this pattern: 0 timeout means "no waiting", and to wait indefinitely, you use timeout of INFINITE which is actually UINT_MAX or, equivalently, -1 (in Win API, timeouts are unsigned, not time_t).
That’s just a very bad API. You could do the same in Rust? There are nice C APIs that would use a nice struct that describes the timeout you want… eg GCD.
I'm not sure a struct is the right thing here in general. You often want an algebraic datatype.
What Rust calls an enum, but an enum with payloads. In C terms it would be a discriminated union.