Hacker News
points by dboreham 2 days ago | 0 comments
> musl can return “*” for errors

wtaf ?

[–] za_creature 2 days ago
There is no doubt in my mind that this was intentional.
[–] mmoll 2 days ago
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.
[–] zbentley 2 days ago
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!

[–] ghostly_s 2 days ago
But choosing "*" as the character to return is an insane choice for anyone who knows the basics of linux sysadmin, or hell anyone who's ever used a search field in any context and thus knows it is ubiquitously used as a wildcard operator.
[–] pdw 2 days ago
But it fits the common Linux implementation of the function... e.g. the Debian manpage says:

> Upon error, crypt_r, crypt_rn, and crypt_ra write an invalid hash to the _output_ field of their _data_ argument, and crypt writes an invalid hash to its static storage area. This string will be shorter than 13 characters, will begin with a ‘*’, and will not compare equal to _setting_.

[–] crunchatized 13 hours ago
That says 'begin with,' which does not remotely require it be exactly the 1-character string '*' which probably was a bit of an insane choice under GP's logic.

And that's also only in Debian 11 and later. That man page said something entirely different in Debian 10 and earlier, when they shipped glibc's libcrypt. Now they ship libxcrypt instead.

Fedora asked libxcrypt to give them back glibc's behavior, returning NULL, and they obliged by adding ./configure --disable-failure-tokens. But it's an option that Debian never enabled.

https://github.com/besser82/libxcrypt/issues/25

https://salsa.debian.org/md/libxcrypt/-/blob/master/debian/r...

[–] cloudfudge 2 days ago
It's almost sensible if you only think of one side. It's like if /etc/passwd put "NO_PASSWORD" in the field for disabled logins and someone was like, oh, I'll be clever and say crypt() returns "NO_PASSWORD" on error because that's obviously invalid and won't match. But that's exactly what *does* make it match.
[–] bryan_w 2 days ago
But this also needs the "ddos" patch that was submitted later as well.

That seems orchestrated.

[–] bryan_w 2 days ago
Also, from the patch author:

https://lists.llvm.org/pipermail/llvm-dev/2019-June/133308.h...

[–] bch 2 days ago
> musl switched to returning `*` so folks stopped accidentally using NULL results, which was causing crashes and risked false-positives comparing NULL to itself.

Of all the things to mitigate...

1) crashing should be an option - it noisily indicates "there's a problem" and if it faults and dumps core, you've got a guided tour of your problem.

2) Is testing NULL as a passable option likely? I'd guess a more likely idiom is:

  if(NULL==(str=somefunc(arg1, arg2))){
    return err;
  }
[–] xp84 2 days ago
> crashing should be an option

Wise words. I think early in my career I tended towards coding as defensively[1] as possible in a very specific way of "don't crash", as I definitely was not working in codebases I felt I had a full picture of. "I don't know who calls this method, and I don't want to get a call in the middle of the night that I broke prod" so I would say, return an empty string when asked to format a datetime except I'm passed a string or an int. (Yes, I was working in dynamic, untyped languages, too).

Later on, even in untyped languages, I came around more to "Throw loud and specific exceptions when asked to do things that are nonsensical" - it requires more overall cojones though because it may cause "your code" to crash. But the realization I came to was that it's better to crash ('a MaliciouslyLongPasswordError was uncaught') because "AuthManager asked PasswordService to check a password that was 2MB long" than to paper that over until something like this comes out. Or, the more common situation, you're passed garbage, you return something that won't choke the caller at the time, but what it saves to the database in that scenario ends up being corrupt data. 6 months later someone notices, and has to sort out a data mess that may or may not be recoverable.

[1] note that most "code defensively" advice is good. What I was doing could be thought of as a foolish misinterpretation of the phrase.

[–] oasisaimlessly 2 days ago
FYI, your learned approach is also known as "fail fast" methodology[1][2].

[1]: https://www.martinfowler.com/ieeeSoftware/failFast.pdf

[2]: https://en.wikipedia.org/wiki/Fail-fast_system

[–] zbentley 2 days ago
“A good offense (SIGABRT) is the best defense(ive code).”

I dig it.

[–] PokestarFan 2 days ago
I think the issue is that a lot of callers were not aware that crypt() could error at all and all the issues are downstream of that. If OpenSSH knew that crypt() could error they would have handled it (possibly by inspecting errno).
[–] creata 2 days ago
I don't think that's the problem. The man page for crypt clearly says that erroring is a possibility and that it's not standardized how it errors:

> The behavior of crypt on errors isn't well standardized. Some implementations simply can't fail (except by crashing the program), others return a null pointer or a fixed string. Most implementations don't set errno, but some do. POSIX specifies returning a null pointer and setting errno, but it defines only one possible error, ENOSYS, in the case where crypt is not supported at all.

In my opinion the problem is that the behavior of crypt is underspecified - it isn't even specified how it signals failure, which is shocking.

Another disturbing decision that crypt's signature takes in the hash its output is supposed to be compared against - if it weren't for that, this half-baked "on error the output won't compare equal to the input" measure that crypt and crypt_* use wouldn't have been possible.

You can't check errno to see if there was a failure because, as the man page says, the value of errno is unspecified when crypt succeeds.

[–] throwaway2037 2 days ago

    > You can't check errno to see if there was a failure because, as the man page says, the value of errno is unspecified when crypt succeeds.
You can set errno to zero before the call. If the value is non-zero after the call, that is a good heuristic for failure.
[–] zbentley 2 days ago
...unless crypt() is one of the many, many functions which themselves internally call functions that set errno. In which case you can't be sure.
[–] za_creature 2 days ago
As opposed to the false positives returned by comparing '*' to itself.

Why * specifically? Why not some random guid generated before committing, if it's so important to not let buggy software crash?

[–] zbentley 2 days ago
I'm agreeing with you, to be clear.

But buggy software might not crash; it might compare NULL == NULL and misbehave, instead.

So there's really no good option here, which is why C is a rancid choice of language for publicly-usable APIs. But we're stuck with it. So it goes.

[–] dboreham 2 days ago
I've been programming in C since 1978 and never until now heard of a function returning "*" on error.
[–] zbentley 2 days ago
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.

[–] za_creature 2 days ago
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.

[–] tredre3 2 days ago
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).
[–] zbentley 2 days ago
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.

[–] cowlevel 2 days ago
C programs wouldn't accidentally compare NULL to NULL. They'd either check for null explicitly, or they'd pass the null to strcmp and crash.
[–] nextaccountic 1 day ago
Yep. See: underhanded c code contest

https://www.underhanded-c.org/

[–] dboreham 2 days ago
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?"
[–] JdeBP 2 days ago
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 "*".)

[–] boomlinde 2 days ago
> 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.

[–] JdeBP 2 days ago
Well caught. I was reading what man -s 3 crypt brought up, which mentions GNU and -lcrypt and only right at the very bottom says something else.

* https://manpages.debian.org/trixie/libcrypt-dev/crypt.3.en.h...

Although that's a fine distinction for the point being made, as it's still the manual page for the crypt() function that one gets on the operating system, which that other manual page is not.

[–] boomlinde 1 day ago
That naturally depends on the operating system and the crypt implementation they include.
[–] JdeBP 1 day ago
Not really. For Linux-based operatings systems, the GNU libc manual cryptography section is a shadow of its former self. It has no crypt() function to accompany it any more, and has not had since 2023.

* https://sourceware.org/git/?p=glibc.git;a=commit;h=e6e3c6668...

Notice that Debian didn't list your manual page, which doesn't accompany a crypt() function at all, since it isn't actually the GNU libc manual but a separate independent manual for Linux. The GNU libc manual nowadays acknowledges Michael Glad's work but no longer documents crypt(). It used to, and it used to explain some of the 'impossible' encrypted string values:

* https://sourceware.org/glibc/manual/2.38/html_node/Passphras...

You noticed that I'd not got the GNU libc manual. Neither had you. (-:

[–] ruslan 2 days ago
> 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.

[–] JdeBP 2 days ago
I suggest finding a code path where FreeBSD crypt() ever returns NULL, which would mean one of its internal functions returning non-zero, before getting too excited. They all end in return (0) . (-:

* https://cgit.freebsd.org/src/tree/lib/libcrypt

As I said, the callers of crypt() have for a long time been using the knowledge that crypt() on their platform simply never returns NULL. Unix has never been returning NULL from crypt() as far back as 7th Edition, when it simply always returned a pointer to a single internal static buffer.

* https://github.com/dspinellis/unix-history-repo/blob/Researc...

The reality is that all C library people have to work with a corpus of Unix applications that do not check for crypt() returning NULL that goes back almost 50 years, and an error return mechanism that barely exists outwith the pages of the SUS.

That includes callers in single-user mode in process 1 in some configurations.

* https://cvsweb.netbsd.org/bsdweb.cgi/src/sbin/init/init.c?an...

[–] ruslan 2 days ago
Well, crypt_blowfish() has a number of returns with (-1). I'm not familiar with Blowfish, so it's hard for me to trace the possibility of failure here. But what if in a distant future someone adds more hashing algorithms to the list without possessing historical knowledge of this "never return not 0" ?

* https://cgit.freebsd.org/src/tree/secure/lib/libcrypt/crypt-...

Failing to check result of crypt() in init cannot do much harm, doing so is equivalent of going system reboot (crash init :-)). Besides, whoever has access to system console can do whatever they want with the system, no password can prevent from that.

[–] JdeBP 2 days ago
I have no doubt, despite that Marshall Kirk McKusick is still committing in 2026, that FreeBSD developers will come along that do not know history. But they will get pointed to the manual page that (amusingly) says that returning NULL is 'not a standard behaviour' and that AT&T Unix never did that.

* https://man.freebsd.org/cgi/man.cgi?query=crypt&sektion=3&fo...

NetBSD's manual is more detailed on the subject, and explains the bind that library implementors have been in for decades, thanks to almost 50 years of applications knowing what crypt() 'really' does.

* https://man.netbsd.org/crypt.3#RETURN%20VALUES

And this despite the SUS trying to shoehorn an error return mechanism into crypt(3) in order to accommodate the problems that operating systems had exporting encryption code, two decades afterwards. (The 1997 GoSolo book explains that this is why there's an ENOSYS error.)

[–] za_creature 2 days ago
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.

[–] odo1242 2 days ago
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
[–] mmoll 2 days ago
No. Musl claims (claimed?) to be strictly POSIX compliant, and POSIX requires crypt to return NULL on errors.
[–] tredre3 2 days ago
"*" 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.

[–] gf000 2 days ago
No, it's absolutely not defensible.
[–] dboreham 2 days ago
Precisely.