334459 - Variable "cipherName" tracked as NULL was passed to a function that dereferences it. [@ PORT_Strdup - SSL_SecurityStatus]

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P3)

Description

[timeless]

found by coverity

Comment 1

[timeless]

Attachment: don't jump after looking

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Comment on attachment 218797 [details] [diff] [review]
don't jump after looking

If cipherName is ever NULL here, there is a BIG flag elsewhere in the 
SSL code.  It would require that ss->sec.cipherType contain an invalid
value, outside of the range of the arrays ssl_cipherName and 
ssl3_cipherNamem which would indicate a coding error in libSSL.

This patch does avoid that crash, but it should be more aggressive.
We should put in an assertion that cipherName is not NULL.
So add one more line, right here.


>-	if (cipherName && PORT_Strstr(cipherName, "DES")) isDes = PR_TRUE;

        PORT_Assert(cipherName);

>+	if (cipherName) {
>+            if (PORT_Strstr(cipherName, "DES")) isDes = PR_TRUE;
> 
>-	if (cp) {
>-	    *cp = PORT_Strdup(cipherName);
>-	}
>+            if (cp) {
>+                *cp = PORT_Strdup(cipherName);
>+            }
>+        }
> 
> 	if (kp0) {
> 	    *kp0 = ss->sec.keyBits;

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

> If cipherName is ever NULL here, there is a BIG flag elsewhere in the 
Make that:  a big BUG

Comment 4

[Alexei Volkov]

tip:
/cvsroot/mozilla/security/nss/lib/ssl/sslauth.c,v  <--  sslauth.c
new revision: 1.16; previous revision: 1.15

3.11 branch:
/cvsroot/mozilla/security/nss/lib/ssl/sslauth.c,v  <--  sslauth.c
new revision: 1.15.2.1; previous revision: 1.15

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

CID 202