Comment by amluto

> When the salt is "x", it'll return "*" and still not match, won't it? The code is correct as long as the second argument is the hash that's being checked. And like you say, the sha512 path isn't relevant since you only get there if the salt started with "$".

I think you're right. That's what I get for doing a very very quick read.

> I think you're misunderstanding the intent of that check. It was added by the 2016 change linked in the writeup[1] and causes OpenSSH's xcrypt() to select and use a salt from a different line of /etc/shadow to prevent timing attacks. It is correct for a crypt() implementation that returns NULL on error.

I don't care what the intent of the check is. If you're doing a fake password hash to obscure timing, remember that it's fake and don't let the user log in! Don't deliberately pass nonsense into the live check and rely on the nonsense check failing to prevent the fake check from accidentally working. And don't have a code path that thinks it's checking the real password but passes in the wrong second parameter to crypt().

Or, better yet, have an entirely separate function fake_check_password() that does the fake check, returns void, and cannot possibly be confused with the actual password check code.