← Back to context

Comment by citrin_ru

2 days ago

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.

The idea of trying to provide a workaround for buggy code calling crypt() is terrifying. For sure, trying to grease the skids for buggy code calling printf(), go for it. But it is hard to imagine cases where slightly-incorrect usage of crypt() is a good idea.

This is still a consequence of a poor type system.

  • I don't think so. Even in a language with a good type system like Rust, somebody could have made this API:

        /// Hash the phrase with the given setting.
        fn crypt(phrase: &[u8], setting: &[u8]) -> [u8; CRYPT_OUTPUT_SIZE];
    

    Then, in a later version, `crypt` needs to become fallible, so they need to signal errors somehow. But because they really want to preserve backward compatibility, they can't change the output type to `Result<..., ...>`. And so they end up doing the same thing that was done in libcrypt - they output some error value and hackily make sure it's not equal to the `setting` string, under the mistaken assumption that the output of `crypt` would only ever be compared to the `setting` string. And then someone creates the exact same bug.

    Maybe I'm wrong, but it looks to me like the exact same incentives and means to screw things up exist in C and in Rust for this particular bug.

    • IMO type safety requires both language features and a mindset where you take efforts to keep things type safe. Your example has only the former.

      For example, in a Rust web app, you can choose to use String for encoded URLs, unencoded URL fragments, escaped html, unescaped HTML, usernames, etc. Or you can choose to use newtypes and carefully design all APIs to only take and return the correct newtypes.

    • You are looking at it from the wrong direction.

      The problem is not that you can write bad stuff in rust, the problem is that this particular usage pattern can't be sanely coded in C. This is a public API that should inform users and enforce certain properties. A Results type does exactly that, you would have to go way out of your way to force invalid stuff through that type alone.

      7 replies →

    • OP suggests it was always fallible, they switched to a sentinel to mitigate callers dereferencing null, which they wouldn't have with Option/Result. https://git.musl-libc.org/cgit/musl/commit/?id=b3c4cc12

      But in your version, you can't preserve backwards compatibility, making an infallible function fallible is a breaking change. For something comparable to libc you might want to (eg Rust std still has mem::uninitialized), but even with security issues, I feel like people would be more inclined to deprecate for a new function (or panic, which does have its own problems).

      A good type system also means things are written with better forwards compatibility, and can be safely changed later. The Rust equivalent to `crypt` likely wouldn't exist: a low-level library would just provide hash functions, leaving callers to mitigate DOS etc; a broader library might return some opaque handle with equality/serialization, which you couldn't try to compare to a passwordless user; a high-level library would parse the account store itself. None of these really need to change in incompatible ways, they have all the information to add features and fixes at their own level of abstraction.

    • Yes, even in Lean you can just refuse to prove any properties.

      But if you have a stronger and more expressive type system, you are more inclined to use it, instead of looking for silly workaround like in C.

    • You wouldn't be concerned about breaking backward compatibility with rust though, would you? A recompile would be needed anyway and the change would be detected and enforced. The rationale for the bad API here (two types in one variable) was that users were ignoring NULL - that's the part which wouldn't happen in rust.

      1 reply →