Hacker News
points by GuB-42 2 days ago | 0 comments
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.

[–] LordDragonfang 2 days ago
> 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)

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

[–] Xirdus 2 days ago
Maybe your compiler isn't, but GCC is designed to summon every nasal demon it can. It especially loves to elide null pointer checks. There was a PR that almost made it to Linux mainline that did some weird stuff in kernel space where memory mapped to address 0 is valid and contains important stuff. It got a pointer, read data from it, then checked if pointer is null before giving root permissions for some operation. Reading the code, everything seems alright. Even stepping through it with debugger shows that it works. But when compiled with -O2 or higher, GCC removed the null check and now everybody got root permissions unconditionally - because reading from null pointer is UB regardless of circumstances, and GCC treats UB as dead code even if it is reachable.
[–] cowlevel 2 days ago
> GCC is designed to summon every nasal demon it can.

Which is zero, to be clear. There's not even a standard USB device class for a nasal demon releaser

A null check elision is not a demon up my nose - it's a null check elision.

[–] Xirdus 1 day ago
Sure, and char[] is no string because you can't tie anything with it. And don't get me started on the insects that allegedly live inside digital source files - that's an absurd concept, no self respecting computer specialist should be talking like that.
[–] neomantra 1 day ago
Having made devices for other human holes, you can just use the standardized HID class. It can be adapted for nasal demons incoming and outgoing.
[–] miki123211 1 day ago
Wait, so if you're running on embedded, you're not allowed to store anything at address 0, requiring you to waste that little bit of RAM?

That's... a bizarre choice, considering how religious C is about not making any assumptions about what a machine can and cannot do, even going to the trouble of working with non-IEEE floats and int types that are not a multiple of 8 bits.

[–] Xirdus 1 day ago
Unfortunately yes. ISO/IEC 9899:1999 footnote 83: "Among the invalid values for dereferencing a pointer by the unary * operator are a null pointer".
[–] annex-winged-cr 1 day ago
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
[–] miki123211 1 day ago
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.

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

[–] poly2it 2 days ago
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.
[–] eru 2 days ago
> [...] 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?

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

These types are not the same:

    Ok<String>
    Err<String>
[–] markasoftware 2 days ago
Those are not types at all. Ok and Err are type constructors for the Result<A, B> type that take arguments of type A and B respectively.
[–] SkiFire13 2 days ago
> type constructors

Nit: the type constructor is `Result`: given two types `A` and `B` (not instances of those types, but actual types) it gives you a new type `Result<A, B>`.

`Ok` and `Err` are called "variants" and can be used to construct a new _instance_ of `Result<A, B>`.

[–] markasoftware 1 day ago
Ah, thanks! I'd remembered the term "data constructor" from Haskell and called it a type constructor instead.
[–] kazinator 1 day ago
Ok and Err are "thingies" which are in the type system, which give rise to distinctions that allow for compile-time diagnostics. They are part of the type.

Ok and Err are not just type constructors because they appear on the left hand side in pattern matching: they "do" matching and destructuring also.

[–] kazinator 1 day ago
The success and error types are not the same, because they are in different positions in the type, and so distinguished by different patterns.

If you have literally the above convention where a valid result is a String, and an error result is also a String (e.g. error message) that is still type safe.

  match (whatever()) {
    Ok(s) => println!("good result string: {s}"),
    Err(s) => println!("error string: {s}"),
  }
The success type and error type having to be different is not a good way to express the idea; rather that success and error cases are distinguished by type (even if the success and error data units are of the same type).

In other words, the "Ok" and "Err" entities are an ingredient of the type and that effectively makes the result and error be distinguished at the type level.

But we could also argue that Rust still allows you to write an integer-returning function such that -1 means error.

[–] eru 1 day ago
Basically, you need the whole 'success and error types are different' in a language like Python if you make the return type something like "int | string", where 'string' stands for your error message.

Of course that breaks down, when the success type is also an error message, eg when you are looking up something in a catalogue of error messages.

Rust has no trouble with this, for exactly the reasons you line out.

[–] quotemstr 2 days ago
Contemporary opinion among who? Words mean things, and "type safe" simply does not mean "agrees with my preferred style of error reporting"
[–] poly2it 2 days ago
Papers on type soundness from the last 30 years usually explore the domain further than C semantics.
[–] pron 2 days ago
Soundness means that the program behaves in accordance with what its types express. You're right that C may be unsound due to undefined behaviour, but I don't think this is about soundness. What you're saying is that papers on types show how you can use types to express more constraints.
[–] ranger_danger 2 days ago
There can be other valid perspectives and definitions of that word than yours.

I think almost all (tech) arguments stem from simple disagreements and/or different assumptions made on the definition of a (subjective) term.

[–] pron 2 days ago
You may be right in general, but soundness comes from formal logic, where they're pretty fastidious with their definitions. Soundness certainly doesn't mean the expressivity of the types (i.e. what constraints they can represent); it's about the relationship between the runtime behaviour of a program and the types in the code.
[–] ranger_danger 2 days ago
Again I think you're trying to push a specific definition as more correct than OP's somehow. Why not just ask them what they meant to understand their intent/context instead of saying they are wrong?

Unless the point of your argument isn't to understand OP but simply to "win."

[–] ciefa 2 days ago
Can you link some?
[–] quotemstr 2 days ago
That's not the same as asserting that soundness requires adherence to the current fashion, sum types over results and errors. TIMTOWTDI.

You don't have to think C is sound or good to distinguish a disfavored API style from mathematical unsoundness. There are infinitely many type safe APIs and languages that are, despite their soundness, ugly and hard to use.

Let's not project every dimension along which a language can be good or bad onto the axis of safety.

[–] majorchord 2 days ago
Opinion among people who know better than to try to authoritatively explain subjective words with rigid dogma, because that just makes them look silly.
[–] alerighi 1 day ago
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:

    crypt_error_t crypt(...args, char *result, size_t resultLength)
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

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

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

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