Hacker News

points by gf000 2 days ago | 0 comments

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.

[–] creata 2 days ago

I don't think so. In both Rust and C, the solution is the same - make the function explicitly fallible, instead of trying to preserve backward compatibility. In C, the function would probably just look like this, returning zero on success and negative on failure:

    int crypt2(const char *phrase, const char *setting, char *out);

You can call such a signature "insane" if you want, and that's fine, but it would have averted the bug that is the topic of discussion.

[–] markasoftware 2 days ago

what you are suggesting is a form of type safety. There are still degrees of type safety even in C, and this bug resulted from deciding that backwards compatibility is more important than type safety, which I think we agree on.

[–] lokar 2 days ago

And C has effectively deprecated bad APIs over the years. Just not this one.

[–] quotemstr 2 days ago

It's a perfectly sane signature, especially coupled with an API surface that uses the return channel for errors generally.

[–] kazinator 1 day ago

It would not have averted the bug. The bug is that out is the value "*" even when setting is also "*".

It would have averted the bug in a program which checks the int return value for error.

But we had that already: the null value, different from any valid string. That had to be done away with because programmers couldn't be relied upon to check for it.

Those same programmers are not going to check an integer value which doesn't even threaten them with a segfault.

(However, there is a GCC attribute that can be given to functions such that a diagnostic is produced if their value is not used: that could be profitably used with the above API.)

[–] cowlevel 2 days ago

This isn't good either. Crypt is a pure computation that can't fail for arbitrary reasons. The former implementation could fail if the salt was invalid, which is something you could guarantee. Now it also fails if the password is invalid. That's new behaviour and no matter which way you wrap it up it has the potential to be incompatible. They should have added the maximum length recommendation to the man page, and let DoS attacks continue until the calling software was updated, not the crypt library.