[Bug 2207] Potential NULL deference, found using coverity

[bugzilla-daemon at mindrot.org]

https://bugzilla.mindrot.org/show_bug.cgi?id=2207

--- Comment #2 from Arthur Mesh <arthurmesh at gmail.com> ---
Not sure why e-mail correspondence didn't make it here -- hence
reposting:
On Mon, Mar 03, 2014 at 09:27:38PM +0000, bugzilla-daemon at mindrot.org
wrote:
> We don't normally mark crashers as security bugs unless they take down
> the master sshd process.

Noted.

> That being said, there is no NULL dereference here anyway. See the
> "kdfname == NULL"

Disagree. Let me try to be more specific:

Let's for the sake of argument assume:
kdfname = "bcrypt"
passphrase = NULL
ciphername = "none"

Please ignore this bug if such assumptions are invalid.

 277         if ((passphrase == NULL || !strlen(passphrase)) &&
 278             strcmp(ciphername, "none") != 0) {
 279                 /* passphrase required */
 280                 goto out;
 281         }

Given the assumption, condition in line 277 is false.

 283         if (kdfname == NULL ||
 284             (!strcmp(kdfname, "none") && !strcmp(kdfname,
"bcrypt"))) {
 285                 error("%s: unknown kdf name", __func__);
 286                 goto out;
 287         }

Given the assumption, condition in line 283 is false.

 288         if (!strcmp(kdfname, "none") && strcmp(ciphername, "none")
!= 0) {
 289                 error("%s: cipher %s requires kdf", __func__,
ciphername);
 290                 goto out;
 291         }

Furthermore, condition in 288 is false as well.

 338         if (!strcmp(kdfname, "bcrypt")) {
 339                 if ((salt = buffer_get_string_ret(&kdf, &slen)) ==
NULL) {
 340                         error("%s: salt not set", __func__);
 341                         goto out;
 342                 }
 343                 if (buffer_get_int_ret(&rounds, &kdf) < 0) {
 344                         error("%s: rounds not set", __func__);
 345                         goto out;
 346                 }
 347                 if (bcrypt_pbkdf(passphrase, strlen(passphrase),
salt, slen,
 348                     key, keylen + ivlen, rounds) < 0) {
 349                         error("%s: bcrypt_pbkdf failed",
__func__);
 350                         goto out;
 351                 }

Condition in 338 is true, and line 347 could produce a NULL dereference
(strlen(NULL)).

Condition in 338 is true, and line 347 could produce a NULL dereference
(strlen(NULL)).

(Again, assuming that lines 339 and 343 do not fail).

Perhaps I am missing something obvious here..

Thanks

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.