[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160327033007.GA29317@openwall.com>
Date: Sun, 27 Mar 2016 06:30:07 +0300
From: Solar Designer <solar@...nwall.com>
To: musl@...ts.openwall.com
Cc: Timo Teras <timo.teras@....fi>
Subject: Re: [PATCH] crypt_blowfish: allow short salt strings
On Sun, Mar 27, 2016 at 05:11:21AM +0300, Solar Designer wrote:
> There is one maybe-bug seen in your test results: the "*" return on
> error. Normally, it would be "*0" or "*1" for crypt_blowfish, and it
> would never match the salt input. Rich, did musl retain this behavior?
> Where does the bare "*" come from? The concern here is that a "*" might
> also be returned when crypt() is called with "*" for the salt input.
> Then its output would match what may be a stored hash placeholder, where
> "*" means locked account or an error having occurred when the password
> was set. We must deny access in such cases, but returning "*" on all
> errors would grant access. This could be a musl or PHP security bug, if
> it's indeed as bad as it appears from that test.
I just discussed this with Rich.
Yes, musl's modified crypt_blowfish returns "*" on error. No, this
isn't a security bug in musl as a whole, because that code path is not
reached when the setting argument is "*" as well. So no match.
However, there's risk for code reuse from musl by other projects, and
for potential cut-down revisions of musl (with only bcrypt left, invoked
unconditionally). So I think a change is needed, either reintroducing
the "*0" / "*1" behavior (my preference) or returning NULL (Rich's
preference) on error like glibc does (and unfortunately crashing older
programs that don't expect this).
Alexander
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
