Comment by throw10920
2 days ago
This is a side-effect of Unix-y "it's all plain text" design that leans extremely heavily upon in-band signalling (among other design decisions).
If /etc/shadow was a JSON (or XML, even, although that would make me feel gross), this bug would have been much easier to see, because a field that reads '"hash": ""' sticks out a lot* more than a custom microformat that you don't understand.
With data structures, there's never "plain text" - it's all structured. The question is whether you want to use existing formats and tools, or create a new one for every tool and literally make footguns like this.
A few years back I un-bashed my life as much as possible in favor of nushell (though it's not the only shell to encourage structure). I feel like it has really helped me notice a lot of this-needs-more-structure type problems outside of my shell that I otherwise wouldn't have.
The elegance of the unix-y approach is actually that it's all bits. You can just join things together and shove the bits through, that's a wild simplification compared with having vendor-specific kinds of plugs. But we could have that kind of compatibility while also benefiting from structure.
So I encourage everybody to give a shell with structured output a try, let it rewire your brain, and then ask yourself: how is the OS holding this back from being everything it could be? Because if we build that OS, it won't encourage these problems.
When I imagine it, it would probe either side of a pipe to see if they support negotiation. If not, send the bits through in the same old unixy way. If so, use the newest schema for structured data that is supported by both sides.
At Amazon we had RecordStream which implemented most of the standard Unix tools on structured streams. It was pretty easy to write filters, aggregators, and formatted output in something not much more complicated than sed/awk/cut/join/sort pipelines. There was a convenient structured log format for services that was often used as input.
https://github.com/benbernard/RecordStream
That looks really cool. I've been heading the opposite direction: posix shell for everything.
I have a bunch of management scripts, etc, that need to run on everything from a potato to a T-1000, and that's the only language that's always available. (OpenBSD sh, FreeBSD sh, busybox, bash, dash, zsh all can cope with it).
I'd love something that's at least JSON-typed (or equivalent), but that can be bootstrapped up on least-common-denominator posix.
Has anyone built something similar? (Yes, I'm being picky -- bun / ts is already a pretty low bar for cross platform support, but these scripts do stuff like install node, so I'm not totally crazy for asking.)
2 replies →
Returning “*” instead of NULL to signal failure is the real footgun. Don’t blame the format.
For historical reasons, we could’ve ended up JSON as “hash”: “*” just the same. The /etc/shadow format lets you skip values completely, like “::”, so your argument doesn’t hold.
Terminating an app because of a NULL dereference is the other footgun. Sadly, we still have “panic” in modern languages like Go and Rust.
"panic" is a good thing. There's always some error condition that your code cannot handle. At some point, the least bad thing to do is crash.
For instance, what if your error handling code throws?
One spectacular example of panic saving the day was the Apollo 11 1202 error. It was basically a watchdog timer that fired if the landing computer was starved of CPU time. A misconfiguration caused the first moon landing to hit CPU overload. When that happened, the computer displayed 1202 and rebooted, then ran things in priority order. Apollo 11 landed in the middle of a tight 'panic, reboot, adjust thrusters' loop. Thanks to core memory, most of the heap survived the reboot, so the reboot step was fast enough for it to mostly land without human interaction. The pilot had to make ~1 adjustment to get it to actually hit the landing site, but the panic-ridden computer handled the rest of the descent.
https://dpma.de/english/our_office/publications/ingeniouswom...
sshole-type APIs < panicking APIs < required error handling.
Panic is fine, but fundamentally a whole-program crash is undesirable in a really wide range of use-cases. I don't want other SSH connections to crash (or the server to stop accepting valid logins until someone restarts sshd) if one person tries to log in with a credential that triggers a panicking bug in crypt(). Same for crashes in request handlers and so on.
The language/runtime itself doesn't (and probably cannot, at least if it's a general purpose language) distinguish between local and global error conditions, so aborts end up being either unnecessarily damaging, or languages like Go end up normalizing panic "localization"/handling in dangerous ways. Isolation using threads isn't robust enough. Isolation using processes is far too fiddly, slow, and opaque.
The better solution is required error handling. If user code must engage with the possibility of an error state in order to access a result, that's a huge step forward. They can do the error handling wrong, but if you make most people have to do it at all most of the problems will go away.
There are other good solutions too, like Erlang's process boundaries, but those tend to have drawbacks that make them inapplicable to a lot of programming areas. Tagged unions, on the other hand, are only inapplicable in C library code because of language defects and terrible community conventions around ABIs.
"I forgot to check for an error" should not be implicated in bugs in 2026; we have had a large menu of possible alternatives (checked exceptions, unchecked exceptions, tagged unions, etc.) for decades. But we're still over here writing C (and Go), languages pathologically incapable of providing this ability.
3 replies →
> is the real footgun
This contains a false snuck premise - that one and only one thing was the footgun. This is categorically not true. An obtuse, poorly-understood, purpose-built text-based DSL is absolutely a footgun and if it was replaced with a JSON and inactive accounts had been indicated by `"active": false` instead of a freaking asterisk then this would absolutely would not have happened.
> For historical reasons, we could’ve ended up JSON as “hash”: “” just the same.
That's dishonest, because it intentionally conceals the fact that that's a vanishingly* unlikely scenario.
People have brains that, factually, are far more likely to see a single context-less character "" and not realize the implications of it than to see `"hash": ""` and think "oh, that's normal".
This is just straight-up psychologically true.
> The /etc/shadow format lets you skip values completely, like “::”
Totally irrelevant.
> so your argument doesn’t hold.
False `::` is both (1) still harder to understand than `"active": false` and (2) even though the syntax existed it wasn't used here because everyone thought they understood the system.
Don't make excuses for this insane text syntax. It is, factually, harder to understand than a decently-written JSON format.
The issue isn't the config, but the fact that crypt() doesn't have any way to signal failure. In better designed languages you would have exceptions, result types, or something. Even in C you have "return integer indicating success or error" which would've prevented the confusion.
Now the public API is hard to change but you could still avoid the issue by returning a longer token like "==DANGEROUS_CRYPT_ERROR_length==" or "==DANGEROUS_CRYPT_ERROR_unsupported_alg==" which would never match the password field unless someone was really trying to induce the issue.
>but the fact that crypt() doesn't have any way to signal failure
https://pubs.opengroup.org/onlinepubs/009604599/functions/cr...
>Upon successful completion, crypt() shall return a pointer to the encoded string. The first two characters of the returned value shall be those of the salt argument. Otherwise, it shall return a null pointer and set errno to indicate the error.
returning a sentinel value of NULL and setting errno is pretty standard C, and shouldn't cause an error.
OP's sshhole link specifically notes that it _should_ return NULL, but some MUSL devs decided they wanted it to return trash instead, and the trash they returned instead of NULL caused this.
All because some devs were too lazy to check the result and another dev decided to be clever and "fix" NULL errors from crypt by sending back 'unmatchable' trash banking on devs not checking NULL.
edit: apparently crypt implementations have been trash ignoring the NULL return for a long time. this isn't only on MUSL, but it's still because people ignored and tried to work around the existing out-of-band error mechanism.
> returning a sentinel value of NULL and setting errno is pretty standard C, and shouldn't cause an error.
> OP's sshhole link specifically notes that it _should_ return NULL
I meant there's no mechanism besides returning NULL that would make it hard to ignore the issue (exceptions need to be caught, results need to be unwrapped and their failure handled, an integer return value makes it clearer that there's something there that you need to check). I also missed errno codes when glancing at the manpage earlier which should illustrate the problem.
And yes, whoever first had the brilliant idea to start returning garbage never should've done that, I'm not disputing that.
This is true, and "trash" is putting it mildly.
But all those sentinels were added for the reason that, when your code is a public API intended to be widely used by who-knows-who, it needs to be really difficult to misuse it. NULL-plus-errno is easy to misuse. Yes, it's a dangerous mistake and your own fault if you do it. But people have been making that mistake for years and don't show any signs of stopping.
The root cause here is that C is fundamentally a rotten choice for public APIs of any kind. I griped more here: https://news.ycombinator.com/item?id=50381800
The spec implies a string of at least two characters in the success case, am I reading that right?
If this is true, then the entire thread branch here on HN is entirely wrong. What is going on??
13 replies →
Yup, that's the way to explain it in order to blame musl.
Here's how to explain it in order to blame openssh:
openssh processes passwords and doesn't want to reveal through timing whether a user has a valid password (or exists at all). So far so good.
What they do if they hit a user with no valid password (i.e. "*") is that they look for another user with a valid password, to use the seed from. A bit weird already, but OK from a perspective of avoiding implementation specifics on seeds and hash algorithms. They know at this point that the result must be to deny access even if the hashed value matches, because that hash is for another user.
So they put the password received over ssh into crypt() with this other user's hashed value to get a seed. And then accept it if it matches, apparently?
Homework: what happens if the password actually matches another user? Can I log in as root if openssh randomly picks my user for the fake salt, and I type my own password correctly?
Except it's not trash. It's a backdoor funded by our glorious leader Kim Jong Un.
Personally I think there was nothing wrong with returning NULL. "But if people don't check for it it crashes their programs!" Good! Let it crash loudly! Then they notice and fix the issue.
Also I think it could have returned ":". Given the file format of /etc/shadow and /etc/passwd I don't think it's possible to have a : as a value for anything.
> Also I think it could have returned ":". Given the file format of /etc/shadow and /etc/passwd I don't think it's possible to have a : as a value for anything.
I wonder if somebody else would have thought the exact same thing in a different (non-/etc/*) context, and then you get the same effect in a more obscure manner.
1 reply →
> The issue isn't the config
That's a meaningless statement. There isn't a singular "the" issue like you said. Any one of several different technical decisions would have completely prevented the issue. Including, to my point, /etc/shadow being a JSON file where an account being locked is an `"active": false` field instead of an asterisk with no intrinsic meaning.
No, that's just more in-band signaling!
Some badly designed password hashing algorithms (cough bcrypt cough) have maximum password lengths, ignoring everything after X characters. So it could happen that I set my password to a 100-character string, it gets saved as "==DANGEROUS_CRYPT_ERROR_length==", and then anybody can log in to my account by entering an unrelated long string that returns the same "error".
If a function promises a string result, it should only provide a string if the function succeeded. The null was much better than magic strings.
I know, it's bad. I agree that it should've stayed NULL. But if you really really really think you have an acceptable reason to do something like this (and you probably don't), the least you can do is use a non-trivial token.
1 reply →
Yes, absolutely. Take for example this cURL command:
Random parts of it will be interpreted by the shell, others by cURL, or the filesystem, or the remote server. They each have their own encoding rules, nested. I count ten distinct separators and nine ways of escaping/tagging. It's one command. No, just no.
We have seriously overused strings and we're now paying the price.
Also: free text is the input format of LLMs...
At this point I’m resigning myself to think that we’ve just gotta wait for some people to die. It feels very dramatic and glib (no pun intended lol) and whatever to say, but at this point it feels like there’s a certain hostility in some parts to changing anything from the platonic ideals of however computing happened to work in 2004. “Oh we don’t need to mess with it!”
How did you arrive at that conclusion? If anything, this vulnerability happened because someone decided to change the behaviour of crypt() without fully understanding the consequences. This specific vulnerability did not exist in 2004.
I agree with him. I only managed to get asterisks enabled by default in sudo because sudo-rs isn't owned by people who have decided that the 70s was the perfect decade and nothing should ever change. And look at the insane pushback when it did!
I can't even get an obvious UX improvement through PAM because they like it broken.
https://github.com/linux-pam/linux-pam/issues/778
I think this is a bit unfair. The way I see it, a lot of people are saying that changing breaks lots of old stuff, and risks complete fragmentation. Those are real costs to change. It might still be worth it, but let's not pretend that everyone is resisting change just because it's change.
The costs to change are dropping significantly with AI.
3 replies →
> "it's all plain text" design that leans extremely heavily upon in-band signalling
Honestly I've been shaking my fist at these since 0 becomes True/Success in Bash and the opposite everywhere else
Error code 0 means success in a lot more places than just bash.
That's not specifically bash though, that's process exit codes
Yes. But it's still contradictory
3 replies →
I'd say another UNIX-y thing - "C is good enough" - is part of the problem. C makes programming painful which results in all sorts of hacks.
E.g. in Pascal you can can define a record and then defined file of record to have typed I/O. I.e. it comes with built-in serialization format supported by compiler. Not a good one - but perhaps a step in a good direction.
OTOH C people starving for serialization often believe that `struct` byte layout made by a specific version of a compiler is a serialization format - a view not endorsed by the standard.
Pascal's record typed files have the exact same problems regarding endianness and word size as directly writing structs to files in C, so I don't see how they're any better. They're also a complete deadend and unusable for pretty much any file format from the last 40 years+, since you can't even handle a header or magic bytes at the start.