Comment by mpyne
2 days ago
OK, I think I see what’s going on with this, in this configuration.
The goal from an attacker’s perspective is to get this to be true: `$ret = crypt($pw, $hash); $ret == $hash` when called by OpenSSH’s sshd. This is how sshd validates that the user logging in knew the account password.
The `$hash` is the “settings” to the crypt call and will normally look like “$6$--gibberish” for users with passwords set, but could look like “*” or “!” for users not intended to be able to login.
The bug is from the intersection of all of these things:
1. musl can return “*” for errors, rather than leave the return value NULL. Other libc have adopted something like this too, but seem to use a different result “*0”.
2. musl can also return “+” for errors, but only does so if $hash starts with “*”. This is intended to eliminate the possibility of an error causing `$ret == $hash` to be true, however it assumes that applications calling `crypt()` will set $pw and $hash to values corresponding to the `$ret == $hash` comparison to be made immediately afterwards.
3. OpenSSH can be convinced to pass the hash of a different user if the hash is “*” or “!” to avoid timing attacks that can enumerate the user list, so it will set $pw and $hash to correspond to different users. $hash will be a real “$6$--gibberish” style hash in this case, and OpenSSH expects `crypt()` to either return NULL or return a hash string. But now there’s two hashes OpenSSH uses. It does: `$ret = crypt($pw, $hash2); $ret == $hash1`;
4. This should normally not be a problem because OpenSSH will pass in valid $pw and $hash2 that should not lead to errors in hashing, so the $ret that musl generates should be a real hash that cannot match the “*” or “!” $hash1 for the login-disabled user. However as a DDoS mitigation, musl will return error if the password is too long when using SHA-2-based hashes (the default, it seems).
5. This error will result in a non-NULL result of “*” or “+”. Since OpenSSH passed in a real hash ($hash2), musl will return “*”, which compares equal to the actual user hash $hash1 for the disabled-login user.
The OpenSSL adjustment ensures a minimum hash length for the user before logging them in, even if crypt() succeeds somehow.
I think the conclusion that can be drawn from this is that the behavior of crypt() is howling-at-the-moon insane, and it should no longer be considered for any system built in this century, at least not without extensive wrappers that correct and normalize all these edge cases.
Conclusion is type safety is important
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.
45 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.
17 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.
4 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.
7 replies →
Not really. This issue is caused purely by the handling of the too-long input in the wrong way.
Firstly, why do crypt functions return a valid string to indicate an error? The problem is that the classic null return makes some programs crash which are not expecting it.
musl has a correct protocol for the error string, similar to the *0/*1 convention. It also uses two symbols, * and +. If the salt string is *, then it uses \+ specifically to thwart this issue: the program blindly assuming that the password is correct because the output of the function matches the hashed password field.
Where it seems they screwed up is this:
I.e. in this special anti-DDoS measure that checks for a too long input, they forgot to implement the key piece of logic that the output of the function, when an error is being reported, must be different from its second argument (the "salt", which is usually the entire hashed password or the contents of that field from a password file).
What we want is:
Thus the application ends up comparing "*" and "+", or "+" and "*" so the authentication fails.
Under no circumstances should "*" be returned when the second argument is "*", or "+" when it is "+", because it must be assumed that the app doesn't do any sanity checking, and blindly compares the second argument to the return value.
musl does implement it this way.
However OpenSSH passes a different hash to musl’s crypt() than what it later uses to compare, so musl doesn’t switch to the alternate return output because from its perspective, it’s already guaranteed not to accidentally compare the same as the input hash.
The conclusion is, again!, that C is an outdated tool for secrure software.
Is really the solution to a bug where a function had a wrapper that tried to solve its documented issue of returning *, to make yet another wrapper to try to solve the documented issue of returning true?
But, but 'always "succeeding" but giving an unmatchable hash is reportedly a better behavior than failing on error.'!!
Caveat:
> $ret = crypt($pw, $hash); $ret == $hash
musl is actually safe against this use. OpenSSL takes a bizarre code path, detects that $hash is too short, calls crypt($pw, $other_string), forgets that it detected that the hash was too short, and checks $hash == return value.
See my other comment.
I did note that musl would normally work just fine here. Though I appreciate that your other comment goes into much better detail about what was actually going on in the code, as my analysis basically stopped once I understood the high-level flow of what was going on.
Except OpenSSH qould have worked just fine if musl behaved correctly. Instead musl ignored the spec and tried to hack around badly written programs but ended up creating a security hole in a correctly written one!
This is a vastly clearer explanation than the linked page. Thank you!
Yep, the linked page doesn't /feel/ AI written (maybe it's using a grok or muse model whose tone I don't quite recognize yet?) but it kinda just states a lot of facts that make sense on their own but don't cohere into a whole vulnerability like the comment above does.
I think it's just Claude prompted to write in a casual, conversational, no caps style.
The surface-level hints are gone, but the attempt to make every sentence pithy, the decisions on what information to present and what to assume the audience knows, and the over-emotionalization are all still there.
It feels very AI written, it's just not the blatantly AI default style. Pretty much any prompt with a "write X in this style: [example text]" will produce writing like this.
Of course it is. "one debian box, one alpine box" is an obvious Claudeism.
2 replies →
Because Claude wrote the linked page. Claude is impressively bad at communicating these days. with gpt-5.6+ approaching Claude level like 30% of time and Gemini much clearer but also the most annoying 7 year old ever, it’s a pretty lame environment especially for people who r new to everything
I wondered about this myself as the explanations and documents I have been maintaining are becoming increasingly 'slop', but a different kind of slop.
In the past I could get an explanation, where I was able to track the line of thought. Now when I read through the explanation it is like someone has taken two or three explanations and merged them into each other, like there is a combining of iterations without clarity.
example:
The tree is blue.
The tall tree has the color of blue.
the tree, while tall, is blue.
That finally gets squished into: the blue tall tree has height and is evident of blue.
Is this double speak or injected double effort of words?
Until Opus 5.0 I'd have agreed with you, but Opus 5.5 is honestly one of the best I've ever sparred with, vastly better than the rest, including Astra imho. Especially so when prompting for clear top-down, first-principles communication.
4 replies →
Some of the page is also just wrong.
Except it turns out that at least one of those things is because I didn't enable JS, and the static text is wrong!
(Turning on JS did not actually help, because then the page is filled with horrible, cheesy animations and I can't actually read any of it.)
Thank you for explaining it with proper capitalization. I understood your explanation much better than the author's.
> musl can return “*” for errors
wtaf ?
There is no doubt in my mind that this was intentional.
I agree. The “excuse” for implementing those behaviors is just providing plausible deniability. The commit that lets crypt return “*”, followed closely by a change that lets you easily trigger that behavior sounds just like how somebody would sneakily introduce a back-door into tons of linux systems while making it look as innocent as possible.
Well, it was intentional, but not in the way I think you mean.
musl switched to returning `*` so folks stopped accidentally using NULL results, which was causing crashes and risked false-positives comparing NULL to itself.
That was a misfeature and should never have been implemented. But I bet it was an easy band-aid change to make!
35 replies →
Yep. See: underhanded c code contest
https://www.underhanded-c.org/
Of all the possible strings this seems like the absolute worst choice. If I were typing that string I'd pause to think "wait, what if the caller uses this with a regex?"
One has to remember the context here. The only error possible according to the SUS is ENOSYS, literally that crypt() is not actually implemented in the library. The traditional AT&T Unix crypt() was always implemented, and its encryption did not have failure modes, so there was never any error return to handle. This is why programs were written to never check the result of crypt(). It was only ever NULL on systems where one couldn't use the mechanism at all in the first place.
So the knock-on effect is that libraries that do have errors other than ENOSYS to accommodate, have to return magic 'impossible' values instead of setting errno and returning NULL, because of the weight of existing applications that expect nothing but the standard, traditional AT&T Unix (and indeed old BSD), behaviour.
Check the GNU libc crypt() doco. It defines a whole family of 'impossible' values that signal errors, by dint of listing a load of magic characters that should never be in the encrypted string. Including asterisk.
Also check FreeBSD's OpenPAM for a case in point. Even today, it does not check for a NULL return from crypt() and leaps straight to strcmp().
* https://cgit.freebsd.org/src/tree/lib/libpam/modules/pam_uni...
If you want to see real history here, check out the 1992 comment by Eric Andrew Young, that is still in the source for OpenBSD today (in des/des_fcrypt.c), on how SSLeay's crypt() had to be adjusted so that any single character password field, including "*", did not end up successfully matching itself.
* https://cvsweb.openbsd.org/annotate/src/lib/libcrypto/des/At...
It's worth pointing out that musl tried to do the same as SSLeay, and deliberately returns something different to the original salt so that they will not compare equal. It does not return "*" if the input is actually "*" in the first place, contrary to the headlined cartoon explanation. (The locus of the problem is the caller calling it with a salt that is not "*" and then comparing it with "*".)
> Check the GNU libc crypt() doco. It defines a whole family of 'impossible' values that signal errors, by dint of listing a load of magic characters that should never be in the encrypted string. Including asterisk.
No. https://man7.org/linux/man-pages/man3/crypt.3.html
My guess is that you are reading the libxcrypt crypt(3) manual: https://www.mankier.com/3/crypt
That's not GNU libc, but frequently deployed on Linux systems.
3 replies →
> Also check FreeBSD's OpenPAM for a case in point. Even today, it does not check for a NULL return from crypt() and leaps straight to strcmp().
Damn, that's a bug! Please file bug report.
3 replies →
The most charitable interpretation here is that musl authors are incompetent.
There is simply no universe in which a security-conscientious reviewer looks at the code, spots * being a valid return value, remembers that * is the default value in /etc/shadow for "no password login please" and then is all like:
This is fine, you check for * and return + instead.
There are more 32-long character sequences than atoms in the universe. Pick one, any of them instead.
Returning a string on error honestly is defensible (especially in a language like C where the type system doesn’t have exceptions or result types), the question is why the string was “*” instead of like “crypt ERROR: input password cannot be longer than 256 characters” lol
No. Musl claims (claimed?) to be strictly POSIX compliant, and POSIX requires crypt to return NULL on errors.
"*" is what's been historically used to represent an invalid/no value in passwd/shadow, so it makes some sense.
Of course given the chance that one might do a direct string comparison between crypt's return and the passwd/shadow file's hash field it was still a not well thought out choice.
No, it's absolutely not defensible.
Precisely.
> The goal from an attacker’s perspective is to get this to be true: `$ret = crypt($pw, $hash); $ret == $hash` when called by OpenSSH’s sshd.
A typo: `$ret = crypt($pw, $salt)`
weird chain of events and could have been handled in a better way for sure
"The OpenSSL adjustment ensures a minimum hash length for the user before logging them in, even if crypt() succeeds somehow."
OpenSSL??
Sorry, should have written OpenSSH.
Great explanation. This affirms my biases that shell scripting and everything-is-strings is a mistake. Horrific.
Shell is for humans to read and act on it.
Not to programmatically parse 'ls' output or some /proc/some/file.
Eg. git started as Perl scripts then moved to C to realy have data straight from C calls from system.
So for example all Internet things calling "openssl -some -options ...." are pure trash :>>
Even good, well-founded intentions can end terribly. The mistake is doubling down on those decisions when the mistake has become obvious.
I am also in the camp that CLI tools should return objects that can be parsed (by the shell or otherwise).