Hacker News
points by annex-winged-cr 1 day ago | 0 comments
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.

[–] kazinator 1 day ago
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.

[–] f33d5173 1 day ago
.unwrap()

Now what?

[–] ahepp 1 day ago
I think the argument would be that in C people will look at a char* and think "string" rather than "string or null". Thus consumers of the library not checking the result and the maintainer trying to be helpful by putting a "safe" value in there for the error case.

Whereas if the library returns something that is unambiguously an option type, consumers unambiguously call unwrap at their own risk.

Of course you do have a point (assuming I'm picking up what you're suggesting correctly) that all pointers in C are option types. But I think there's a cognitive ergonomics aspect to this where an option type more clearly relays the library authors' intent.

[–] shakow 1 day ago
First you shouldn't have unwraps in prod code, and you can verify that in your CI/build process/whatever. Second, now you have a crash of the SSH server, which is arguably much better than some random logging in as root.
[–] f33d5173 1 day ago
This isn't about "you", it's about whether any consumer of your code is using unwrap on the result, which currently is perfectly safe to do. The equivalent in C is returning NULL, which would also have caused a crash. They decided not crashing was preferable, which is how we ended up here.
[–] kazinator 1 day ago
"you shouldn't have unwraps in production code" is exactly the same form of argument as "you shouldn't be assuming crypt returns a valid non-null pointer in production code".
[–] dadoum 22 hours ago
Absolutely not. Unwrap is a language rule, so everyone knows unwrap is not a safe thing to do and writing it even once is an explicit dismissal (and you should document it). On the other side, knowing that crypt does not necessarily return a hash is easily overlooked and forgotten, there is no code smell that can hint you that you are doing things wrong.
[–] shakow 1 day ago
I don't really care about the form of the argument when the concrete results are that (i) “no unwrap in prod” is trivial to ensure through the toolchain, (ii) even though, its consequences are “damn it crashed” instead of “welp I guess this guy is root now”.
[–] kazinator 3 hours ago
In this situation, the decisions to make crypt implementations stop returning null in favor of a string error token were made not because C programs that misuse crypt are difficult to fix, but because they are in the form of unknown binaries in the wild that you cannot fix.

And that analogous situation can happen in the safer language where crypt returns an Optional(String) which some bad programs assume is always present. You don't know where all such programs exist, and people use them who don't have the code and cannot fix them.

Returning an error token which is carefully different from crypt's second argument was an attempt to fix programs that cannot be hunted down, patched and recompiled, even if fixing them would be easy to do.

Of course the tooling for C pointers being null or not is not comparable to optional types; that's not debatable.

[–] eviks 17 hours ago
Now you have an easy way to discover the bug