351769 - pk12util leaks password strings

Status: RESOLVED FIXED

(NSS :: Tools, defect, P2)

Description

[Nelson Bolyard (seldom reads bugmail)]

In function P12U_GetP12FilePassword of pk12util, we find this loop:

	for (;;) {
	    p0 = SECU_GetPasswordString(NULL,
					"Enter password for PKCS12 file: ");
	    if (!confirmPw)
		break;
	    p1 = SECU_GetPasswordString(NULL, "Re-enter password: ");
	    if (PL_strcmp(p0, p1) == 0)
		break;
	}

SECU_GetPasswordString returns a strdup'ed string.  If the two strings
(p0 and p1) don't match, the loop repeats without zeroing and freeing 
the just-gotten values, leaking them.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Worse, this loop doesn't check p0 or p1 for being non-NULL before
passing their values to PL_strcmp.  Boom.

Comment 2

[Neil Williams]

Fixed by patches to bug 351767.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Neil, bug 351767 was fixed in 3.11.7, but this bug is targetted at 3.11.8.
which is correct?